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

wdomirror: fix build failure by applying patch from GH #5 #172441

Merged
merged 1 commit into from May 11, 2022
Merged

wdomirror: fix build failure by applying patch from GH #5 #172441

merged 1 commit into from May 11, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 11, 2022

ZHF: #172160

Description of changes

A recent change to xdg-shell-client-protocol causes a build failure.
This was reported upstream as GH #5. User @AlexBMJ posted a two-line
fix in that issue, but the developer has not yet responded:

progandy/wdomirror#5 (comment)

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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

A recent change to xdg-shell-client-protocol causes a build failure.
This was reported upstream as GH #5.  User @AlexBMJ posted a two-line
fix in that issue, but the developer has not yet responded:

progandy/wdomirror#5 (comment)
@ghost
Copy link
Author

ghost commented May 11, 2022

I just tested the resulting binary.

This is a really nifty program! I'd hate to see it dropped from nixpkgs.

@ghost
Copy link
Author

ghost commented May 11, 2022

The ZHF announcement PR says: "Please ping @NixOS/nixos-release-managers on the PR and add the 0.kind: build failure label to the pull request. If you're unable to because you're not a member of the NixOS org please ping @dasJ, @tomberek, @jonringer, @Mic92"

I am unable to add this build label, so I therefore am issuing this ping.

@Mindavi Mindavi added the 0.kind: build failure A package fails to build label May 11, 2022
@Mindavi
Copy link
Contributor

Mindavi commented May 11, 2022

I don't know anything about the code, but it seems at least somewhat sane. I wanted to ask to fetchpatch this, but I guess that'll be a bit hard like this, so 👍 from me.

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

1 package built:
  • wdomirror

@Mindavi Mindavi merged commit dfca4b7 into NixOS:master May 11, 2022
@ghost
Copy link
Author

ghost commented May 11, 2022

Yeah I know, I felt really guilty not using fetchpatch, but the guy who posted the patch posted it as (a) part of a github comment (not a PR) and (b) as part of a shell script with the patch as a here-doc inside the script which applies the patch.

I suppose I could have fetchurl'ed the github comment and then done some sed/awk/whatever to it but I think that would've been just as ugly.

So yeah, it's not pretty. But the tool is very cute.

@ghost ghost deleted the pr/zhf/wdomirror branch May 11, 2022 08:19
@ghost ghost mentioned this pull request May 11, 2022
13 tasks
@ghost
Copy link
Author

ghost commented May 11, 2022

Ah, see #172466

Shortly after this merged, one of the upstream contributors responded to my request and there is now a PR we can reference with a patch that applies cleanly. So we can drop the patch.

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.

1 participant