-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
{ids,checks}: update for new builder UID/GID values #1069
Changes from all commits
bda49fe
f29c6fc
9818968
2af5f0f
9c60c95
88b97aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,18 +4,21 @@ on: | |||||
push: | ||||||
|
||||||
env: | ||||||
CURRENT_STABLE_CHANNEL: nixpkgs-23.11-darwin | ||||||
CURRENT_STABLE_CHANNEL: nixpkgs-24.05-darwin | ||||||
|
||||||
jobs: | ||||||
test-stable: | ||||||
runs-on: macos-12 | ||||||
timeout-minutes: 30 | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
# We use the Determinate Systems installer for 2.18 because the | ||||||
# Sequoia UID/GID changes have not yet been backported to the | ||||||
# official installer for that version. | ||||||
- name: Install nix corresponding to latest stable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
uses: DeterminateSystems/nix-installer-action@main | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They recommend deploying the action directly from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW the action downloads an unversioned installer anyway so there's no meaningful pinning to be done here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with these changes being put in another PR, I think we will be able to pin both the nix-installer binary used and the Nix version installed using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The upstream installer only supports installing one version of Nix, AFAIK. Again this is just a temporary hack for a few days to ensure that we can test 2.18 at all. If we tried to use the upstream installer we could not activate the system. This was the only way I could make the tests not go red because of this PR. |
||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.13.6/install | ||||||
nix-package-url: https://releases.nixos.org/nix/nix-2.18.5/nix-2.18.5-x86_64-darwin.tar.xz | ||||||
Comment on lines
18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better to have a job for each installer rather than replacing the installer in this job There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have no choice until the Nix team release a 2.18 with the fix, per the comment. This is just a hack taking advantage of the fact that the DetSys installer already deployed the fix and can install multiple Nix versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently we're still running the tests on macOS 12, so adding a second job seems fine to me, even if the Nix installer doesn't support the Sequoia UID migration yet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test will fail because this PR means it will print the message informing that the migration script has to be run and error out. Which is the correct behaviour to ensure that users are prepared for the Sequoia update, but doesn't test what we want. Note that the upgrade breaks Nix entirely if no migration was done and the user will not even be able to activate nix-darwin to see the message. That's why it's urgent that people are informed and run the migration script before upgrading to Sequoia. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about in a different job running the official Nix installer then the migration script in CI and then try to do installation again? If this is too much extra work, we can leave it out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That could work, but it seemed more fiddly than just changing a few lines until the patched installer was out. If you'd prefer that I can try to implement it tomorrow, but I'll be busy for most of the day and I'm worried about how many users we'll be able to get this change in front of before Sequoia drops. I lean towards iterating on non-essential things after merge. |
||||||
- run: nix-build ./release.nix -I nixpkgs=channel:${{ env.CURRENT_STABLE_CHANNEL }} -I darwin=. -A tests | ||||||
- run: nix-build ./release.nix -I nixpkgs=channel:${{ env.CURRENT_STABLE_CHANNEL }} -I darwin=. -A manpages | ||||||
- run: nix-build ./release.nix -I nixpkgs=channel:${{ env.CURRENT_STABLE_CHANNEL }} -I darwin=. -A examples.simple | ||||||
|
@@ -26,7 +29,9 @@ jobs: | |||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- name: Install nix from current unstable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
uses: cachix/install-nix-action@v27 | ||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.24.6/install | ||||||
- run: nix-build ./release.nix -I nixpkgs=channel:nixpkgs-unstable -I darwin=. -A tests | ||||||
- run: nix-build ./release.nix -I nixpkgs=channel:nixpkgs-unstable -I darwin=. -A manpages | ||||||
- run: nix-build ./release.nix -I nixpkgs=channel:nixpkgs-unstable -I darwin=. -A examples.simple | ||||||
|
@@ -36,18 +41,20 @@ jobs: | |||||
timeout-minutes: 30 | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
# We use the Determinate Systems installer for 2.18 because the | ||||||
# Sequoia UID/GID changes have not yet been backported to the | ||||||
# official installer for that version. | ||||||
- name: Install nix corresponding to latest stable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
uses: DeterminateSystems/nix-installer-action@main | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.13.6/install | ||||||
nix_path: nixpkgs=channel:${{ env.CURRENT_STABLE_CHANNEL }} | ||||||
nix-package-url: https://releases.nixos.org/nix/nix-2.18.5/nix-2.18.5-x86_64-darwin.tar.xz | ||||||
- name: Install ${{ env.CURRENT_STABLE_CHANNEL }} channel | ||||||
run: | | ||||||
nix-channel --add https://nixos.org/channels/${{ env.CURRENT_STABLE_CHANNEL }} nixpkgs | ||||||
nix-channel --update | ||||||
- name: Install nix-darwin and test | ||||||
run: | | ||||||
export NIX_PATH=$HOME/.nix-defexpr/channels | ||||||
export NIX_PATH=nixpkgs=channel:${{ env.CURRENT_STABLE_CHANNEL }} | ||||||
|
||||||
# We run nix-darwin twice to test that it can create darwin-configuration correctly for us | ||||||
# but we expect it to fail setting up /etc/nix/nix.conf | ||||||
|
@@ -82,8 +89,9 @@ jobs: | |||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- name: Install nix from current unstable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
uses: cachix/install-nix-action@v27 | ||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.24.6/install | ||||||
nix_path: nixpkgs=channel:nixpkgs-unstable | ||||||
- name: Install nixpkgs-unstable channel | ||||||
run: | | ||||||
|
@@ -125,10 +133,13 @@ jobs: | |||||
timeout-minutes: 30 | ||||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- name: Install nix version corresponding to latest stable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
# We use the Determinate Systems installer for 2.18 because the | ||||||
# Sequoia UID/GID changes have not yet been backported to the | ||||||
# official installer for that version. | ||||||
- name: Install nix corresponding to latest stable channel | ||||||
uses: DeterminateSystems/nix-installer-action@main | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.13.6/install | ||||||
nix-package-url: https://releases.nixos.org/nix/nix-2.18.5/nix-2.18.5-x86_64-darwin.tar.xz | ||||||
- name: Install nix-darwin | ||||||
run: | | ||||||
mkdir -p ~/.config/nix-darwin | ||||||
|
@@ -209,7 +220,9 @@ jobs: | |||||
steps: | ||||||
- uses: actions/checkout@v3 | ||||||
- name: Install nix from current unstable channel | ||||||
uses: cachix/install-nix-action@v23 | ||||||
uses: cachix/install-nix-action@v27 | ||||||
with: | ||||||
install_url: https://releases.nixos.org/nix/nix-2.24.6/install | ||||||
- name: Install nix-darwin | ||||||
run: | | ||||||
mkdir -p ~/.config/nix-darwin | ||||||
|
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.
The manual should be updated to 24.05 as well
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.
It already was, I think.
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.
https://github.com/LnL7/nix-darwin/blob/master/.github/workflows/update-manual.yml#L24
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.
Ah, I thought you meant the README. I can handle this but would prefer to do it in a separate PR given the urgency of this one.