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

Various fetchcargo improvements #74863

Merged
merged 3 commits into from Dec 23, 2019
Merged

Various fetchcargo improvements #74863

merged 3 commits into from Dec 23, 2019

Conversation

@alyssais
Copy link
Member

alyssais commented Dec 2, 2019

Motivation for this change
  • Most stdenv wrappers already forward unused arguments -- it allows greater customisation. We just have to be careful to remove arguments we're using that shouldn't be passed to stdenv. I've been conservative here, because fetchcargo checksums shouldn't change lightly.

  • If a custom unpackPhase is used for the package, it needs to also be used for fetchcargo so the same source is available for vendoring.

  • I exposed fetchcargo as rustPlatform.fetchcargo, because it might be useful to provide from another derivation.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

alyssais added 3 commits Dec 2, 2019
Most stdenv wrappers already work like this -- it allows greater
customisation.  We just have to be careful to remove arguments we're
using that shouldn't be passed to stdenv.  I've been conservative
here, because fetchcargo checksums shouldn't change lightly.
If a custom unpackPhase is used for the package, it needs to also be
used for fetchcargo so the same source is available for vendoring.
@alyssais alyssais requested review from LnL7 and Mic92 as code owners Dec 2, 2019
@alyssais alyssais changed the title Fetchcargo Various fetchcargo improvements Dec 2, 2019
@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Dec 13, 2019

Could you provide an example, how do you use this feature?

@alyssais

This comment has been minimized.

Copy link
Member Author

alyssais commented Dec 15, 2019

inherit cargo;
};
buildRustPackage = callPackage ../../../build-support/rust {
inherit rustc cargo fetchcargo;

This comment has been minimized.

Copy link
@7c6f434c

7c6f434c Dec 15, 2019

Member

How this passed fetchcargo get used?

This comment has been minimized.

Copy link
@alyssais

alyssais Dec 16, 2019

Author Member

Not sure I understand? It’s used here:

This comment has been minimized.

Copy link
@7c6f434c

7c6f434c Dec 17, 2019

Member

Oh my bad, I looked at the changes, that you add fetchcargo argument but do not add any uses of it. Now I see that the point is to pass the local version referencing correct cargo.

@alyssais alyssais mentioned this pull request Dec 16, 2019
4 of 10 tasks complete
@alyssais alyssais merged commit b9d274b into NixOS:master Dec 23, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@alyssais alyssais deleted the alyssais:fetchcargo branch Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.