An aside from folks sorta getting their feet wet (the toes went first) with Java - what JDK is ideal for nixpkgs work? Does it even matter? Please feel free to DM me as to not derail overall thread. Many thanks and this feels great. @doronbehar hope you are well!
CI doesn’t run nixpkgs-review automatically, and I don’t think ofborg maintainers will want to make it run it. Also users don’t do it for every PR they review, so I’m not sure what kind of automation you can use here. I agree though that perhaps if you run a nixpkgs-review, it should run the tests unconditionally, perhaps even without adding a --tests flag, or perhaps only add a --no-tests flag. In anycase, I wouldn’t call that automation, since running nixpkgs-review is not automated.
1 Like
As far as I remember, ofBorg runs passthru.tests automatically, without relying on nixpkgs-review.
So I believe this part is already solved.
Or is there something I am not understanding? Serious question.
Should we educate people to run the passthru.tests, then?
Or no amount of education is sufficient? If not, what other solution we have?
Understood.
Yes but I was talking about passthru.tests of packages that depend on a change of a PR, and are not affected directly by it. Ofborg doesn’t run passthru.tests for these packages, and it doesn’t even build them. And even if you @ofborg build them it won’t run these tests. They are way too hidden.
That’s sound like a much greater effort in comparison to convincing people that the docs should recommend putting a --version test in the installCheckPhase and add that phase to the Java packages that are made up by a prefetched jar file.
1 Like
Is this a bug or a feature?
And contradicting the current manual because of a fringe exception?
What about opening an issue and linking it here?
I think it has been done already? [Umbrella issue] Improve JDK infrastucture · Issue #313216 · NixOS/nixpkgs · GitHub
No body will want to make ofborg run nixpkgs-review automatically, with or without including the dependent packages’ passthru.tests. So this is niether bug, nor feature, but a design choice.
1 Like
I’m not sure if this is off topic or not (if it is feel free to correct me): I currently feel that the barrier to entry to building JDK applications in the sandbox is way too high.
One of the packages I maintain (Keystore Explorer) is just built by grabbing its prebuild from the release tarball. I also maintain androidenv and it’s historically been difficult to do sandboxed builds of Android applications due to Gradle incompatibility issues.
All this to say: Any change positively affecting the ease of use of Gradle would be very welcome to me. 
1 Like
You are then proposing a workaround, “no one wants to run tests, but tests should be run, so let’s force the tests!”.
Let me know if I understand: your idea is that if some package does not build with a newer JDK, then this package should block that JDK upgrade?
(Do not consider the obvious solution “create a whole new derivation for the newer JDK while the older will be pinned for that package”.)
Well, I can say by my own experience that it will make too much people angry.
That’s pretty correct yes.
Not necessarily block the upgrade. But this situation will raise people’s attention to these failing packages, and invite them to consider doing either or a few of the actions below:
- @ mention the maintainers of the failing packages.
- If the maintainers don’t reply, or if there is no maintainer, check when was the package updated last time
- If the package hasn’t seen an update in Nixpkgs for a long time, check if also upstream hasn’t updated it a long time.
- Check if upstream has had issues opened in their bug tracker regarding issues with new JDKs, or other failures.
- Check if upstream is responsive in general to issues opened in their bug tracker.
- Consider marking the package as broken, since there is no chance it will ever be functional.
- Consider removing it from Nixpkgs.
BTW regarding the passthru.tests v.s the installCheckPhase test, the above thinking led me to another argument in favor of installCheckPhase:
It may be possible with those pre-built .jar derivations, that people have these packages added to their environment.systemPackages, and they haven’t tried to use these packages for a long time, and hence they are not aware that some of them are not even functional, because even nixos-rebuild doesn’t run passthru.tests. If these packages had installCheckPhase failing when people run nixos-rebuild, it will raise their attention to this packages, and will force them to actually do something about them.
Hopefully, in a case such as described above, those users will open a Nixpkgs PR to either mark that package as broken or remove it.
1 Like
Historically, I wrote testVersion and the documentation on how to test what mostly just to record what appeared to be the consensus at the time.
The main advantage of passthru.tests over installCheckPhase here is probably that passthru.tests will test the ‘actual’ package and not risk leaking build-time stuff into the tests. I wonder if we should promote checking versions both in installCheckPhase and in passthru.tests, especially in derivations that essentially download (rather than build) a binary?
I don’t think there is value in a passthru.tests.version if you also run the same command in the installCheckPhase, and I think that in general, a --version test in an installCheckPhase is a good practice.
I can’t think of any such package where it will be make a difference. Do you have an example?
This too, sounds super obscure to me. I can imagine a badly design program would create files in $PWD when it launches, even if it is only asked for it’s --version, but I can’t imagine how would a program write undesired files to $out - how would it even know about this directory?
I can only imagine a program attempting to write something into the directory of the binary itself, which is at $out/bin, but if this program is so badly designed, we don’t have to apply this installCheckPhase…
In anycase, I think that such rare and problematic programs shouldn’t influence our policy so broadly.
That’s pretty correct yes.
A quick and dirty workaround that will not be quick but always dirty.
BTW regarding the passthru.tests v.s the installCheckPhase test, the above thinking led me to another argument in favor of installCheckPhase:
I have a better idea:
Force everyone to run the Passthru Tests!
It has all the advantages of your approach without the drawbacks.
The alternative is to deprecate and remove passthru tests - since, taking your argument to its limit, they are merely decorative.
I wonder if we should promote checking versions both in
installCheckPhaseand inpassthru.tests, especially in derivations that essentially download (rather than build) a binary?
Double check looks so weird.
I wonder what happens when a package fails passthru but works in installCheck (or vicae-versa)…
In anycase, I think that such rare and problematic programs shouldn’t influence our policy so broadly.
Famous last words…
Indeed the far-from-obvious Nixpkgs codebase is full of corner cases that forces both Nix, Nixpkgs and the packages themselves.
What about not being able to use strictDeps everywhere without reordering the *Inputs?
My point is that because the drawbacks are so rare, if even exist, I don’t think it is fare to force all people to do something which is barely supported by our nixpkgs tools.
Why take it to the limit? Sometimes people use passthru.tests.nixos = nixosTests.${pname} which are helpful when it is a leaf package that gets an update.
It’s OK that in a large codebase like Nixpkgs there are edge cases that you handle separately from the general case. The question is how much effort you give to make your policy, or a certain general function, inclusive for edge cases that you haven’t even seen evidence that they exist.
passthru.tests can detect packaging issues (e.g. insufficiently wrapped programs) that installCheckPhase can miss (AFAICS).
1 Like
You want automation, then arguments like “sometimes people . . .” are of no effect. Indeed you rejected all alternatives because "sometimes people . . . ".
You want automation, and those tests are not automatically executed, therefore they are mere decorative curiousity – at least for this use case.
And it looks more insane for packages that already have testers.testVersion implemented - they should be converted to installChecks because we want to break flag JDK updates! Nice…
============
If you allow me a suggestion,
I would just create something like a NixOS test module that merely includes all those less than 80 Jar-based packages and their passthru tests.
That’s inaccurate. You seem to refuse to understand the (apparently too subtle for you) difference between a leaf package that gets an update (for which ofborg will run all of it’s passthru.tests) v.s updates to something like a openjdk for which nixpkgs-review won’t run any of the dependent packages’ passthru.tests.
I think I’ve made my self clear enough, so I won’t keep up the discussion here. This can be further discussed at the Nixpkgs issue / PR I will open about this subject.
This is precisely the automation I am talking about.
Have you read the suggestion I did at the end?
Yes, you have made yourself clear. It does not imply your workaround is good or convincing.
Let’s keep it civil, please. There’s to be no fighting in the war room!
I’m not totally caught up but just want to keep this civil and on track. I appreciate the energy and drive you both bring and you’re doing a mostly thankless job.
2 Likes