Update cargo builder to fetch registry dynamically. #27964

kevincox · 2017-08-05T14:53:13Z

Motivation for this change

See commit message

The biggest benifit is that we no longer have to update the registry
package. This means that just about any cargo package can be built by
nix. No longer does cargo update need to be feared because it will
update to packages newer then what is available in nixpkgs.

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Rename deps attribute

Fixes #14469
Fixes #23282

Mic92 · 2017-08-06T14:59:13Z

pkgs/build-support/rust/default.nix


  patchRegistryDeps = ./patch-registry-deps;

-  buildInputs = [ git rust.cargo rust.rustc ] ++ buildInputs;
+  buildInputs = [ git rust.cargo rust.rustc strace ] ++ buildInputs;


strace should be probably removed again.

Good catch, fixed.

I usually use sysdig for those cases as I don't need to modify the environment.

The biggest benefit is that we no longer have to update the registry package. This means that just about any cargo package can be built by nix. No longer does `cargo update` need to be feared because it will update to packages newer then what is available in nixpkgs. This works very similarly to the old technique except instead of using cargo's internal cache and stripping out as much as possible it uses the new custom registry support to generate a minimal registry with exactly what we need. Like this existing method this will download crates multiple times for different packages. This is a shame because the Cargo.lock hash hashes inside of it but Nix doesn't really support this. This also uses the new --frozen and --locked flags which is nice. Currently cargo-local-registry only provides binaries for Linux and macOS 64-bit. This can be solved by building it for the other architectures and uploading it somewhere (like the NixOS cache). This also has the downside that it requires a change to everyone's deps hash. And if the old one is used because it was cached it will fail to build as it will attempt to use the old version.

Mic92 · 2017-08-06T17:22:56Z

pkgs/build-support/rust/cargo-local-registry.nix

+let
+  version = "0.1.4";
+  platform =
+    if stdenv.system == "x86_64-linux"


We will also need aarch64. Maybe we can work-around getting cargo build in bootstrap phase without this package and build cargo-local-registry from source.

There are a lot of dependencies and other stuff to manage. I don't think I know enough about the rust setup to do it myself but if someone can take it on that would be great. I was focused here on showing that cargo-local-registry is probably the way forward.

However building might not actually be necessary if we can rely on the cached deps pacakge. This package is archetecture independant so once we have them we can just build cargo-local-registry from source. I don't know if there is a policy about relying on the cache for things like this.

Maybe we can use this one for bootstrapping: #24991
I have not found yet find to the time to inspect it closer.

The reason I prefer this approach is it doesn't require any preprocessing or extra files. It simply reads the Cargo.toml file. I do think that those are fixable but this is also a nice quick improvement on the current state.

But a solution for aarch64 must be found first. If upstream adds support for this architecture as well, I am ok with that.

Or we could just build our own boostrap binary 🤷‍. Once the bootstrap is in place we can just grab a couple of versions back.

@Mic92 As far as i can tell there is currently no support for rust bootstrap for aarch64 either? There are aarch64 builds available on the official rust installer page. Adding a binaries for aarch64 to upstream cargo-local-registry would be easy tough.

@kevincox I was working on something similar but based on cargo-vendor. Not sure yet which one is better suited but according to the authore (which is the same for both) cargo-vendor is getting more attention at the moment.

@kevincox I forget to mention that cargo-vendor might be easier to bootstrap soon as it will soon have a tarball with vendored dependencies available. See: alexcrichton/cargo-vendor#51

So I'm not an expert on the differences but it seems like cargo-local-registry is designed to "mirror" the required dependencies whereas cargo-vendor is supposed to the upstream project to vendor dependencies themselves. Either will probably work in practice but cargo-local-registry looks like a better fit for our use case.

And it seems like the bootstrapping issue shouldn't be a major concern as cargo can cross-compile easily. Someone just needs to but in some effort to bootstrap the first version.

That being said I'm happy for whatever solution works.

ckauhaus · 2017-08-11T11:07:19Z

Might solve #23282

kevincox · 2017-08-11T11:50:50Z

@ckauhaus It would, added to the description.

ckauhaus · 2017-08-11T12:25:17Z

In its current form, this PR breaks all existing Rust packages in an unobvious way. Due to the transition from registry.index to local-registry, the required format of the deps derivation has changed. This means we need new depsSha256 checksums everywhere.

I'd personally prefer to rename this attribute (e.g., cargoDepsSha256) so that old Rust builds print a clear error message.

zimbatm · 2017-10-09T18:17:20Z

Merged this into #30088

kevincox · 2017-10-12T19:42:19Z

Thanks @zimbatm. I'll follow that change. For now I'll leave this open but it can basically be considered dead and I'll close it one #30088 gets merged.

gleber · 2017-11-18T12:25:15Z

@kevincox #30088 has been merged, please close this PR.

kevincox added 0.kind: enhancement 6.topic: developer experience 6.topic: rust 6.topic: user experience labels Aug 5, 2017

kevincox force-pushed the cargo-live branch from 1278a2f to 8c5014d Compare August 5, 2017 15:13

Mic92 reviewed Aug 6, 2017

View reviewed changes

kevincox force-pushed the cargo-live branch from 8c5014d to 5be1f32 Compare August 6, 2017 15:44

Mic92 reviewed Aug 6, 2017

View reviewed changes

bachp mentioned this pull request Oct 9, 2017

Rust 1.20.0 #30088

Merged

17 tasks

kevincox closed this Nov 18, 2017

kevincox deleted the cargo-live branch September 20, 2023 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update cargo builder to fetch registry dynamically. #27964

Update cargo builder to fetch registry dynamically. #27964

kevincox commented Aug 5, 2017 •

edited

Mic92 Aug 6, 2017

kevincox Aug 6, 2017

Mic92 Aug 6, 2017

Mic92 Aug 6, 2017

kevincox Aug 6, 2017

Mic92 Aug 10, 2017 •

edited

kevincox Aug 10, 2017

Mic92 Aug 11, 2017 •

edited

kevincox Aug 11, 2017

bachp Oct 3, 2017

bachp Oct 3, 2017

kevincox Oct 3, 2017

ckauhaus commented Aug 11, 2017

kevincox commented Aug 11, 2017

ckauhaus commented Aug 11, 2017

zimbatm commented Oct 9, 2017 •

edited

kevincox commented Oct 12, 2017

gleber commented Nov 18, 2017

Update cargo builder to fetch registry dynamically. #27964

Update cargo builder to fetch registry dynamically. #27964

Conversation

kevincox commented Aug 5, 2017 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Aug 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Aug 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckauhaus commented Aug 11, 2017

kevincox commented Aug 11, 2017

ckauhaus commented Aug 11, 2017

zimbatm commented Oct 9, 2017 • edited

kevincox commented Oct 12, 2017

gleber commented Nov 18, 2017

kevincox commented Aug 5, 2017 •

edited

Mic92 Aug 10, 2017 •

edited

Mic92 Aug 11, 2017 •

edited

zimbatm commented Oct 9, 2017 •

edited