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

[staging-next] rustPlatform.fetchCargoTarball: deprecate #378288

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

alyssais
Copy link
Member

This needs to land at the same time as Cargo 1.84.0 to make sure nobody ends up making bad FODs. Draft for now because there's still at least one remaining user in Nixpkgs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@alyssais
Copy link
Member Author

@infinisil do you know why eval is not failing here? I know of at least one attribute, ja2-stracciatella, that produces a warning with this change on current staging-next, but CI is nevertheless happy. OfBorg used to fail if any packages produced an evaluation warning I think. (ja2-stracciatella will no longer be an example of a warning next time master is merged into staging-next btw.)

@infinisil
Copy link
Member

@alyssais Known problem, didn't get around to it yet: #371223

@alyssais
Copy link
Member Author

Oh no, I was really counting on this to make sure I'd taken care of all the users in Nixpkgs. I guess I can run the old OfBorg eval checks locally if I have lots of RAM…

@TomaSajt
Copy link
Contributor

If throw still works in CI:

we could add an opt-in flag temporarily:
if set, do nothing; else, throw

And opt in at all currently known remaining usages of fetchCargoTarball

@alyssais
Copy link
Member Author

I can run the checks locally, but it'll be at the weekend.

@TomaSajt
Copy link
Contributor

TomaSajt commented Feb 1, 2025

Since people don't often use fetchCargoTarball by itself, maybe buildRustPackage's boolean flag should be mentioned in the warning.

Or maybe just add another warning into buildRustPackage when the boolean flag is set to false.

@alyssais
Copy link
Member Author

alyssais commented Feb 1, 2025

The more information we put in the warning, the less likely people are to read the full explanation in the release notes, but I suppose it's worth it for buildRustPackage since it's so common.

@alyssais
Copy link
Member Author

alyssais commented Feb 1, 2025

Although then it would be weird not to mention how to transition if you are using fetchCargoTarball, so the warning message would get very long…

@alyssais
Copy link
Member Author

alyssais commented Feb 2, 2025

I've put more information in the deprecation warning. Still running the eval checks.

@alyssais alyssais mentioned this pull request Feb 3, 2025
11 tasks
@alyssais alyssais marked this pull request as ready for review February 3, 2025 17:13
@nix-owners nix-owners bot requested review from winterqt, figsoda and zowoq February 3, 2025 17:14
doronbehar added a commit to doronbehar/nixpkgs that referenced this pull request Feb 3, 2025
@alyssais
Copy link
Member Author

alyssais commented Feb 3, 2025

I've verified using the old OfBorg eval checks that there are no warnings on staging-next, so it's time! Since CI can't catch it, we'll probably see new PRs introducing uses of fetchCargoTarball that will warn for some time to come, sadly.

Still happy to take feedback on wording.

@alyssais alyssais force-pushed the fetchCargoTarball-deprecate branch from e4cdc4a to eda3147 Compare February 3, 2025 18:58
@alyssais
Copy link
Member Author

alyssais commented Feb 3, 2025

(I did expand the warning to explain inline what to do when using buildRustPackage)

@alyssais alyssais merged commit 9bc77ee into NixOS:staging-next Feb 3, 2025
22 of 25 checks passed
@alyssais alyssais deleted the fetchCargoTarball-deprecate branch February 3, 2025 19:07
@TomaSajt
Copy link
Contributor

TomaSajt commented Feb 3, 2025

What do you think the deprecation process will look like?

Perhaps something like this:

  • after now until 25.05: have this warning
  • after 25.05 until 25.11: make the useFetchCargoVendor flag a NOOP with warning and always use fetchCargoVendor
  • after 25.11: finally remove useFetchCargoVendor flag

jtacoma pushed a commit to jtacoma/nixpkgs that referenced this pull request Feb 3, 2025
@PerchunPak
Copy link
Member

@alyssais as far as I understand, using buildRustPackage without useFetchCargoVendor is deprecated?

This throws a warning for some reason
https://github.com/catppuccin/nix/blob/7f2e0e709ad3e47a2ae0e735168438144c134947/pkgs/whiskers/package.nix

@K900
Copy link
Contributor

K900 commented Feb 8, 2025

Correct.

PerchunPak added a commit to PerchunPak/catppuccin-nix that referenced this pull request Feb 8, 2025
Using `buildRustPackage` without it is deprecated
NixOS/nixpkgs#378288 (comment)
isabelroses pushed a commit to catppuccin/nix that referenced this pull request Feb 9, 2025
fix(whiskers): enable `useFetchCargoVendor`

Using `buildRustPackage` without it is deprecated
NixOS/nixpkgs#378288 (comment)
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.

5 participants