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

fetchCargoTarball: support vendoring extra manifests #104281

Conversation

steveej
Copy link
Contributor

@steveej steveej commented Nov 19, 2020

Motivation for this change

This is helpful in a situation where not all crates in the given
repository are listed in the toplevel Cargo.toml but are required during
the build-process.

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 nixpkgs-review --run "nixpkgs-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.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Can you add some documentation for this? I assume that https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md would be the right place but I am not 100% sure.

This is helpful in a situation where not all crates in the given
repository are listed in the toplevel Cargo.toml but are required during
the build-process.
@steveej steveej force-pushed the pr/fetchcargotarball-extra-manifests branch from 1c2f8f5 to 162971c Compare November 19, 2020 18:55
@steveej steveej marked this pull request as draft November 19, 2020 22:23
@steveej
Copy link
Contributor Author

steveej commented Nov 19, 2020

My use-case for this has ceased. It might still make sense to merge this since it's a feature offered by cargo vendor and we may want to expose it our API.

@steveej steveej marked this pull request as ready for review November 19, 2020 22:25
@stale
Copy link

stale bot commented Jul 19, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 19, 2021
@andir
Copy link
Member

andir commented Jul 19, 2021

Please add a test case so that we can ensure your feature continues working whenever we refactor the code.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 19, 2021
@stale
Copy link

stale bot commented Apr 19, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 19, 2022
@steveej
Copy link
Contributor Author

steveej commented Aug 25, 2022

i haven't been using this myself so i won't pursue this PR any longer

@steveej steveej closed this Aug 25, 2022
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

3 participants