Skip to content

Comments

[staging-next] gst_all_1.gst-plugins-rs: add temporary fix for cargo-c test issue#239681

Merged
vcunat merged 1 commit intoNixOS:staging-nextfrom
lilyinstarlight:fix/cargo-c-gst-plugins-rs-hell
Jun 26, 2023
Merged

[staging-next] gst_all_1.gst-plugins-rs: add temporary fix for cargo-c test issue#239681
vcunat merged 1 commit intoNixOS:staging-nextfrom
lilyinstarlight:fix/cargo-c-gst-plugins-rs-hell

Conversation

@lilyinstarlight
Copy link
Member

Description of changes

Yay, gst-plugins-rs broke for two staging cycles in a row now...

Tests fail to run due to upstream cargo-c issue after the bump in #236611, so this makes a temporary fix to allow it to still build for now while upstream comes up with a more permanent solution I suppose

Given this new behavior for cargo-c seems to be intentional and gst-plugins-rs is not testing with this cargo-c version yet upstream (and would require non-negligible amounts of reworking for this problem), I opted to just inline a patched cargo-c instead of trying to untangle the cargo_wrapper mess in gst-plugins-rs enough to fix it there (Meson cargo workspace support when...)

Related: lu-zero/cargo-c#323 lu-zero/cargo-c#138

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.

Copy link
Contributor

@tjni tjni left a comment

Choose a reason for hiding this comment

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

Thank you so much for tracking this down!

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 25, 2023
Copy link
Member

Choose a reason for hiding this comment

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

Patching Inline like this has a chance to miss other usages of the package which wouldn't be ideal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Like other usages within nixpkgs or other usages within the same derivation?

If you have a suggestion, I don't particularly mind changing how this is done if the result is the same

Copy link
Member

Choose a reason for hiding this comment

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

other usages within the same derivation?

that one mostly.

I think this could either be moved into a let in where you probably want to rename it to cargo-c' or in callPackage. I think we would want to do the first here.

Copy link
Member Author

@lilyinstarlight lilyinstarlight Jun 26, 2023

Choose a reason for hiding this comment

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

I pushed an update to move cargo-c' into the let ... in in the derivation (derivation hash didn't change, so it should be functionally the same)

@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 25, 2023
@lilyinstarlight lilyinstarlight force-pushed the fix/cargo-c-gst-plugins-rs-hell branch from 9582fec to 42e6f4c Compare June 26, 2023 11:35
@vcunat vcunat merged commit 0dd2108 into NixOS:staging-next Jun 26, 2023
@lilyinstarlight lilyinstarlight deleted the fix/cargo-c-gst-plugins-rs-hell branch June 26, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants