Improve the JDK infrastructure on nixpkgs

Development
9

Thanks for kicking off this effort @jlesquembre.

I am using Java daily on nixos and darwin targets, and so far I found the support to be really good.

I don’t have commit access, but I have been looking for meaningful ways to contribute back to the community. Happy to help with code reviews and testing (especially on aarch64 darwin) if needed. I’d also be happy to work on PRs if I manage to get up to speed.

1 Like
10

As the author of the big bump PR that seems to have kicked all of this off, I’m glad to see people excited to discuss it! As mentioned, please take a look at the PRs linked in the umbrella issue! The big bump, which should address a lot of the outdated packages, has been ready to merge for quite a while now, and I hope it gets merged sometime in the next 3 months.

Once I get a bit more time, I’ll try and put down my thoughts on the various sub-issues linked in the umbrella issue.

1 Like
11

I think we should add a --version test in an installCheckPhase or something like that

testers.testVersion?

There is no updateScript for JDK derivations.

I do not believe this is “possible”, since not every Java program builds on all JDK.

Also, we have a much more serious problem in my modest opinion: Java bootstrap depends on open source abandonware.
I do not know how much Nixpkgs care about bootstrappability, but I believe this is far from a serious priority.

12

I am a maintainer of two such packages: jquake and protege-distribution.
While the first one is a proprietary application, I packaged the second like that because, at the time, I couldn’t figure out a way of building complex Maven applications with an acceptable level of maintenance burden on the Nixpkgs side of things.

If we manage to get a good solution for Maven, then maybe I could deprecate that package in favor of a source-built one.

13

As the maintainer of maptool, I would also be very happy to move to a source-built package if there was a better story for managing Gradle dependencies.

14

The difference, is that installCheckPhase makes the build fail if --version | grep ${version} fails, whereas testers.testVersion is not being run by nixpkgs-review.

15

You can try out our (somewhat new) maven.buildMavenPackage.

I’d also encourage you to add a some kind of an installCheckPhase for the proprietary package, so we’ll be able to know via nixpkgs-review if it gets broken without running it ourselves.

1 Like
16

whereas testers.testVersion is not being run by nixpkgs-review

Is this a bug or a feature of nixpkgs-review?

2 Likes
17

looks like a missing feature Add flag to build package `passthru.tests` by B4dM4n · Pull Request #397 · Mic92/nixpkgs-review · GitHub

1 Like
18

Even with that PR, or something alike merged, people won’t get used to add the --tests flag to their nixpkgs-review runs, so you might still be missing breakages. I don’t understand this insistence on passthru.tests.testVersion where an installCheckPhase is a much stronger and simpler compliance that a package can fulfill.

1 Like
19

The Nixpkgs manual says:

Tests that are part of the source package are often executed in the installCheckPhase.

Prefer passthru.tests for tests that are introduced in nixpkgs because:

  • passthru.tests tests the ‘real’ package, independently from the environment in which it was built
  • we can run passthru.tests independently
  • installCheckPhase adds overhead to each build

Should the manual be changed? Or nixpkgs-review? Or people’s habits for how and when they use nixpkgs-review versus other commands?

1 Like
20

Thanks for quoting Nixpkgs’ manual :slight_smile: . This was discussed a bit at #119731 but people gave their attention in the discussions there on topics other then this. Someone there did raise my opinion, but nobody commented on this opinion…

I noted myself to discuss this at Nixpkgs at some point when I’ll have time.

2 Likes
21

If the problem is human failure, then it can and should be fixed by automation.
The CI should add those flags, then.

2 Likes
22

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!

23

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
24

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.

25

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
26

Is this a bug or a feature?

And contradicting the current manual because of a fringe exception?

27

What about opening an issue and linking it here?

28

I think it has been done already? [Umbrella issue] Improve JDK infrastucture · Issue #313216 · NixOS/nixpkgs · GitHub