Skip to content

nix: migrate off deprecated crane stdenv arg to stdenvSelector#2379

Open
amaanq wants to merge 3 commits into
TraceMachina:mainfrom
amaanq:crane-stdenvselector
Open

nix: migrate off deprecated crane stdenv arg to stdenvSelector#2379
amaanq wants to merge 3 commits into
TraceMachina:mainfrom
amaanq:crane-stdenvselector

Conversation

@amaanq
Copy link
Copy Markdown

@amaanq amaanq commented May 29, 2026

Description

Crane deprecated passing stdenv into mkCargoDerivation, however, since the input of crane here is a bit behind, no warning is emitted yet. I use Nativelink between my own machines, but I set the crane input here to follow a more up-to-date crane, and as such, I get hit with the following warning:

evaluation warning: passing in an `stdenv` override into `mkCargoDerivation` is now deprecated.
                    Changing the default `stdenv` must be done at the `craneLib` level like so:

                      craneLib = (inputs.crane.mkLib pkgs).overrideScope (final: prev: {
                        # Set this to the chosen stdenv, e.g. `p.clangStdenv`
                        stdenvSelector = p: p.stdenv;

This PR's change is defensive in nature, but I went ahead and updated the crane input as well. If that's undesireable, please let me know and I can revert that, the fix works without bumping crane.

Thanks for the really cool project :)

Fixes # (issue)

(No issue, just something I ran into)

Type of change

A defensive future-bug fix.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I ran this locally, overriding my input with the local path.

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

@amaanq is attempting to deploy a commit to the native-link-web-assets Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 29, 2026

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nativelink Ready Ready Preview, Comment May 30, 2026 2:09am
nativelink-aidm Ready Ready Preview, Comment May 30, 2026 2:09am

Request Review

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

Very interesting pull request @amaanq! Thank you for the fix.

@amaanq
Copy link
Copy Markdown
Author

amaanq commented May 29, 2026

Seems like the failing mac job may just be flaky

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

@amaanq This is a weird failure.

@amaanq
Copy link
Copy Markdown
Author

amaanq commented May 30, 2026

@amaanq This is a weird failure.

Yeah it seems like a transient network failure from a glance, maybe re-running that job might fix it?

Comment thread flake.nix
}: let
craneLibFor = p: (crane.mkLib p).overrideToolchain pkgs.lre.stableRustFor;
nightlyCraneLibFor = p: (crane.mkLib p).overrideToolchain pkgs.lre.nightlyRustFor;
# On Linux we build fully static musl binaries, elsewhere the default stdenv.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like this but I'd like to explore giving people the option to run glibc at times when they need it. Musl is good default, though, for obvious reasons.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure, that sounds reasonable. I could do that in a follow-up pr

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

I am going to cut a release with this fix soon.

@MarcusSorealheis
Copy link
Copy Markdown
Collaborator

@amaanq LGTM

@amaanq
Copy link
Copy Markdown
Author

amaanq commented May 30, 2026

I am going to cut a release with this fix soon.

awesome!

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