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

Previous Next

To reply to this bug, email your comments to 56054 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Artyom V. Poptsov" <poptsov.artyom <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add maven-shared-invoker
Date: Sat, 18 Jun 2022 17:58:12 +0300

[Message part 1 (text/plain, inline)]

Hello,

this patch adds Apache Maven Invoker[1] under the name
'maven-shared-invoker'.

[0001-gnu-Add-maven-shared-invoker.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

- Artyom

References:
1. https://maven.apache.org/shared/maven-invoker/index.html

-- 
Artyom "avp" Poptsov <poptsov.artyom <at> gmail.com>
Home page: https://memory-heap.org/~avp/
CADR Hackerspace co-founder: https://cadrspace.ru/
GPG: D0C2 EAC1 3310 822D 98DE  B57C E9C5 A2D9 0898 A02F

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 56054 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: "Artyom V. Poptsov" <poptsov.artyom <at> gmail.com>
Cc: 56054 <at> debbugs.gnu.org
Subject: Re: [bug#56054] [PATCH] gnu: Add maven-shared-invoker
Date: Sat, 18 Jun 2022 21:56:51 +0200

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 <at> 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 <at> 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 <at> 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?

Message #11 received at 56054 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: "Artyom V. Poptsov" <poptsov.artyom <at> gmail.com>, 56054 <at> debbugs.gnu.org
Subject: Re: bug#56054: [PATCH] gnu: Add maven-shared-invoker
Date: Thu, 04 Aug 2022 11:11:49 +0200

Hey Artyom,

Did you have a chance to look at Julien’s comments below?

Thanks,
Ludo’.

Julien Lepiller <julien <at> lepiller.eu> skribis:

> 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 <at> 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 <at> 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 <at> 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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.