Skip to content
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

Generate fallback-paths in a flake output instead of "hidden" perl #11565

Closed
wants to merge 1 commit into from

Conversation

roberth
Copy link
Member

@roberth roberth commented Sep 23, 2024

Motivation

Currently, nix-fallback-paths.nix is generated in the release script. Let's make it more declarative instead, so that

  • Users can check that the fallback paths file is correct, by building it
  • It makes the release process easier to understand
  • It removes a duplication of the systems attrset
  • It simplifies the release script

The new package provides the contents for nix-fallback-paths.nix in NixOS, but without the weirdly specific name that's for the purpose of nixos-rebuild.

TODO Follow-up:

  • update release script to download the file instead of generating it

Question:
Do we want to rename this and provide JSON? I think using a data format for data is good, as it makes alternative use cases possible. The current name is a layer violation.

(Ideally we should have a store-level json based package description format, but that's overkill for this particular use case; it would make the entire solution reusable though, whereas the current thing or this PR are still an ad hoc format.)

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@roberth roberth requested a review from edolstra as a code owner September 23, 2024 00:58
@roberth roberth requested a review from Mic92 September 23, 2024 00:58
@roberth roberth force-pushed the add-all-output-paths-json branch from 27951ab to 6ab29bf Compare September 23, 2024 01:06
This implements the `fallback-paths.nix` format in NixOS
@roberth roberth force-pushed the add-all-output-paths-json branch from 6ab29bf to 7bd3f3b Compare September 23, 2024 01:07
@roberth roberth marked this pull request as draft September 23, 2024 01:16
@edolstra
Copy link
Member

See my comment here. We shouldn't have multiple release processes.

@Mic92
Copy link
Member

Mic92 commented Sep 23, 2024

My answer here: NixOS/nixpkgs#343657 (comment)

@roberth
Copy link
Member Author

roberth commented Sep 23, 2024

We shouldn't have multiple release processes.

Yeah I marked it as draft when I realized this was (obviously) already in the release script.
So while its original purpose isn't good, I think it's still beneficial to make declarative what we can, and do less in the release script.

  • Users can check that the fallback paths file is correct, by building it
  • It makes the release process easier to understand
  • It removes a duplication of the systems attrset
  • It simplifies the release script

I'll update the description

@roberth roberth changed the title Add nix-all-output-paths Generate fallback-paths in a flake output instead of "hidden" perl Sep 23, 2024
@roberth roberth marked this pull request as ready for review September 24, 2024 09:48
@roberth
Copy link
Member Author

roberth commented Sep 24, 2024

It's unnecessarily difficult to simplify the release script without having an example of this on Hydra, so I'll make this a two-parter.

Part one ready for review, right here!

@edolstra
Copy link
Member

It simplifies the release script

Does it though? I feel that

# Upload nix-fallback-paths.nix.
write_file("$tmpDir/fallback-paths.nix",
    "{\n" .
    "  x86_64-linux = \"" . getStorePath("build.x86_64-linux") . "\";\n" .
    "  i686-linux = \"" . getStorePath("build.i686-linux") . "\";\n" .
    "  aarch64-linux = \"" . getStorePath("build.aarch64-linux") . "\";\n" .
    "  riscv64-linux = \"" . getStorePath("buildCross.riscv64-unknown-linux-gnu.x86_64-linux") . "\";\n" .
    "  x86_64-darwin = \"" . getStorePath("build.x86_64-darwin") . "\";\n" .
    "  aarch64-darwin = \"" . getStorePath("build.aarch64-darwin") . "\";\n" .
    "}\n");

is simpler than nix-all-output-paths.nix. It's also a lot more efficient (a Nix expression that depends on the Nix closures for every platform is very painful for Hydra, since the builder needs to fetch all of those closures from the binary cache just to write a little text file.) It also means that we can't make a release if any of the non-critical systems is broken.

all-output-paths.{nix|json} seems like a misnomer since it doesn't include all output paths, just the bin paths. Including (say) the doc outputs might be useful for updating the manual on the website.

@roberth
Copy link
Member Author

roberth commented Sep 24, 2024

Does it though? I feel that [...] is simpler

Most of the complexity is in the interfacing with Hydra.
It should be little more than (formats.json {}).generate when we get rid of the .nix.

What weighs heavier to me is that it's hidden away in a release script.

a misnomer since it doesn't include all output paths

Fair. I meant to refer to all system types, but that's not clear at all.

painful for Hydra

Ok.

... back to draft

@roberth roberth marked this pull request as draft September 24, 2024 16:53
@roberth roberth closed this Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants