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.

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

Java packages #20444

Merged
merged 10 commits into from
Feb 28, 2017
Merged

Java packages #20444

merged 10 commits into from
Feb 28, 2017

Conversation

NeQuissimus
Copy link
Member

@NeQuissimus NeQuissimus commented Nov 15, 2016

Note: I am replacing #12770 with this.

I have taken most of the concepts from the mentioned PR and cleaned them up.
I have also added a full build of JUnit 4.12 and a sample project, which uses said JUnit version.

Most artifacts are binaries because an extremely simplistic project like the one I created requires hundreds of artifacts. Building all of those from source is (surprisingly) impossible. - The Java world for you :D

I am terrible at rebasing in git, so I suggest we squash upon PR merge. (I did things out of order and in weird chunks, so it would not be a straight rebase with squashing; unless we remove context)

Here is what I have done:

Build support

Essentially what was already in #12770 except much less copying because as it turns out, Maven does like symbolic links if you do it right :)
I have a way to fetch Maven Central binaries (fetchMaven) and a build helper (mavenbuild).
I have also added optional skipTests and quiet parameters (both of which default to true).

Why default skipTests to true?
Turns out, JUnit expects hamcrest-core on the classpath without defining it as an explicit runtime dependency. If somebody knows how to convince a pom.xml that depends on JUnit but not hamcrest-core to run tests in offline mode, please do provide feedback.

Plugins

I have added a large number of default Maven plugins.
Theses are needed to get Maven to do anything at all...
I have created mavenMinimal for this purpose, a list of plugins and their dependencies that are required no matter what you are building. This list of automatically included when using mavenbuild.

Libraries

So far, I have all the libraries that are needed to

  • Run Maven (3.3.9)
  • Build JUnit 4.12
  • Run tests using JUnit 4.12

Mirror

I have added a maven alias to the fetchurl mirrors. This is not strictly required but may help in the future if URLs change or another mirror can be added.

Useful stuff

  • JUnit 4.12
  • Maven-Hello v1.0 and v1.1 (sample project to prove things work, code on GitHub)

Collections

Generally, if you have one of the Maven libraries that you depend on, they eventually pull in all their brothers and sisters with the same versions numbers as well. So I created mavenLibs collections to make things easier.

SHA512

Sorry @copumpkin, I know we just talked about it but the number of SHA512 in my branch had already been in the triple digits, so I kept it this way. (Forgive me :D)

Maven

The Maven/Java environment is dirty, crazy and unorganized for the most part.
Try building a "Hello World" application and you need to pull in hundreds of dependencies.
Don't even think about building all of those from source (see #12770) for examples of circular dependencies and seemingly endless chains of dependencies that will feel like time travel into the beginnings of Java.
This is why the code in this PR looks the way it does and lists more than 3,500 new LoC.
Compared to the old PR I have already abstracted and massively simplified...

Future

I would ask for any feedback I can get here.
This has been a lot of work but if most people are against this, I won't be mad.
If we decide to include javaPackages into nixpkgs, we should definitely write some documentation, which is why I added the appropriate label.

Note sure what else to say...

@mention-bot
Copy link

@NeQuissimus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @cmfwyp and @civodul to be potential reviewers.

@NeQuissimus
Copy link
Member Author

/cc @copumpkin @shlevy @puffnfresh @bendlas @Mathnerd314 because they all had comments on the old PR

commonsLoggingApi_1_1
findbugsJsr305_2_0_1
googleCollections_1_0
junit_3_8_1
Copy link
Member

Choose a reason for hiding this comment

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

junit_3_8_1 + junit_3_8_2 - isn't that duplicate classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

At runtime, yes... But they are both required as transitive dependencies...

Copy link
Member

@kamilchm kamilchm Nov 16, 2016

Choose a reason for hiding this comment

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

strange, things like this could leads to https://en.wikipedia.org/wiki/Java_Classloader#JAR_hell and there are tools to work with it, like http://docs.jboss.org/tattletale/userguide/1.1/html/reports.html#multiplejarfiles
But it could be more an issue for maven itself than this derivation.
How did you generate the list of dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed my local ~/.m2 built the project using the "normal" way and then did a find ~/.m2/repository -name '*.jar' -o -name '*.pom'

Copy link
Member

Choose a reason for hiding this comment

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

IMO maven shouldn't use junit at runtime at all. On a first glance it could be test dependency of https://mvnrepository.com/artifact/org.apache.maven.surefire/maven-surefire-common/2.17
Anyway, it's ok if it doesn't break it.
We could try to optimize it later. It may be worth to dig into mvn dependency:tree and treat compile, test, provided scopes differently instead of listing ~/.m2 tree.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, you could strip certain dependencies if you choose to skip tests. However, that did not seem worth the effort right now. Most builds from source should want to not skip tests :)

@kamilchm
Copy link
Member

Just an idea. It would be nice if fetchMaven could use attribute names from maven space, like:

fetchMaven rec {
  version = "2.4";
  artifactId = "maven-install-plugin";
  groupId = "org.apache.maven.plugins";
...

instead of:

fetchMaven rec {
  version = "2.4";
  baseName = "maven-install-plugin";
  package = "/org/apache/maven/plugins";
...

@NeQuissimus
Copy link
Member Author

Re: groupId/artifactId

YES!

@NeQuissimus
Copy link
Member Author

Ping :)

@garbas
Copy link
Member

garbas commented Jan 4, 2017

restarted travis builds

@garbas
Copy link
Member

garbas commented Jan 4, 2017

looks like travis fails

@garbas
Copy link
Member

garbas commented Jan 4, 2017

sorry i just notices, that this PR is meant for feedback. ignore me

@copumpkin
Copy link
Member

👍

@copumpkin
Copy link
Member

I say we merge it and iteratively improve on it. It fills in a big gap in nixpkgs today, and isn't going to hurt/conflict with any of the other package ecosystems by being merged.

@globin
Copy link
Member

globin commented Feb 13, 2017

The travis failures definitely look valid.

@NeQuissimus
Copy link
Member Author

I will take a look at this today or tomorrow (I hope). Seems that Maven Central is sensitive to the (unintentional) double slash all the sudden...

@NeQuissimus
Copy link
Member Author

Found two typos, everything should be good to go now. Let's see what Travis thinks...

@NeQuissimus
Copy link
Member Author

There we go, Travis is happy

@copumpkin
Copy link
Member

One more question: do people always just ask for the pom and jar combination or are there use cases for only asking for one? I'm wondering if we could cut down on the hash noise a bit by making fetchMaven do a recursive hash on the two files it retrieves at the same time. Not sure if that would complicate other things, but copying and pasting long strings of junk is bad enough without having to do it twice for every package 😄

@copumpkin
Copy link
Member

For example, I have a (yet unpushed, sorry) fetchIvy fetcher that will use ivy to retrieve a whole pile of dependencies at once, and just hash once per bundle of dependencies.

@NeQuissimus
Copy link
Member Author

I found it actually depends on the artifact... Some are POM-only, most want JAR and POM but it is technically allowed to only use the JAR and leave the POM alone (i.e. if you want to avoid pulling transitive dependencies). By doing it this way, I just decided to give all the options :)

@copumpkin
Copy link
Member

@NeQuissimus I guess I'm saying that you could still allow people to ask for all those combinations without requiring one hash per thing you ask for. It might not be worth it, but as a potentially avid user of this thing I'm a little concerned about the code's UX 😄

@NeQuissimus
Copy link
Member Author

I am hoping to eventually have some sort of auto-generation. I mean, for now if you want to build your own Maven project, you will probably need to do so much by hand, it is going to get cumbersome real quick. This is essentially more of a PoC than anything, I suppose :)

@spacekitteh
Copy link
Contributor

How's this going?

@NeQuissimus
Copy link
Member Author

This is done from my POV

@copumpkin
Copy link
Member

Fine by me!

@@ -9761,6 +9761,10 @@ in

leaps = callPackage ../development/tools/leaps { };

### DEVELOPMENT / JAVA MODULES

javaPackages = recurseIntoAttrs (callPackage ./java-packages.nix { });
Copy link
Member

Choose a reason for hiding this comment

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

I actually think this should be callPackages rather than callPackage. The difference is that the former recurses (one level) into the inner attrset and makes the individual derivations there overridable. I think you'll find that in the current setup, override[Derivation] won't work on anything inside there.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's going to mess with my lists. Is there a straight-forward List -> Set function?
callPackages wants things to be sets.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't see the lists you're speaking of. Can you point them out? It looks like java-pacakges.nix results in a big attrset full of attributes it inherits from other .nix files?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what I get

~/dev/nixpkgs  java_packages ✗                                                                                                                                                      
▶ nix-build -A javaPackages.mavenHello_1_1 --show-trace
error: while evaluating the attribute ‘buildPhase’ of the derivation ‘maven-hello-1.1’ at /home/nequi/dev/nixpkgs/pkgs/development/java-modules/build-maven-package.nix:10:10:
while evaluating ‘optionalString’ at /home/nequi/dev/nixpkgs/lib/strings.nix:138:26, called from /home/nequi/dev/nixpkgs/pkgs/development/java-modules/build-maven-package.nix:24:7:
while evaluating ‘unique’ at /home/nequi/dev/nixpkgs/lib/lists.nix:432:12, called from /home/nequi/dev/nixpkgs/pkgs/development/java-modules/build-maven-package.nix:12:14:
while evaluating ‘flatten’ at /home/nequi/dev/nixpkgs/lib/lists.nix:90:13, called from /home/nequi/dev/nixpkgs/pkgs/development/java-modules/build-maven-package.nix:12:22:
while evaluating the attribute ‘mavenMinimal’ at /home/nequi/dev/nixpkgs/pkgs/development/java-modules/maven-minimal.nix:13:3:
while evaluating ‘flatten’ at /home/nequi/dev/nixpkgs/lib/lists.nix:90:13, called from /home/nequi/dev/nixpkgs/pkgs/development/java-modules/maven-minimal.nix:13:18:
while evaluating ‘concatMap’ at /home/nequi/dev/nixpkgs/lib/lists.nix:79:18, called from /home/nequi/dev/nixpkgs/lib/lists.nix:92:10:
while evaluating anonymous function at /home/nequi/dev/nixpkgs/lib/lists.nix:92:21, called from undefined position:
while evaluating ‘flatten’ at /home/nequi/dev/nixpkgs/lib/lists.nix:90:13, called from /home/nequi/dev/nixpkgs/lib/lists.nix:92:24:
while evaluating ‘mkAttrOverridable’ at /home/nequi/dev/nixpkgs/lib/customisation.nix:111:33, called from /home/nequi/dev/nixpkgs/lib/attrsets.nix:199:52:
value is a list while a set was expected

Copy link
Member Author

Choose a reason for hiding this comment

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

In maven-minimal I build up a long list of default dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also genAttrs if you want to specify a function to create attribute names

Copy link
Member Author

Choose a reason for hiding this comment

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

This is going to open up all kinds of weird little things. I won't get around to doing this in the near future... I had hoped to get this into the next release, but I will try to fix this first.

Copy link
Contributor

Choose a reason for hiding this comment

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

dang :\ maybe try to break it up into smaller, mergable chunks?

Copy link
Contributor

Choose a reason for hiding this comment

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

or merge it now as is, and then work on fixing it

Copy link
Member Author

Choose a reason for hiding this comment

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

This is pretty much the bare minimum to build anything with Maven :) If we can deal with callPackage instead of callPackages, let's merge...

@copumpkin
Copy link
Member

Sure, fine by me. Thanks for the effort and patience!

@copumpkin copumpkin merged commit 1f77939 into NixOS:master Feb 28, 2017
@spacekitteh
Copy link
Contributor

Don't forget to make an issue in the issue tracker btw!

@sephalon
Copy link
Contributor

@NeQuissimus I'm trying to package an Ant-based project right now that needs a couple of JARs not shipped inside the tarball from Maven Central. Could you please give an example on how to add a Java package to the build inputs and modify the classpath?

@NeQuissimus
Copy link
Member Author

You're going to need to write something similar to pkgs/development/java-modules/m2install.nix. I have not dealt with this stuff in a while, sorry.

@NANASHI0X74
Copy link
Contributor

looks like no one got round to doing all those improvements and fixes in the end, huh? 🙂

I'm currently wondering about wether to build an existing maven based project with nix. I'm fairly new to writing my own derivations and it's been quite a while since I've used java and I've also never really got to know the build systems in the java ecosystem, mostly I relied on my IDE doing the work for me back then.
Some documentation on how to use this to package maven projects would be nice :D
when is fetchmaven needed for example?
I'll just try to naively use mavenbuild on the repository like it's done with maven-hello.
Maybe I can write the documentation myself later if I succeed

@tomodachi94 tomodachi94 added the 6.topic: java Including JDK, tooling, other languages, other VMs label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants