documentation: add documentation for buildMaven #100660

fzakaria · 2020-10-15T23:37:50Z

Motivation for this change

Add nice markdown documentation for how to use mvn2nix plugin and the
buildMaven function within nixpkgs.

The goal is to have a thorough documentation similar to that of other languages.
ex. https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/ruby.section.md

Things done

CC @jerith666 @volth @zimbatm @charles-dyfis-net

doronbehar

A few comments:

Double invocation
- You mention twice that subsequent runs my result in different output hashes. If our situation is so awful, putting it in the docs is a kind of "recommendation" to keep using this method, so perhaps it'll be better not to document this?
- Why do you use src = ./.;? I thought this is a guide that teaches people how to contribute java packages to Nixpkgs.
buildMaven
- It should be made clear that a Nixpkgs contributor needs to download the project's source code, and run mvn org... in the project's root directory, or where ever the mvn entry point file (pom.xml?) is.
- You kind of mix the "build" sections.
General
- I'd start this document by writing that the derivation one would callPackage to should look like the derivation in the end.
- Isn't pom.xml a file that all maven projects need?
- So what build.repo returns is the value repository should get? (If the answer is yes, then you should make it clearer :))
- Perhaps it'd be better to just give two full examples of using buildMaven and using the double invocation (if we'd decide it's good to even document it).

Oh and I know you hate docbook, so am I. But, there's 1 advantage in docbook that's not available in our (current) markdown format: Take a look here:

nixpkgs/doc/languages-frameworks/go.xml

Lines 39 to 68 in 0826f2e

    
             vendorSha256 = "1879j77k96684wi554rkjxydrj8g3hpp0kvxz03sd8dmwr3lh83j"; <co xml:id='ex-buildGoModule-1' /> 
        
             runVend = true; <co xml:id='ex-buildGoModule-2' /> 
        
             meta = with lib; { 
        
               description = "Simple command-line snippet manager, written in Go"; 
        
               homepage = "https://github.com/knqyf263/pet"; 
        
               license = licenses.mit; 
        
               maintainers = with maintainers; [ kalbasit ]; 
        
               platforms = platforms.linux ++ platforms.darwin; 
        
             }; 
        
           } 
        
           </programlisting> 
        
             </example> 
        
             <para> 
        
              <xref linkend='ex-buildGoModule'/> is an example expression using buildGoModule, the following arguments are of special significance to the function: 
        
              <calloutlist> 
        
               <callout arearefs='ex-buildGoModule-1'> 
        
                <para> 
        
                 <varname>vendorSha256</varname> is the hash of the output of the intermediate fetcher derivation. 
        
                </para> 
        
               </callout> 
        
               <callout arearefs='ex-buildGoModule-2'> 
        
                <para> 
        
                 <varname>runVend</varname> runs the vend command to generate the vendor directory. This is useful if your code depends on c code and go mod tidy does not include the needed sources to build. 
        
                </para> 
        
               </callout> 
        
              </calloutlist> 
        
             </para>

And here: https://nixos.org/manual/nixpkgs/unstable/#ex-buildGoModule-1

Docbook can link lines in source code to documentation, which is really nice.

doc/languages-frameworks/maven.md

doronbehar · 2020-10-16T07:41:59Z

doc/languages-frameworks/maven.md

+  version = "0.1";
+  name = "${pname}-${version}";
+  src = ./.;
+  buildInputs = [ jdk maven makeWrapper ];


Technically they aren't needed as they are referenced thanks to the makeWrapper call.

Suggested change

buildInputs = [ jdk maven makeWrapper ];

buildInputs = [ makeWrapper ];

We're invoking mvn in via PATH in buildPhase though. I figure a jdk command is also necessary for mvn compile

I'll add a section on "Choosing the correct JDK"
The current mvn derivation allows overriding JAVA_HOME

❯ cat /nix/store/2hhmmj0vbb5d181nfx2mx3p7k54q44ij-apache-maven-3.6.3/bin/mvn #! /nix/store/6737cq9nvp4k5r70qcgf61004r0l2g3v-bash-4.4-p23/bin/bash -e export JAVA_HOME=${JAVA_HOME-'/nix/store/knh4n8fzrslyzgrw6p8nh5v07s0ycznc-openjdk-8u265-ga'} exec "/nix/store/2hhmmj0vbb5d181nfx2mx3p7k54q44ij-apache-maven-3.6.3/maven/bin/mvn" "$@"

What i've done in the past is just make sure to set JAVA_HOME to the correct JDK (i.e. JDK8 vs JDK11).
An alternative approach would also be to overrideAttrs

fzakaria · 2020-10-16T16:02:36Z

@doronbehar thank you for your in depth review -- I will incorporate most/all of your feedback.

For now, I will iterate using markdown purely due to my familiarity. I want to separate the issue of content vs. problems with how I use markup. A pragmatic approach may be to follow-up with another pull-request to convert it to docbook once the content is solid.

doc/languages-frameworks/maven.md

roberth · 2020-10-16T18:32:53Z

Looking already pretty good. Feel free to link to your own project.

fzakaria · 2020-10-16T23:09:59Z

Thanks for the feedback everyone (@roberth & @doronbehar)
I have made the documentation much more explicit by going through an example Maven project with a dependency.
I demonstrate building the Maven repository using two strategies, building the JAR for use as a library & wrapping the JAR for it to be runnable.

If the documentation is pretty close to the mark, I'd like to be able to submit it and continue to iterate on it.
Further improvements (docbook or smaller nits) can be done in a follow-up I hope.

doc/languages-frameworks/maven.md

doronbehar

It's getting better. But:

Again, I tend to think we should not document the "double invocation" method.. But even if we do, it should be mentioned after mvn2nix-maven-plugin.
Again, it would be easier IMO to understand it all, if the 1st expression the reader will see, is an expression of a derivation that uses buildMaven, in the let repository = .

doc/languages-frameworks/maven.md

fzakaria · 2020-10-19T22:07:40Z

Thanks again for the suggestions.
I applied them directly mostly and reworked it according to the feedback @roberth & @doronbehar

doc/languages-frameworks/maven.md

nixos-discourse · 2020-10-21T06:02:50Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/rfc-0072-fcp-switch-to-commonmark-for-documentation/9560/4

doronbehar

I notice you use a lot _ instead of a ``` where I'd use the later. Why?

doc/languages-frameworks/maven.md

doronbehar · 2020-10-22T13:25:22Z

doc/languages-frameworks/maven.md

+
+We will use the same repository we built above (either _double invocation_ or _buildMaven_) to setup a CLASSPATH for our JAR.
+
+### CLASSPATH


I'm a little bit confused about this section. In the derivation of it you create a wrapper, and that calls Main as it seems. What's the difference between this and the last example expression?

So fundamentally, your Java invocation needs to have all dependencies on the CLASSPATH.

CLASSPATH is the search path Java uses to lookup classes

The CLASSPATH can be set explicitly within a JAR which the subsequent section does through the help of Maven via the plugin via the MANIFEST file.

Here is the contents of the JAR and we see the CLASSPATH is set and expected at a fixed location relative to the JAR.

❯ unzip -q -c /nix/store/8d4c3ibw8ynsn01ibhyqmc1zhzz75s26-maven-demo-1.0/share/java/maven-demo-1.0.jar Manifest-Version: 1.0 Archiver-Version: Plexus Archiver Built-By: nixbld Class-Path: . ../../repository/com/vdurmont/emoji-java/5.1.1/emoji-jav a-5.1.1.jar ../../repository/org/json/json/20170516/json-20170516.jar Created-By: Apache Maven 3.6.3 Build-Jdk: 1.8.0_265 Main-Class: Main

The first approach though instead builds the JAR normally and then augments the makeWrapper to set the CLASSPATH via the command line.

You've given feedback on providing context on which is appropriate which I will do.

Fundamentally, if you are a nixpkgs author wrapping another project, the method by setting the CLASSPATH via makeWrapper is preferable since it's a bit more of a pain to create a patch file to change the project's pom.xml

If you are a project writer and using nix-shell or nix-build to build your project, it would be easier for you to modify your pom to add the required plugin.

I don't read your comments, as I expect these explanations to be in the documentation.

doronbehar · 2020-10-22T13:27:11Z

doc/languages-frameworks/maven.md

+}
+```
+
+### MANIFEST file via Maven Plugin


Is this a necessity to the above? It should be made clear in general, whether sections are targeted towards developers of a shell.nix with maven, or casual contributors to Nixpkgs that just want to add a Java program to Nixpkgs, and compile it from source. Hence, I don't understand in what case one should need to "augment" the pom.xml file.

I added such sections.

doronbehar · 2020-10-22T13:28:58Z

doc/languages-frameworks/maven.md

+{ pkgs ? import <nixpkgs> { }, stdenv ? pkgs.stdenv, lib ? pkgs.lib
+, maven ? pkgs.maven, callPackage ? pkgs.callPackage
+, makeWrapper ? pkgs.makeWrapper, jre ? pkgs.jre }:


If it'll be clearer when a nix expression is targeted at contributors or shell.nix writers, I should tell you when to remove these ? defaults, since in many cases these are not recommended as callPackage takes care of these defaults.

These defaults exist because for the purpose of demonstration each file (https://github.com/fzakaria/nixos-maven-example) is individually runnable via nix-build

doronbehar · 2020-10-22T13:29:21Z

doc/languages-frameworks/maven.md

+    mkdir -p $out/share/java
+
+    # create a symbolic link for the repository directory
+    ln -s ${repository} $out/repository


Why is this needed?

Please see the comment above about the CLASSPATH value in the jar's MANIFEST file.

doc/languages-frameworks/maven.md

fzakaria · 2020-10-22T16:42:56Z

I notice you use a lot _ instead of a ``` where I'd use the later. Why?

I think you mean *; it might be a habit I picked up from running prettier (an opinionated formatter).

fzakaria

(Oops, I did a bunch of single comments instead of review ; sorry)

fzakaria · 2020-10-22T16:56:06Z

doc/languages-frameworks/maven.md

+    mkdir -p $out/share/java
+
+    # create a symbolic link for the repository directory
+    ln -s ${repository} $out/repository


Please see the comment above about the CLASSPATH value in the jar's MANIFEST file.

doronbehar · 2020-10-23T05:31:06Z

I notice you use a lot _ instead of a ``` where I'd use the later. Why?

I think you mean *; it might be a habit I picked up from running prettier (an opinionated formatter).

No, I meant ``` - i.e code keywords like `buildMaven`.

doc/languages-frameworks/maven.md

doronbehar · 2020-10-23T07:54:03Z

@fzakaria I hope my review isn't to harsh :). I just want to make it clear for everyone in the future. Thanks for doing this anyway!

fzakaria · 2020-10-26T17:34:14Z

@doronbehar thank you for the review.

I'm hoping we can get this in soon; it's seen quite a few revisions. Improvements can continue to happen in subsequent pull-requests. It's definitely a place that will help others who want to use Java + Nix.

The documentation is pretty thorough but does expect minimal understanding of how Java works which I consider acceptable since each language-support documentation isn't meant to expect 0 understanding of the language it's building upon.
(This is with respect to the portion involving CLASSPATH)

I'm happy to explain things 1:1 over some comments but I would like to push back a little on expecting 0 knowledge in the documentation.

fzakaria · 2020-11-02T18:20:05Z

@roberth @doronbehar ping 👍

roberth · 2020-11-02T19:24:02Z

Content looks good, but it isn't incorporated into the manual yet.
Seems like you'll need to rename it to maven.section.md so make can write the xml file

nixpkgs/doc/Makefile

Line 89 in 8b6e981

%.section.xml: %.section.md

and reference it.

nixpkgs/doc/languages-frameworks/index.xml

Line 21 in 29c4fc9

<xi:include href="lua.section.xml" />

Building the manual is documented here https://nixos.org/manual/nixos/stable/index.html#sec-writing-docs-building-the-manual

fzakaria · 2020-11-02T19:37:04Z

@roberth I didn't even know that's how it works -- thanks for teaching me 👍

I've made the changes you requested and confirmed it works.

doronbehar

Content really looks great now! Thanks @fzakaria 🎉.

Only some slight formatting nitpicks, if you don't mind :).

doc/languages-frameworks/maven.section.md

Add nice markdown documentation for how to use mvn2nix plugin and the buildMaven function within nixpkgs. Update doc/languages-frameworks/maven.md Co-authored-by: Robert Hensing <roberth@users.noreply.github.com> Apply suggestions from code review Co-authored-by: Doron Behar <doron.behar@gmail.com> Apply suggestions from code review Co-authored-by: Doron Behar <doron.behar@gmail.com> Apply suggestions from code review Co-authored-by: Doron Behar <doron.behar@gmail.com>

roberth

🚀

doronbehar · 2020-11-03T22:24:50Z

Thanks again @fzakaria 🐳 .

fzakaria · 2020-11-04T03:34:14Z

We did it!

nixos-discourse · 2020-11-04T17:04:16Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/maintained-usable-tooling-for-building-clojure-projects-in-nix/1556/6

nixos-discourse · 2020-11-24T23:34:23Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/gnu-guix-1-2-0-released-2020-blog-gnu-guix/10167/7

fzakaria changed the title ~~documentation: add documentation for maven~~ documentation: add documentation for buildMaven Oct 15, 2020

fzakaria force-pushed the faridzakaria/maven-documentation branch from c439d08 to ee91f64 Compare October 15, 2020 23:38

ofborg bot added the 8.has: documentation label Oct 15, 2020

fzakaria force-pushed the faridzakaria/maven-documentation branch from ee91f64 to ffbb3a8 Compare October 15, 2020 23:44

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 15, 2020

fzakaria force-pushed the faridzakaria/maven-documentation branch from ffbb3a8 to b424e0a Compare October 15, 2020 23:49

doronbehar requested changes Oct 16, 2020

View reviewed changes

roberth reviewed Oct 16, 2020

View reviewed changes

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

fzakaria force-pushed the faridzakaria/maven-documentation branch 2 times, most recently from 1a340ab to e279c12 Compare October 16, 2020 23:07

fzakaria requested review from roberth, doronbehar and volth October 16, 2020 23:07

roberth reviewed Oct 17, 2020

View reviewed changes

doronbehar requested changes Oct 17, 2020

View reviewed changes

fzakaria force-pushed the faridzakaria/maven-documentation branch from e279c12 to 40a3228 Compare October 19, 2020 22:05

fzakaria requested review from roberth and doronbehar October 19, 2020 22:07

roberth reviewed Oct 19, 2020

View reviewed changes

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

fzakaria requested a review from roberth October 19, 2020 22:51

fzakaria force-pushed the faridzakaria/maven-documentation branch from 0c3be23 to 49b7bee Compare October 19, 2020 22:54

fzakaria mentioned this pull request Oct 20, 2020

lib: Add listFilesRecursive function to filesystem #101096

Merged

10 tasks

doronbehar requested changes Oct 22, 2020

View reviewed changes

fzakaria commented Oct 22, 2020

View reviewed changes

fzakaria force-pushed the faridzakaria/maven-documentation branch from 159cf98 to a812da3 Compare October 22, 2020 18:21

fzakaria requested a review from doronbehar October 22, 2020 18:32

doronbehar requested changes Oct 23, 2020

View reviewed changes

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

doc/languages-frameworks/maven.md Outdated Show resolved Hide resolved

fzakaria force-pushed the faridzakaria/maven-documentation branch from b05139d to dcda7ad Compare October 26, 2020 17:29

fzakaria requested a review from doronbehar October 26, 2020 17:29

fzakaria force-pushed the faridzakaria/maven-documentation branch from 914e0da to 3ae7bac Compare November 2, 2020 19:37

doronbehar requested changes Nov 3, 2020

View reviewed changes

fzakaria force-pushed the faridzakaria/maven-documentation branch from e92f82c to b9321ad Compare November 3, 2020 17:23

fzakaria requested a review from doronbehar November 3, 2020 17:23

roberth approved these changes Nov 3, 2020

View reviewed changes

doronbehar approved these changes Nov 3, 2020

View reviewed changes

doronbehar merged commit 80b96cf into NixOS:master Nov 3, 2020

	vendorSha256 = "1879j77k96684wi554rkjxydrj8g3hpp0kvxz03sd8dmwr3lh83j"; <co xml:id='ex-buildGoModule-1' />

	runVend = true; <co xml:id='ex-buildGoModule-2' />

	meta = with lib; {
	description = "Simple command-line snippet manager, written in Go";
	homepage = "https://github.com/knqyf263/pet";
	license = licenses.mit;
	maintainers = with maintainers; [ kalbasit ];
	platforms = platforms.linux ++ platforms.darwin;
	};
	}
	</programlisting>
	</example>

	<para>
	<xref linkend='ex-buildGoModule'/> is an example expression using buildGoModule, the following arguments are of special significance to the function:
	<calloutlist>
	<callout arearefs='ex-buildGoModule-1'>
	<para>
	<varname>vendorSha256</varname> is the hash of the output of the intermediate fetcher derivation.
	</para>
	</callout>
	<callout arearefs='ex-buildGoModule-2'>
	<para>
	<varname>runVend</varname> runs the vend command to generate the vendor directory. This is useful if your code depends on c code and go mod tidy does not include the needed sources to build.
	</para>
	</callout>
	</calloutlist>
	</para>

	buildInputs = [ jdk maven makeWrapper ];
	buildInputs = [ makeWrapper ];


		We will use the same repository we built above (either _double invocation_ or _buildMaven_) to setup a CLASSPATH for our JAR.

		### CLASSPATH

documentation: add documentation for buildMaven #100660

documentation: add documentation for buildMaven #100660

Conversation

fzakaria commented Oct 15, 2020 • edited

Motivation for this change

Things done

doronbehar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzakaria commented Oct 16, 2020

roberth commented Oct 16, 2020

fzakaria commented Oct 16, 2020 • edited

doronbehar left a comment

Choose a reason for hiding this comment

fzakaria commented Oct 19, 2020

nixos-discourse commented Oct 21, 2020

doronbehar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzakaria commented Oct 22, 2020

fzakaria left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doronbehar commented Oct 23, 2020 • edited

doronbehar commented Oct 23, 2020

fzakaria commented Oct 26, 2020

fzakaria commented Nov 2, 2020

roberth commented Nov 2, 2020

fzakaria commented Nov 2, 2020 • edited

doronbehar left a comment

Choose a reason for hiding this comment

roberth left a comment

Choose a reason for hiding this comment

doronbehar commented Nov 3, 2020

fzakaria commented Nov 4, 2020

nixos-discourse commented Nov 4, 2020

nixos-discourse commented Nov 24, 2020

fzakaria commented Oct 15, 2020 •

edited

fzakaria commented Oct 16, 2020 •

edited

doronbehar commented Oct 23, 2020 •

edited

fzakaria commented Nov 2, 2020 •

edited