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

wofi: fixes bad patch hash #83535

Merged
2 commits merged into from Mar 28, 2020
Merged

wofi: fixes bad patch hash #83535

2 commits merged into from Mar 28, 2020

Conversation

ghost
Copy link

@ghost ghost commented Mar 27, 2020 •
edited by ghost

Things done

Fixes hash inconsistency reported by @wkral : I'm really surprised that files on paste.sr.ht have changed, if anybody knows how works SouceHut, I would be glad to be enlighten :).

Related to changes in #83283.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

wkral
wkral approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@wkral wkral left a comment

Choose a reason for hiding this comment

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

Rebuilt with this change and it works for me. I'm pretty surprised about sourcehut, but I wonder if paste is like snippets or pastebin and things can be edited afterward.

Also thanks for your original fix. Wofi works first time now, where I was previously just launching some GTK application to initialize something before it would work in sway.

@ghost
Copy link
Author

ghost commented Mar 27, 2020

I'm pretty surprised too, I thought it was a diff generated from a specific commit, and it seems that's not the case.

Glad to read ! 🎉

@ofborg ofborg bot requested a review from erictapen March 27, 2020 22:28
erictapen
erictapen approved these changes Mar 28, 2020
View reviewed changes
Copy link
Member

@erictapen erictapen left a comment

Choose a reason for hiding this comment

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

Thanks again. Would you be interested in taking maintainership of this package? I don't use it anymore.

@ghost
Copy link
Author

ghost commented Mar 28, 2020

Yes, no problem for me, I'm using Sway as my daily driver for work ;).

@ghost
Copy link

ghost commented Mar 28, 2020

Fixes #83627

@ghost
Copy link

ghost commented Mar 28, 2020 •
edited by ghost

Is it possible that you copied the hash over from the fetchurl? fetchpatch normalizes the patch and hashes it afterwards, so using fetchpatch will result in a different result than using fetchurl. The hash in the old derivation matches the one from nix-prefetch-url https://paste.sr.ht/blob/1cbddafac3806afb203940c029e78ce8390d8f49.

In other words: Are you sure the fetchpatch hash was correct at some point?

@ghost
Copy link
Author

ghost commented Mar 28, 2020

I have to say that I'm not sure. I believe I could have done the mistake, indeed.

If you check the previous PR i switched from fetchurl to fetchpatch at some point, and I believe I kept the same hash. I was building on my system without any issue and I was not aware of this implementation detail. I'm really wondering why it was building (and still builds) on my computer ? I've tried on another computer, and it doesn't work indeed. Is there some kind of cache that uses only the hash to retrieve it ? Is there something I can do to avoid this mistake in the future ?

@ghost ghost changed the title wofi: fix paste.sr.ht variable hashes for patch wofi: fixes bad patch hash Mar 28, 2020
@ghost
Copy link
Author

ghost commented Mar 28, 2020

@petabyteboy : I've pushed an update

@ghost
Copy link

ghost commented Mar 28, 2020

Yes you are correct about the cache. The whole nix store is a giant cache. And both fetchurl and fetchpatch are based on fixed-output derivations. This is a special type of derivation where Nix will allow network access, but require the output hash to be known in advance, and after that it will use the hash to address the output in the store.

About how to prevent such problems: It's a bit difficult. If you change an expression (for example the version), you should probably try to replace the hashes that could depend on it with 00000...
For fetchurl and fetchpatch I think there is no solution that allows detecting those mismatches without testing for it.

ghost
ghost approved these changes Mar 28, 2020
View reviewed changes
Copy link

@ghost ghost 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 for me now 🎉

@ghost ghost merged commit c2600f0 into NixOS:master Mar 28, 2020
@ghost
Copy link
Author

ghost commented Mar 28, 2020

Thanks, I understand why I've made the mistake and how to not reproduce it:

  • Change the hash with some 0 to be sure it downloads it
  • I will always try on my second computer every derivation before submitting them

Sorry about that !

@ghost ghost mentioned this pull request Mar 29, 2020
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wkral @erictapen