[bug#56054] [PATCH] gnu: Add maven-shared-invoker

[Julien Lepiller]

Thanks for the patch! It's mostly good, but I have some comments below
:)

Le Sat, 18 Jun 2022 17:58:12 +0300,
"Artyom V. Poptsov" <poptsov.artyom@gmail.com> a écrit :

> +(define-public maven-shared-invoker
> +  (package
> +    (name "maven-shared-invoker")
> +    (version "3.2.0")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "mirror://apache/maven/shared/"
> +                                  "maven-invoker-" version
> "-source-release.zip"))
> +              (sha256
> +               (base32
> +
> "0yhgxvwpmyfhqaksdfmj9c4ml4pj60gnin8bq1a92ximf1dyyjyc"))
> +              (patches
> +               (search-patches
> +                "maven-shared-invoker-exception-handler-fix.patch"
> +                "maven-shared-invoker-rename-test-classes.patch"))))
> +    (build-system ant-build-system)
> +    (arguments
> +     `(#:jar-name "maven-shared-invoker.jar"
> +       #:source-dir "src/main/java"
> +       #:tests? #f))                      ; Tests require Maven
> itself

How so? Tests are usually just junit tests and it's easy to run them.
How are they so different from the usual tests?

If it really requires maven, have you tried building it with the
maven-build-system? There's a way to remove plugins from the pom file,
so maven doesn't complain. The pom file doesn't look too complex, so I
think it could work.

> +    (propagated-inputs
> +     (list maven-parent-pom-35))

Yes you should propagate the parent, but that's only because maven
needs it when it reads this package's pom file. So, please keep it and
install this package from its pom file, like the others :)

> +    (native-inputs
> +     (list unzip
> +           maven-surefire-plugin
> +           java-javax-inject
> +           java-junit))

I'm surprised here you need maven-surefire-plugin. What is it used for
exactly? From my understanding it can't be called outside of maven, and
we don't use maven to install this package.

The pom file lists java-javax-inject as a normal dependency, so it
should be propagated instead. The pom file also lists
maven-shared-utils. Is it needed? If so please add it to the propagated
inputs, otherwise fix the pom file (with a patch to upstream I guess).

> +    (home-page
> "https://maven.apache.org/shared/maven-invoker/index.html";)
> +    (synopsis "Invoke Maven programmatically")

> Sep 17 00:00:00 2001 +From: "Artyom V. Poptsov"
> <poptsov.artyom@gmail.com> +Date: Tue, 14 Jun 2022 23:53:13 +0300
> +Subject: [PATCH 1/2] MavenCommandLineBuilder: Fix exception handling
> +
> +*
> src/main/java/org/apache/maven/shared/invoker/MavenCommandLineBuilder.java
> +  (setGoals): Catch 'Exception' instead of 'CommandLineException' as
> +  'CommandLineException' is never thrown in the "try" block.
> +---
> + .../apache/maven/shared/invoker/MavenCommandLineBuilder.java    | 2
> +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git

This looks like a simple patch, but I don't understand why it's needed.
Is this a fix from upstream? Did you create it? For what reason?

> @@ -0,0 +1,126 @@ +From 4bce3183b25c44ab406c2f4d8541a0a520b15a3d Mon
> Sep 17 00:00:00 2001 +From: "Artyom V. Poptsov"
> <poptsov.artyom@gmail.com> +Date: Wed, 15 Jun 2022 07:09:29 +0300
> +Subject: [PATCH 2/2] test: Rename some classes to avoid name
> conflicts +
> +*

I'm lost. What's happening in this patch? Why do you need it
(especially since you couldn't run the tests anyway)? Is this a problem
with upstream, or some issue you encountered because of what Guix is
doing?