got: 0.97 -> 0.98.2 #307538
got: 0.97 -> 0.98.2 #307538
Conversation
ofborg
bot
added
11.by: package-maintainer
|
Note: PR title should be updated to properly indicate this is a |
|
Great catch, @JohnRTitor! The muscle memory in my fingers got (pun intended) the better of me ;) |
|
Result of 1 package built:
|
JohnRTitor
added
the
12.approvals: 1
JohnRTitor
added
12.approvals: 2
|
@NixOS/nixpkgs-merge-bot merge |
|
@JohnRTitor merge not permitted (#305350): |
|
@afh can you migrate this to by-name? |
|
Is there a "general" guideline on which packages should be under a category or in |
| doInstallCheck = true; | ||
|
|
||
| installCheckPhase = '' | ||
| runHook preInstallCheck | ||
| test "$($out/bin/got --version)" = "got ${finalAttrs.version}" | ||
| runHook postInstallCheck | ||
| ''; | ||
| passthru.tests.version = testers.testVersion { | ||
| package = finalAttrs.finalPackage; | ||
| }; |
There was a problem hiding this comment.
@AndersonTorres, is this change a proper and correct improvement? I have yet to fully understand passthru and testers.testVersion, but from my current understanding this should be good. I'd truly appreciate your feedback and expertise on this.
There was a problem hiding this comment.
Yes, it is fine.
As I have explained before, installCheckPhase is meant to run checks provided by the upstream package developer. The difference between checkPhase and installCheckPhase is that installCheckPhase runs after installation while checkPhase runs after build.
Custom scripts are better suited to passthru.tests.
There was a problem hiding this comment.
Thanks for the explanation, @AndersonTorres, that's helpful!
Yet for some odd reason my 🧠 refuses to understand passthru (might be something to do with the wording that seems odd to me—not that it is, just that it makes it harder for me to grok it). Seems I need to revisit and re-read the available documentation…
There was a problem hiding this comment.
passthru is similar to meta in the sense that a modification in passthru does not trigger a mass rebuild.
Usually we use passthru in order to collect some useful info from the derivation and expose it to the external world.
E.G. suppose you have a parameter, uiType, that enables a fancy UI in got, and the end user wants to know which GUI type was used (say, cli, curses, gtk3, qt10).
Then you can write
passthru = {
uiType =
if enableCurses then "curses"
else if enableGtk3 then "gtk3"
else "cli";
};This way, the end user can evaluate got.uiType to know what UI was enabled.
Obviously the code above is a crude example. Usually we do cleaner and more useful stuff.
E.G. in live555 we put a test:
passthru.tests = {
inherit vlc;
};This way we can test this by calling live555.tests.vlc.
The idea is that vlc depends on live555, and sometimes the vlc team does not update its code when live555 introduces a breaking change. So we at Nixpkgs catch this by running this test.
|
Sure thing, @AndersonTorres 👍 Please see comment above for a question I have about an additional change I made regarding |
wegank
removed
the
12.approvals: 2
A very rough guideline is:
|
|
Thanks for the review, @AndersonTorres, and thanks for merging, @Mic92, much appreciated 🙂👍 |
Description of changes
Upstream has switched to libressl (see
ThomasAdam/got-portable@62e037f)Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.