Skip to content

gitoxide: fix build issues with cargo-auditable #253911#254087

Closed
jalil-salame wants to merge 1 commit intoNixOS:masterfrom
jalil-salame:fix-gitoxide
Closed

gitoxide: fix build issues with cargo-auditable #253911#254087
jalil-salame wants to merge 1 commit intoNixOS:masterfrom
jalil-salame:fix-gitoxide

Conversation

@jalil-salame
Copy link
Copy Markdown
Contributor

@jalil-salame jalil-salame commented Sep 8, 2023 •
edited
Loading

Description of changes

fixes #253911

cargo-auditable requires some metadata which is private in gitoxide, for further details look at the linked issues in #253911.

This uses a patch to the Cargo.toml to fix the build errors. It is a simple rename and doesn't affect any kind of functionality AFAIK.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.11 Release Notes (or backporting 23.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.

@ofborg ofborg Bot requested a review from syberant September 8, 2023 20:28
@ofborg ofborg Bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Sep 8, 2023
@VuiMuich
Copy link
Copy Markdown
Contributor

VuiMuich commented Sep 9, 2023 •
edited
Loading

I manually added the fix-cargo-auditable.patch to my flake based home-manager setup. Would this be sort of count as a "confirmation of the fix"?

Edit: update with nix-info

$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.4.10, NixOS, 23.11 (Tapir), 23.11.20230906.0bffda1`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.17.0`
 - channels(root): `"home-manager-23.05.tar.gz, nixos-23.05"`
 - channels(vuimuich): `""`
 - nixpkgs: `/nix/store/vm2xa6ksffj4q96wikraqsrd14j6lhxy-source`

@jalil-salame
Copy link
Copy Markdown
Contributor Author

jalil-salame commented Sep 9, 2023

I manually added the fix-cargo-auditable.patch to my flake based home-manager setup. Would this be sort of count as a "confirmation of the fix"?

Should count, you might also want to run nix-shell -p nix-info --run "nix-info -m" and update your comment with the result, especially if you are not on Linux.

jalil-salame added a commit to jalil-salame/NixOsConfig that referenced this pull request Sep 13, 2023
@jalil-salame
fix(nixpkgs): Apply gitoxide path
7b2b3ab 
Waiting for NixOS/nixpkgs#254087 to be merged
jalil-salame added a commit to jalil-salame/NixOsConfig that referenced this pull request Sep 13, 2023
@jalil-salame
fix(nixpkgs): Apply gitoxide path
3ccd3e6 
Waiting for NixOS/nixpkgs#254087 to be merged
minijackson
minijackson approved these changes Sep 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@minijackson minijackson left a comment

Choose a reason for hiding this comment

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

Works great! Is there any plan to upstream that patch?

Result of nixpkgs-review pr 254087 run on x86_64-linux 1

1 package built:
  • gitoxide

@jalil-salame
Copy link
Copy Markdown
Contributor Author

jalil-salame commented Sep 13, 2023

Works great! Is there any plan to upstream that patch?

I'll notify the maintainer from gitoxide, but they have filed it as a cargo bug.

Kinda strapped for time too, if you have time feel free to make a PR yourself, consider this my approval to upstream it yourself.

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 13, 2023
@jalil-salame jalil-salame mentioned this pull request Sep 13, 2023
Closed
@jalil-salame
Copy link
Copy Markdown
Contributor Author

jalil-salame commented Sep 14, 2023

This might be fixed upstream instead GitoxideLabs/gitoxide#1024

@minijackson
Copy link
Copy Markdown
Member

minijackson commented Sep 14, 2023

Awesome! Can we fetchpatch from the PR?

@jalil-salame
Copy link
Copy Markdown
Contributor Author

jalil-salame commented Sep 14, 2023

Should work, I'll do it as soon as possible (I'll try tonight but it might take another day or two for me to find time)

@jalil-salame
gitoxide: fix build issues with cargo-auditable NixOS#253911
63c21b3 
`cargo-auditable` requires some metadata which is private in `gitoxide`,
for further details look at the linked issues in NixOS#253911.

This uses a patch to the `Cargo.toml` to fix the build errors. It is a
simple rename and doesn't affect any kind of functionality AFAIK.
@jalil-salame
Copy link
Copy Markdown
Contributor Author

jalil-salame commented Sep 14, 2023

Patch was upstreamed, rewrote with fetchpatch, should not be needed if a new version of gitoxide comes up.

minijackson
minijackson approved these changes Sep 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@minijackson minijackson left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 254087 run on x86_64-linux 1

1 package built:
  • gitoxide

@jalil-salame jalil-salame mentioned this pull request Sep 16, 2023
Merged
@davidkna davidkna mentioned this pull request Sep 25, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@minijackson minijackson minijackson approved these changes

@syberant syberant Awaiting requested review from syberant

Assignees

No one assigned

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Build failure: gitoxide 0.29.0

4 participants

@jalil-salame @VuiMuich @minijackson @delroth