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
libusb-compat: 0.1.5 -> 0.1.7 && change libusb source to GitHub #48434
Conversation
|
||
src = fetchurl { | ||
url = "mirror://sourceforge/libusb/${name}.tar.bz2"; | ||
url = "https://github.com/libusb/libusb/releases/download/v${version}/${name}.tar.bz2"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't fetchFromGitHub here be used instead? The release pages don't seem to contain anything other than just the source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, there seems to be some difference in the source archive and release tarball: https://gist.github.com/lschuermann/61ab11d3cc7ae3f9073f0c0e1da04981.
I think most of these files could be generated using autoreconfHook, however I'm not quite sure whether a release tarball or building from the source archive is preferred.
This is the same situation as in #48381.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to use the source tarballs (non-release). This way the build will also succeed if somebody overrides src with something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll change it to use fetchFromGitHub
.
I can't build using @GrahamcOfBorg yet, can you try @infinisil? |
@GrahamcOfBorg build libusb-compat |
No attempt on x86_64-linux (full log) The following builds were skipped because they don't evaluate on x86_64-linux: libusb-compat Partial log (click to expand)
|
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: libusb-compat Partial log (click to expand)
|
@grahamofborg build libusb libusb1 |
@GrahamcOfBorg build libusb libusb1 |
Success on x86_64-darwin (full log) Attempted: libusb, libusb1 Partial log (click to expand)
|
Actually, that's a whole lotta rebuilds, this should go to staging. I doubt ofborg can build this without timeouting Edit: Oh right, the package itself shouldn't timeout, only the stuff that depends on it |
Success on aarch64-linux (full log) Attempted: libusb, libusb1 Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: libusb, libusb1 Partial log (click to expand)
|
My understanding of what |
Oh actually, when you switch this PR to staging, ofborg might not be able to build it because on staging stdenv got changed there. But anyways, this looks good to me if it's on staging. |
I noticed that the switch to staging pulled all the other commits from master over too. Should I rather rebase my commits? I've never contributed something to staging before. 😢 |
@GrahamcOfBorg eval |
44b0761
to
07e4caa
Compare
Success on aarch64-linux (full log) Attempted: libusb Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: libusb Partial log (click to expand)
|
Are there any updates on this pull request, please? |
Forgot about this for a while, putting it on the top of my todo list. |
Please also make the name -> pname transition while at it. |
@lschuermann ping? |
@lschuermann, any chance you have time to get these few changes sorted so we can get this merged? |
Yes, I was very busy these previous months, but starting today I have time for nixpkgs again. Let me sort this this very weekend. Sorry. |
Also, change the source repository to the GitHub repository pointed to by the official website.
07e4caa
to
aa63d51
Compare
Now both This should work, but I didn't manage to build it yet. My server's on it currently, but seeing how many packages I have to build from source this will take some long time. EDIT: Successfully built both |
@@ -43,8 +48,4 @@ stdenv.mkDerivation (rec { | |||
license = licenses.lgpl21Plus; | |||
maintainers = [ ]; | |||
}; | |||
} // stdenv.lib.optionalAttrs withStatic { | |||
# Carefully added here to avoid a mass rebuild. | |||
# Inline this the next time this package changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've inlined this statement as we're changing the expression anyways.
Why would you want to build from git checkout instead of the release tarball? I don't think we want this. The idea was to replace the sourceforge release tarball with the github release tarball, not to replace the tarball with the git checkout. Drop the aa63d51 commit from the PR, but you can keep the |
To put my 2 cents, I think |
Just a side note, https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/fetchgithub/default.nix#L31 |
I also agree that |
I think that was the entire idea. Not building from the release tarball removes unnecessary dependencies, and allows to override the |
You have to distinguish release tarballs (uploaded by the author of the project) and archive tarballs, which are generated by GitHub as a checkout of the commit. We are now using archive tarballs, which is a checkout of the tag and doesn't have the possibility of the author making malicious changes which aren't in the Git repo, like with release tarballs. |
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)