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.

Sign up for GitHub

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

Jump to bottom

cargo-c: 0.9.29 -> 0.9.31 #299789

Merged
merged 1 commit into from
Apr 22, 2024
Merged

cargo-c: 0.9.29 -> 0.9.31 #299789

merged 1 commit into from
Apr 22, 2024

Conversation

cpu
Copy link
Contributor

@cpu cpu commented Mar 28, 2024 •
edited
Loading

Description of changes

This commit:

  • Updates cargo-c from v0.9.29 to v0.9.31.
  • Adds myself as a maintainer, since the prev. maintainer stepped down (fe29880) and I have an interest in keeping cargo-c usable for my work downstream in rustls-ffi.

Changelog:

lu-zero/cargo-c@v0.9.29...v0.9.31

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@cpu
cargo-c: 0.9.29 -> 0.9.31
dbd5d47 
This commit:

* Updates cargo-c from v0.9.29 to 0.9.31.
* Adds myself as a maintainer, since the prev. maintainer stepped down
  and I have an interest in keeping cargo-c usable.

Changelog:

  lu-zero/cargo-c@v0.9.29...v0.9.31
@cpu cpu mentioned this pull request Mar 28, 2024
Merged
iFreilicht
iFreilicht approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@iFreilicht iFreilicht left a comment

Choose a reason for hiding this comment

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

Builds fine on aarch64-darwin and correctly built rustls-ffi:

$ nix build .#cargo-c
$ readlink result
/nix/store/35nw5c5f0nhcc9mpigd2pwhxkra0ffzw-cargo-c-0.9.31
$ git clone --depth=1 [email protected]:rustls/rustls-ffi.git
loning into 'rustls-ffi'...
remote: Enumerating objects: 56, done.
remote: Counting objects: 100% (56/56), done.
remote: Compressing objects: 100% (54/54), done.
remote: Total 56 (delta 2), reused 29 (delta 0), pack-reused 0
Receiving objects: 100% (56/56), 116.09 KiB | 639.00 KiB/s, done.
Resolving deltas: 100% (2/2), done.
$ /nix/store/35nw5c5f0nhcc9mpigd2pwhxkra0ffzw-cargo-c-0.9.31/bin/cargo-cbuild cbuild
    Updating crates.io index
  Downloaded zeroize v1.7.0
  Downloaded rustls-pki-types v1.3.1
  Downloaded rustls-pemfile v2.1.1
  Downloaded getrandom v0.2.11
  Downloaded once_cell v1.19.0
  Downloaded log v0.4.21
  Downloaded rustls-webpki v0.102.2
  Downloaded libc v0.2.153
  Downloaded rustls v0.23.1
  Downloaded 9 crates (1.5 MB) in 0.79s
   Compiling libc v0.2.153
   Compiling cfg-if v1.0.0
   Compiling rustls-pki-types v1.3.1
   Compiling untrusted v0.9.0
   Compiling rustls v0.23.1
   Compiling once_cell v1.19.0
   Compiling zeroize v1.7.0
   Compiling rustls-ffi v0.13.0 (/Users/feuh/repos/rustls-ffi)
   Compiling base64 v0.21.5
   Compiling subtle v2.5.0
   Compiling log v0.4.21
   Compiling rustls-pemfile v2.1.1
   Compiling getrandom v0.2.11
   Compiling cc v1.0.83
   Compiling ring v0.17.5
   Compiling rustls-webpki v0.102.2
    Finished dev [unoptimized + debuginfo] target(s) in 12.34s
    Building pkg-config files
  Populating uninstalled header directory

Can't test much further than that, but for a regular version bump, this seems sufficient to me.

The 5001+ rebuilds aren't great, but not sure anything can be done about that.

@cpu
Copy link
Contributor Author

cpu commented Mar 28, 2024

Thanks for taking a look! Much appreciated.

The 5001+ rebuilds aren't great, but not sure anything can be done about that.

I found that very surprising given the scope of the update and the fact that I find so few references to cargo-c in-tree (just libimagequant, libdovi, and rav1e). I don't have any theories for why it's happening. Do you? 😆

@iFreilicht
Copy link
Contributor

I'm not sure, but it seems to me like these few dependencies are very deep inside the dependency tree. libimagequant is a dependency of pillow, for example, which is used in over 200 other packages AFAICT.
libdovi is a dependency of libplacebo, which is in turn a dependency of ffmpeg, which is used in over 300 packages.

I can confirm this as well, trying to run nixpkgs-review on this PR took forever and then started an enormous fetch of over 30GB:

$ nix build --nix-path nixpkgs=/Users/feuh/.cache/nixpkgs-review/pr-299789/nixpkgs nixpkgs-overlays=/tmp/tmp7wydbhe6 --extra-experimental-features nix-command no-url-literals --no-link --keep-going --no-allow-import-from-derivation -f /Users/feuh/.cache/nixpkgs-review/pr-299789/build.nix
[0/15192 built, 12/124/17689 copied (198.3/90017.3 MiB), 117.0/30596.4 MiB DL] fetching IdnaMappingTable.txt from[0/15192 built, 12/124/17689 copied (198.8/90017.3 MiB), 117.3/30596.4 MiB DL] fetching IdnaMappingTable.txt from
$ git worktree remove -f /Users/feuh/.cache/nixpkgs-review/pr-299789/nixpkgs

I definitely saw Flask in there at some point as well. So yeah, just something we have to live with, I think.

@cpu
Copy link
Contributor Author

cpu commented Mar 29, 2024

I'm not sure, but it seems to me like these few dependencies are very deep inside the dependency tree.

Oh of course. That's a very simple explanation.

So yeah, just something we have to live with, I think.

Agreed. Thanks for digging in.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 29, 2024
marsam
marsam reviewed Apr 10, 2024
View reviewed changes
pkgs/development/tools/rust/cargo-c/default.nix

src = fetchCrate {
inherit pname;
# this version may need to be updated along with package version
version = "${version}+cargo-0.76.0";
hash = "sha256-Uy5Bm8WwN3jQO2btnV/ayxTlIJAe5q2FUvhxCCrn9U8=";
version = "${version}+cargo-0.78.0";
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, does this mean that this release requires Cargo 0.78.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I don't believe that's the case, but I will return with a conclusive answer when I can find time to investigate further.

@marsam marsam changed the base branch from master to staging April 21, 2024 04:01
@marsam marsam merged commit b2232a9 into NixOS:staging Apr 22, 2024
35 checks passed
@cpu cpu deleted the cpu-cargo-c-0.9.31 branch April 22, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marsam marsam marsam left review comments

@iFreilicht iFreilicht iFreilicht approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cpu @iFreilicht @marsam @wegank