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.

Sign up for GitHub

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

Jump to bottom

Migrate patches to overlays #1512

Closed
3 tasks done
aaronmondal opened this issue Dec 3, 2024 · 0 comments · Fixed by #1516
Closed
3 tasks done

Migrate patches to overlays #1512

aaronmondal opened this issue Dec 3, 2024 · 0 comments · Fixed by #1516
Assignees
@aaronmondal
Labels
bug Something isn't working

Comments

@aaronmondal
Copy link
Member

aaronmondal commented Dec 3, 2024 •
edited
Loading

Patching nixpkgs the way we currently do causes IFDs, meaning that we need to realize nixpkgs before we can access store paths. When we now want to access packages from other target systems our evaluation breaks since we can't realize packages for non-native systems.

This has multiple implications:

  • nix flake show is broken since we can't evaluate the cross-compiled variants of nativelink (Broken nix flake on aarch64-darwin #1262)
  • Evaluations likely take much longer than they should since they need to evaluate nixpkgs twice. This might contribute to the slowness observed in Optimize dynamic initialization for LRE workers #1485.
  • If the above holds, it also likely means that we waste quite a bit of resources in most of our nix-driven CI workflows.
  • It's currently impossible to implement sensible crosscompilation toolchains since we can't evaluate store paths for other systems. This prevents us from implementing cross-compilation LRE toolchains and makes it much harder to implement multi-arch images for nativelink.

The naive solution to this issue would be to run a custom fork of nixpkgs with our patches applied ahead of time. This isn't a very maintainable solution though as it would encourage drift from upstream and complicate the setups for projects importing the native-cli or LRE.

The more idomatic solution is to migrate the patches to actual overlays so that we can remove the patch logic entirely. This also makes the overlays more portable and robust to future nixpkgs version bumps.

The TODOs for this issue are the migration of the following patches:

Upstream is aware of these issues and improvments to the UX for naive patch application is in progress at NixOS/nix#6530. However, at the moment we can't predict how long it'll take until we can reasonably use it.
@aaronmondal aaronmondal added the bug Something isn't working label Dec 3, 2024
@aaronmondal aaronmondal self-assigned this Dec 3, 2024
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Migrate pulumi patches to an overlay
c099d2b 
This is the first of three patches that need to be moved away from our
naive patch approach to a proper overlay so that we avoid IFDs during
crosscompilation.

Towards TraceMachina#1512
@aaronmondal aaronmondal mentioned this issue Dec 3, 2024
Merged
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Migrate rust-overlay patch to an overlay
4c6d33a 
Towards TraceMachina#1512
@aaronmondal aaronmondal mentioned this issue Dec 3, 2024
Merged
aaronmondal added a commit that referenced this issue Dec 3, 2024
@aaronmondal
Migrate pulumi patches to an overlay (#1513)
b25fbd1 
This is the first of three patches that need to be moved away from our
naive patch approach to a proper overlay so that we avoid IFDs during
crosscompilation.

Towards #1512
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Migrate rust-overlay patch to an overlay
8be54c5 
Towards TraceMachina#1512
aaronmondal added a commit that referenced this issue Dec 3, 2024
@aaronmondal
Migrate rust-overlay patch to an overlay (#1514)
301e51b 
Towards #1512
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Migrate rust-overlay patch to an overlay
d9c9b66 
Towards TraceMachina#1512
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Introduce the LRE flake overlay
20e8a2e 
This change migrates the last patch to an overlay and removes the patch
structure. This resolves all IFDs, cleans up the main flake and makes
our custom toolchains portable across flakes.

The `customClang` and `customStdenv` packages are now available as
`lre.clang` and `lre.stdenv`.

While the new packages are differently named, they produce the exact
same setup as previously. This can be verified by the fact that
`generate-toolchains` is a noop between the previous commit and this
one.

Since a renaming of the `customClang` executable itself would cause
differences in the lre-cc toolchain we defer this to a future commit.

Fixes TraceMachina#1262
Closes TraceMachina#1512
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Introduce the LRE flake overlay
74a2ba9 
This change migrates the last patch to an overlay and removes the patch
structure. This resolves all IFDs, cleans up the main flake and makes
our custom toolchains portable across flakes.

The `customClang` and `customStdenv` packages are now available as
`lre.clang` and `lre.stdenv`.

While the new packages are differently named, they produce the exact
same setup as previously. This can be verified by the fact that
`generate-toolchains` is a noop between the previous commit and this
one.

Since a renaming of the `customClang` executable itself would cause
differences in the lre-cc toolchain we defer this to a future commit.

Fixes TraceMachina#1262
Closes TraceMachina#1512
@aaronmondal aaronmondal mentioned this issue Dec 3, 2024
Merged
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Dec 3, 2024
@aaronmondal
Introduce the LRE flake overlay
99f93cb 
This change migrates the last patch to an overlay and removes the patch
structure. This resolves all IFDs, cleans up the main flake and makes
our custom toolchains portable across flakes.

The `customClang` and `customStdenv` packages are now available as
`lre.clang` and `lre.stdenv`.

While the new packages are differently named, they produce the exact
same setup as previously. This can be verified by the fact that
`generate-toolchains` is a noop between the previous commit and this
one.

Since a renaming of the `customClang` executable itself would cause
differences in the lre-cc toolchain we defer this to a future commit.

Fixes TraceMachina#1262
Closes TraceMachina#1512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aaronmondal aaronmondal

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce the LRE flake overlay aaronmondal/nativelink
1 participant
@aaronmondal