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

buildRustPackage: preserve output from cargo-vendor #41519

Closed
wants to merge 1 commit into from

Conversation

jbboehr
Copy link
Contributor

@jbboehr jbboehr commented Jun 6, 2018

Motivation for this change

Dependencies specified as git repos are not working due to missing output in .cargo/config. This patch saves the output in fetchcargo and includes parts of it in the one regenerated in buildRustPackage. Note that the temporary cargo-config file must be removed prior to executing cargo update.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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.

I've setup a repository to demonstrate the working configuration: https://github.com/jbboehr/nix-fetch-cargo-update

I believe this implementation is a bit brittle. A "perfect" implementation would probably use a toml parser to remove or replace the relevant sections in .cargo/config.

I tried running nix-shell -p nox --run "nox-review wip" but it crashed, maybe it affects too many packages?

Fixes #41518

Dependencies specified as git repos are not working
due to missing output in .cargo/config. This patch
saves the output in fetchcargo and includes parts
of it in the one regenerated in buildRustPackage.
Note that the temporary cargo-config file must
be removed prior to executing cargo update.
@jbboehr
Copy link
Contributor Author

jbboehr commented Jun 6, 2018

Pretty sure this isn't ready for merging yet, review and feedback would be appreciated!

@jtojnar
Copy link
Contributor

jtojnar commented Jun 6, 2018

See #34034

@jbboehr
Copy link
Contributor Author

jbboehr commented Jun 6, 2018

Whoops, guess I didn't search hard enough for a previous issue.

@jbboehr
Copy link
Contributor Author

jbboehr commented Jun 6, 2018

I'll close this in favor of #34034

@jbboehr jbboehr closed this Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants