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

treewide: migrate to the new rust fetcher #356385

Merged
merged 7 commits into from
Nov 19, 2024

Conversation

JohnRTitor
Copy link
Contributor

@JohnRTitor JohnRTitor commented Nov 16, 2024

Followup of #349360

CC: #327063

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.

@JohnRTitor

This comment was marked as resolved.

@JohnRTitor

This comment was marked as resolved.

@TomaSajt

This comment was marked as resolved.

@TomaSajt

This comment was marked as resolved.

@JohnRTitor
Copy link
Contributor Author

@ofborg build alfis workstyle mullvad chiptrack cloud-hypervisor conduwuit

@TomaSajt
Copy link
Contributor

@ofborg build cloud-hypervisor

@JohnRTitor
Copy link
Contributor Author

@ofborg build cloud-hypervisor

Looks like a hash mismatch for cargoHash, could you take care of it for me and push? Not near a computer right now.

@TomaSajt
Copy link
Contributor

TomaSajt commented Nov 16, 2024

Consistently gets sha256-7wIt4Ul65i0rqwGdsCJNicaU8Vkv/3KbLohc/ybXiEs= locally and sha256-xqMUB9aqkUIpnX0U30CfiWmjDI7IS5SuJIKF5byXIxk= on ofborg...
very weird...
will try to debug somehow

@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 16, 2024

Let's hold any mass migrations until this is resolved.

Very weird, only some packages are affected, in others the hash is rock solid.

Is it because of fetch-submodules?

@TomaSajt
Copy link
Contributor

@ofborg build cloud-hypervisor

@nix-owners nix-owners bot requested a review from figsoda November 16, 2024 20:11
@TomaSajt
Copy link
Contributor

I somehow reproduced the other hash locally, will do some testing to see what triggered it.

And yes, it seems like it is because of submodules.
The earlier hash did not fetch them, while the later one did.

@TomaSajt
Copy link
Contributor

Update: it is just that, submodules. Nothing fancy.

It just turns out I just wasn't rebased on the latest master when locally reproducing the earlier hash...
I feel a bit stupid now.
I don't know why I didn't re-check on your branch...

In any case I'll just update the hash, then

@TomaSajt TomaSajt force-pushed the rust-migrate-to-new-fetcher branch from 1eba5f2 to 1317df3 Compare November 16, 2024 20:51
@TomaSajt
Copy link
Contributor

TomaSajt commented Nov 16, 2024

By the way, did some searching and it looks like cargo's own git fetcher respects a setting that would make certain git deps not fetch their submodules:
rust-lang/cargo#10717

I don't think we should bother to respect that. The more files we fetch, the less likely it is that the current implementation can't handle some edge case.

@niklaskorz
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 356385


aarch64-darwin

⏩ 3 packages marked as broken and skipped:
  • alfis
  • chiptrack
  • mullvad
✅ 3 packages built:
  • alfis-nogui
  • conduwuit
  • workstyle

@niklaskorz
Copy link
Contributor

Not sure if you intend to add more packages and which, but for e.g. zed-editor I migrated to the new fetcher in #356757 and it works fine.

@TomaSajt
Copy link
Contributor

There are still some kinks to iron out, but the submodule stuff should have been the last change that could actually change the hash.

Just opened #356821 which decreases the concurrency to some more sane values.

@JohnRTitor JohnRTitor force-pushed the rust-migrate-to-new-fetcher branch from 1317df3 to 21dc39e Compare November 18, 2024 15:47
@JohnRTitor
Copy link
Contributor Author

@ofborg build alfis workstyle mullvad chiptrack cloud-hypervisor conduwuit xdg-desktop-portal-cosmic

@JohnRTitor
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 356385


x86_64-linux

❌ 4 packages failed to build:
  • cloud-hypervisor
  • cloud-hypervisor.debug
  • conduwuit
  • mullvad
✅ 7 packages built:
  • alfis
  • alfis-nogui
  • chiptrack
  • cosmic-session
  • workstyle
  • xdg-desktop-portal-cosmic
  • xdg-desktop-portal-cosmic.debug

@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 18, 2024

Not sure if you intend to add more packages and which

I do not intend to migrate all packages with a Cargo.lock in nixpkgs in this PR, in fact I think it's just more rebuild work, and the packages should be either mass migrated, or migrated when updating.

The packages listed here were decided randomly, and my goal was to find and iron out the bugs before the mass migration and adoption by other packages.

Mass migration PR: #356862

@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 18, 2024

@TomaSajt when running nixpkgs-review. https://termbin.com/o9z9

┃        > The above exception was the direct cause of the following exception:
┃        >
┃        > Traceback (most recent call last):
┃        >   File "/nix/store/rhm7zn3pxrm6805v5fkbvyrxa53yf8fb-fetch-cargo-vendor-util/bin/fetch-cargo-vendor-util", line 283, in <module>
┃        >     main()
┃        >   File "/nix/store/rhm7zn3pxrm6805v5fkbvyrxa53yf8fb-fetch-cargo-vendor-util/bin/fetch-cargo-vendor-util", line 279, in main
┃        >     subcommand_func()
┃        >   File "/nix/store/rhm7zn3pxrm6805v5fkbvyrxa53yf8fb-fetch-cargo-vendor-util/bin/fetch-cargo-vendor-util", line 270, in <lambda>
┃        >     "create-vendor-staging": lambda: create_vendor_staging(lockfile_path=Path(sys.argv[2]), out_dir=Path(sys.argv[3])),
┃        >                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
┃        >   File "/nix/store/rhm7zn3pxrm6805v5fkbvyrxa53yf8fb-fetch-cargo-vendor-util/bin/fetch-cargo-vendor-util", line 133, in create_vendor_staging
┃        >     pool.starmap(download_tarball, tarball_args_gen)
┃        >   File "/nix/store/px2nj16i5gc3d4mnw5l1nclfdxhry61p-python3-3.12.7/lib/python3.12/multiprocessing/pool.py", line 375, in starmap
┃        >     return self._map_async(func, iterable, starmapstar, chunksize).get()
┃        >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
┃        >   File "/nix/store/px2nj16i5gc3d4mnw5l1nclfdxhry61p-python3-3.12.7/lib/python3.12/multiprocessing/pool.py", line 774, in get
┃        >     raise self._value
┃        > requests.exceptions.ConnectionError: None: Max retries exceeded with url: /crates/rand_chacha/rand_chacha-0.3.1.crate (Caused by None)
┃        For full logs, run 'nix log /nix/store/vxnqpkg5h0x1sq63lp0x4iy0m5gn8pkd-cloud-hypervisor-42.0-vendor-staging.drv'.

@JohnRTitor JohnRTitor mentioned this pull request Nov 18, 2024
14 tasks
@JohnRTitor JohnRTitor force-pushed the rust-migrate-to-new-fetcher branch from 21dc39e to eee0a2c Compare November 18, 2024 16:19
@JohnRTitor
Copy link
Contributor Author

There was a hash mismatch with cloud-hypervisor. Fixed.
@ofborg build alfis workstyle mullvad chiptrack cloud-hypervisor conduwuit xdg-desktop-portal-cosmic

@TomaSajt
Copy link
Contributor

There was a hash mismatch with cloud-hypervisor. Fixed. @ofborg build alfis workstyle mullvad chiptrack cloud-hypervisor conduwuit xdg-desktop-portal-cosmic

I guess you forgot to reset your branch to the upstream one, so you accidentally overrode my fixed hash with the outdated one when rebasing.
But it should be okay now.

@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 19, 2024

@TomaSajt hypervisor and mullvad is failing to be built in nixpkgs-review (ran after garbage collecting) due to maximum no of retries hit (not because of a hash error)

https://termbin.com/3mz8

While Hydra and Ofborg may not necessarily hit the limit (due to low amount of cores and threads per job), running mass rebuilds in nixpkgs-review will be a pain with this

image

@TomaSajt
Copy link
Contributor

You could add a temporary commit on top of the PR that increases the retry count, like shown before.
And then use nixpkgs-review rev HEAD

@ofborg ofborg bot requested a review from nyabinary November 19, 2024 06:06
@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 19, 2024

Gonna merge this, since eval passed along with a nixpkgs-review

The above retry issue will be solved by #357262

@JohnRTitor JohnRTitor merged commit a4d7115 into NixOS:master Nov 19, 2024
30 of 32 checks passed
@JohnRTitor JohnRTitor deleted the rust-migrate-to-new-fetcher branch November 19, 2024 15:53
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.

3 participants