From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 18 15:56:58 2022 Received: (at 56054) by debbugs.gnu.org; 18 Jun 2022 19:56:59 +0000 Received: from localhost ([127.0.0.1]:50060 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o2eZ0-0001sb-KC for submit@debbugs.gnu.org; Sat, 18 Jun 2022 15:56:58 -0400 Received: from lepiller.eu ([89.234.186.109]:40896) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o2eYx-0001sP-9j for 56054@debbugs.gnu.org; Sat, 18 Jun 2022 15:56:57 -0400 Received: from lepiller.eu (localhost [127.0.0.1]) by lepiller.eu (OpenSMTPD) with ESMTP id f7bd797c; Sat, 18 Jun 2022 19:56:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=lepiller.eu; h=date:from :to:cc:subject:message-id:in-reply-to:references:mime-version :content-type:content-transfer-encoding; s=dkim; bh=wgsmT4NLQJFr G23LXgyDO9TdjAuWVy94g9Ldaaj1iBM=; b=lYPwdoVSnM2ZRwf2oKafpkb/QfCf 78GfmOuWvutuTnrLShSB8dZmXFu8hmlMAVu+G50GN8pEiBn6fVe5eh1MnIRvwNVG Kip6Txl6esdu7Ccu/8vjlSF2ftXShQEby/npv6+EYZt7/ySNtR/kaMtdN0q4tskU 7nLXQdqOgIVQ7X2EbveuifMUDkE/PJ4e/JSwX9j+UC65sp0/u2pQemv5z0ZdnE1J aJCiVNYy/L9RfI3HTs5PnUv1RwgCJTVjAUZ0fhoGzDc0XujYqqUDKq9QuKPJ90f9 J3hoDQhWPnNexpPE2sP/5y0GDfgavYiq79CvNfGEqbspzHCaRRz7n92NQg== Received: by lepiller.eu (OpenSMTPD) with ESMTPSA id 1921c05a (TLSv1.3:AEAD-AES256-GCM-SHA384:256:NO); Sat, 18 Jun 2022 19:56:53 +0000 (UTC) Date: Sat, 18 Jun 2022 21:56:51 +0200 From: Julien Lepiller To: "Artyom V. Poptsov" Subject: Re: [bug#56054] [PATCH] gnu: Add maven-shared-invoker Message-ID: <20220618215647.4e25fd30@sybil.lepiller.eu> In-Reply-To: <87a6aavx17.fsf@gmail.com> References: <87a6aavx17.fsf@gmail.com> X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 56054 Cc: 56054@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) 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" a =C3=A9crit : > +(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=C2=A0guess). > + (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" > +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" > +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?