Skip to content

chore: update flake and bump Nix to 2.28.2#3176

Merged
mkenigs merged 11 commits intomainfrom
bump-flake
Jun 10, 2025
Merged

chore: update flake and bump Nix to 2.28.2#3176
mkenigs merged 11 commits intomainfrom
bump-flake

Conversation

@mkenigs
Copy link
Copy Markdown
Contributor

@mkenigs mkenigs commented Jun 2, 2025

Run nix flake update and deal with the fallout. See commits for details of resulting changes required

Release Notes

We could possibly mention we now ship Nix 2.28.2

@floxbot floxbot added the team-cli Tickets relevant to the flox CLI team label Jun 2, 2025
@mkenigs mkenigs force-pushed the bump-flake branch 3 times, most recently from 755aaf4 to cbdf35e Compare June 9, 2025 20:15
@mkenigs mkenigs marked this pull request as ready for review June 9, 2025 21:42
@mkenigs mkenigs requested a review from a team as a code owner June 9, 2025 21:42
Comment on lines -851 to +852
old_lockfile: Option<Lockfile>,
new_lockfile: Lockfile,
old_lockfile: Box<Option<Lockfile>>,
new_lockfile: Box<Lockfile>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: did the struct get too large or why do we need to start box'ing them here?
I assume these rust changes are a result of also bumping the rust toolchains which brought new clippy lints?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah clippy complained, I assume because of a newer clippy

mkenigs added 11 commits June 10, 2025 10:45
This updates Nix to 2.28.2, which required:
- Includes to be reworked
- Patches that have been upstreamed to be dropped
- Using nix-unit as a flake since it doesn't yet support Nix 2.28.2
  includes
The ParsedURL url field was dropped in Nix commit
f705ce7f9a95bc37df236d88eef1dbd9e2ae5f31, which says to use to_string()
instead.
As of Nix commit beed00c04e136e8d685905e4b2b1116ecdf42f63, absPath takes
a string_view
InputPath was renamed to InputAttrPath in Nix commit
7dfff58292475d0aed3dc3e98236ba495c45f261
As of Nix commit 7b6622d73317f0cd093cde96b5898b8af07abea7, a NixInt is
no longer just a typedef so we have to use .value to extract an integer
We should only be using paths from nix.out, not nix.dev. Removing
nix.dev from propagatedBuildInputs reduces our closure size by ~60 MiB.
Before this change, on aarch64-linux:

```
$ nix path-info --size --closure-size --human-readable ./result
/nix/store/58yi21g394bzy5zdg0jzar7x1kg5cp9x-flox-1.4.3-g59e52f0	  82.8 KiB	 373.2 MiB
```

After:
```
$ nix path-info --size --closure-size --human-readable ./result
/nix/store/nwk9pqbxjb37hcwxjd3l5g18g2ig1lmj-flox-1.4.3-g3bb091d	  82.8 KiB	 315.9 MiB
```
I increased large-error-threshold for now to silence some lints, but we
can fix those if we want as followup
Add a newline in expected test output. The previous behavior was
incorrect and logged in NixOS/nix#11991 which
has since been fixed.
Updating our flake increases closure size on aarch64-linux from 294.8
MiB to 315.9 MiB.

The closure size of Nix from 2.24.12 to 2.28.2 went up from 108.4 to
131.1 MiB.

As this is a necessary dependency, increase closure limit size
accordingly.

It does look like Nix may have introduced some unnecessary dependencies,
but if we think those are worth chasing down we can do it in followup.
Bump testing nixpkgs URL to stay in sync with the nixpkgs we use to
build Flox.

If we don't do this, it will cause build tests to fail, as certain build
tests currently depend on the fact that patchShebang will patch using a
bash that is in the closure of flox-interpreter. If the nixpkgs used for
patchShebang is different than the one used to build flox-interpreter,
the build will fail with errors like:
```
❌ 1 packages found in /nix/store/sg8yy9rf2wi4jakxjdfq7ly220647wqh-mypkg-1.0.2a
   -      not found in /nix/store/4w3fvpfl2yjabdb536ij74zlp8hwwz1l-environment-build-mypkg
```
opt-level > 0 causes hyper to create bad requests because of a rustc
bug, so set opt-level to 0

I also left .connection_verbose(true) in our client builder as it was
helpful during debugging.
@mkenigs mkenigs requested a review from ysndr June 10, 2025 16:47
@mkenigs mkenigs added this pull request to the merge queue Jun 10, 2025
@mkenigs mkenigs changed the title Update flake and bump Nix to 2.28.2 chore: update flake and bump Nix to 2.28.2 Jun 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 10, 2025
@mkenigs mkenigs added this pull request to the merge queue Jun 10, 2025
Merged via the queue into main with commit b5adddf Jun 10, 2025
24 checks passed
@mkenigs mkenigs deleted the bump-flake branch June 10, 2025 22:38
ysndr added a commit that referenced this pull request Jul 18, 2025
Originally introduced in <#2789>
the patch was merged upstream into master
in February via <NixOS/nix#12580>.
Assuming it was available via Nix in nixpkgs in late June,
we removed the patch in <#3176>,
which bumped our nix distribution to v2.28.2.

It turns out that the upstream patch was only released via nix **2.29.0**,
thus by removing our patch we regressed that behavior.

This commit reintroduces the patch, while we use nix 2.28.3.
tomberek pushed a commit that referenced this pull request Jul 18, 2025
Originally introduced in <#2789>
the patch was merged upstream into master
in February via <NixOS/nix#12580>.
Assuming it was available via Nix in nixpkgs in late June,
we removed the patch in <#3176>,
which bumped our nix distribution to v2.28.2.

It turns out that the upstream patch was only released via nix **2.29.0**,
thus by removing our patch we regressed that behavior.

This commit reintroduces the patch, while we use nix 2.28.3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-cli Tickets relevant to the flox CLI team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants