-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Macos big sur installer fixes #3996
Conversation
cc @LnL7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me.
scripts/create-darwin-volume.sh
Outdated
esac | ||
i=$((i+1)) | ||
done | ||
diskutil apfs list -plist "$1" | xmllint --xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict/key[text()='Name']/following-sibling::string[contains(translate(text(),'N','n'),'nix')]/text()" - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, did you check this with a few conditions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7ccb5b0
to
af6a895
Compare
@domenkozar I think this is roughly ready. Notes:
|
@abathur I'm a bit short on time to test this. However, if you generate the installer script, it should be trivial to test with github actions once 11.0 is available for testing. |
@domenkozar I have them for the last two commits. I'll ping you on IRC. |
If anyone wants to test this, I've hosted two darwin-x86_64 tarballs with matching stub install scripts (these scripts don't check the tarball hash).
For reference, I also re-ran some trivial CI jobs with these URLs.
Notes:
|
@domenkozar GitHub Actions added Big Sur preview runners today, so I've done comparison runs with the installers above:
|
Could we also run with the tbd branch? |
Keeping this commit narrow for reviewability, but some of these conditionals will change in subsequent commits in this PR. Fixes NixOS#3852.
- xpath -> xmllint: xpath's cli interface changed in Big Sur rather than add conditional logic for picking the correct syntax for xpath, I'm changing to xmllint --xpath, which appears to be consistent across versions I've tested... - /plist/dict/key[text()='Writable']/following-sibling::true[1] doesn't do quite what's expected. It was written to try to select a <true /> node paired with the Writable key, but it will also select the *next* <true /> node that appears even if it was paired with another key. - I think there's also a logic bug in the conditionals here. I'm not sure anyone ever actuall saw it, thanks to the xpath bug, though. With the xpath fix, this conditional passes if /nix does not exist, / IS writable, and the version is Catalina+. I think it meant to test for /nix does not exist, / is NOT writable, and the version is Catalina+. I reworked this lightly to make it a little clearer at the code level.
As mentioned in previous commit, Big Sur changes the syntax for the xpath command slightly. In the process of testing out replacements for these, I noticed a few small simplification wins.
Fixes NixOS#3957. Just runs both forms to minimize moving parts.
The move from release.nix to flake.nix appears to have lost some changes from NixOS#3628 / 1c56f18, leaving create-darwin-volume.sh out of the release tarball. Under the assumption that this was just an accident/byproduct of when flake.nix split off and not intentional, I am restoring those edits.
Some of the changes in NixOS#3788 to support non-systemd Nix installs don't appear to be aware that the darwin installer exists, which resulted in some skipped steps and inappropriate instructions.
Env vars for ZSH were moved from /etc/zshrc to /etc/zshenv in NixOS#3608 to address an issue with zshrc getting clobbered by OS updates, but /etc/zshenv doesn't exist by default--so *nothing* would get set up for zsh users unless they already happened to have /etc/zshenv. Creating these files if they don't exist. Also cut separate creation of profile.d/nix.sh, which isn't needed now.
0baa6e7
to
f289bdb
Compare
I've force-pushed a squashed history which collapses all of the incremental fix commits back down into:
|
Thanks! |
Cherry-picked 4 commits to 2.3-maintenance, we'll hopefully release 2.3.8 once nixpkgs is ready. |
@domenkozar I didn't notice that 2.3.8 has already been cut or I would've done this sooner, but I ran an install test just now on my laptop with Big Sur:
|
Nice!! |
Goal: make sure the installer scripts will run across macOS versions
Caveats, in case this ends up like the last one... 😬
This will also fix #3852 and fix #3957.
A few notes (repeated from a later comment):
file 'nixpkgs' was not found in the Nix search path...
errors on invocation. I don't think I caused this (maybe ec9dd9a did)? If that's correct, it'll need attention outside the scope of this PR.