You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Gary D. Gregory" <gg...@apache.org> on 2024/11/01 13:22:19 UTC
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
There has been no revert from the committer since the -1 on October 24, so I will revert. This affects 11 Java files. This will revert the copy and pasting of code patterns over and over.
Gary
On 2024/10/24 21:17:41 Emmanuel Bourg wrote:
> Le 24/10/2024 à 21:05, Gary Gregory a écrit :
>
> > Uh, no, don't be disingenuous by showing 3 lines, the commit is many pages
> > long of diff. All of it is -1 as it increases the code complexity, increase
> > code duplication, and adds the anti-pattern of throwing RuntimeException
> > more than once.
>
> My commit message could have been clearer, it actually reverted the
> commit 87e898fa [1] and replaced the reflection code with the snippet I
> mentioned. So the code doesn't come out of my hat, that's what
> commons-compress already had for years before.
>
> Adding a dependency such as commons-lang3 has a significant impact for
> the users. In my case it broke an application that was using
> commons-compress but didn't have commons-lang3 on the classpath.
>
> I understand your desire to reuse as much code as possible to ease the
> maintenance, but in this case the user's convenience outweighs the
> maintainer's convenience. commons-lang3 doesn't bring enough value in
> this case to justify its addition to the runtime dependencies.
>
> Emmanuel Bourg
>
> [1] https://github.com/apache/commons-compress/commit/87e898fa
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 17/11/2024 à 23:57, sebb a écrit :
> It seems to me perfectly OK to question a commit regardless of when it happened.
>
> I think Emmanuel is within his rights to question whether adding the
> dependency was the right thing to do.
And I think I'm not the only one surprised by the new dependencies. I
stumbled on this commit from Torsten Curdt, another Commons Compress
contributor, in its jdeb project. The comment in the pom.xml is quite clear:
https://github.com/tcurdt/jdeb/commit/743e5d7f
From 743e5d7f387098c1d1190189f7636045a0fe004a Mon Sep 17 00:00:00 2001
From: Torsten Curdt <tc...@vafer.org>
Date: Sun, 2 Jun 2024 21:30:34 +0200
Subject: [PATCH] working around a dep issue with compress
---
pom.xml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/pom.xml b/pom.xml
index 671f4bfce..7ea500b60 100644
--- a/pom.xml
+++ b/pom.xml
@@ -146,6 +146,12 @@
<artifactId>commons-compress</artifactId>
<version>1.26.2</version>
</dependency>
+ <dependency>
+ <!-- this should not be required but somehow compress introduced
the dep -->
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ <version>3.14.0</version>
+ </dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcpg-jdk15on</artifactId>
@@ -357,6 +363,7 @@
<include>org.apache.commons:commons-compress</include>
<include>commons-codec:commons-codec</include>
<include>commons-io:commons-io</include>
+ <include>org.apache.commons:commons-lang3</include>
<include>org.bouncycastle:bcpg-jdk15on</include>
<include>org.bouncycastle:bcprov-jdk15on</include>
<include>org.tukaani:xz</include>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by sebb <se...@gmail.com>.
On Sun, 17 Nov 2024 at 14:01, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi Niall,
>
> I don't think we are at an impasse: The PMC has approved release after
> release, as recently as 1.27.1 three months ago. Emmanuel has not voted on
> a Compress release in over eight years, since 1.11, that's 20 release votes
> ignored.
So what?
People come and go in Commons, and may change focus.
> Doing drive-by copypasta commits long after releases wanting a time machine
> is not OK IMO.
It seems to me perfectly OK to question a commit regardless of when it happened.
I think Emmanuel is within his rights to question whether adding the
dependency was the right thing to do.
> Gary
>
> On Sat, Nov 16, 2024, 3:42 PM Niall Pemberton <ni...@gmail.com>
> wrote:
>
> > On Fri, 1 Nov 2024 at 13:22, Gary D. Gregory <gg...@apache.org> wrote:
> >
> > > There has been no revert from the committer since the -1 on October 24,
> > so
> > > I will revert. This affects 11 Java files. This will revert the copy and
> > > pasting of code patterns over and over.
> >
> >
> > Seems we’re now at an impasses as Emmanuel has vetoed the commits
> > introducing the dependencies earlier in the year. So are you now going to
> > revert those?
> >
> > Niall
> >
> >
> >
> >
> > >
> > > Gary
> > >
> > > On 2024/10/24 21:17:41 Emmanuel Bourg wrote:
> > > > Le 24/10/2024 à 21:05, Gary Gregory a écrit :
> > > >
> > > > > Uh, no, don't be disingenuous by showing 3 lines, the commit is many
> > > pages
> > > > > long of diff. All of it is -1 as it increases the code complexity,
> > > increase
> > > > > code duplication, and adds the anti-pattern of throwing
> > > RuntimeException
> > > > > more than once.
> > > >
> > > > My commit message could have been clearer, it actually reverted the
> > > > commit 87e898fa [1] and replaced the reflection code with the snippet I
> > > > mentioned. So the code doesn't come out of my hat, that's what
> > > > commons-compress already had for years before.
> > > >
> > > > Adding a dependency such as commons-lang3 has a significant impact for
> > > > the users. In my case it broke an application that was using
> > > > commons-compress but didn't have commons-lang3 on the classpath.
> > > >
> > > > I understand your desire to reuse as much code as possible to ease the
> > > > maintenance, but in this case the user's convenience outweighs the
> > > > maintainer's convenience. commons-lang3 doesn't bring enough value in
> > > > this case to justify its addition to the runtime dependencies.
> > > >
> > > > Emmanuel Bourg
> > > >
> > > > [1] https://github.com/apache/commons-compress/commit/87e898fa
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > > For additional commands, e-mail: dev-help@commons.apache.org
> > > >
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by Gary Gregory <ga...@gmail.com>.
Hi Niall,
I don't think we are at an impasse: The PMC has approved release after
release, as recently as 1.27.1 three months ago. Emmanuel has not voted on
a Compress release in over eight years, since 1.11, that's 20 release votes
ignored.
Doing drive-by copypasta commits long after releases wanting a time machine
is not OK IMO.
Gary
On Sat, Nov 16, 2024, 3:42 PM Niall Pemberton <ni...@gmail.com>
wrote:
> On Fri, 1 Nov 2024 at 13:22, Gary D. Gregory <gg...@apache.org> wrote:
>
> > There has been no revert from the committer since the -1 on October 24,
> so
> > I will revert. This affects 11 Java files. This will revert the copy and
> > pasting of code patterns over and over.
>
>
> Seems we’re now at an impasses as Emmanuel has vetoed the commits
> introducing the dependencies earlier in the year. So are you now going to
> revert those?
>
> Niall
>
>
>
>
> >
> > Gary
> >
> > On 2024/10/24 21:17:41 Emmanuel Bourg wrote:
> > > Le 24/10/2024 à 21:05, Gary Gregory a écrit :
> > >
> > > > Uh, no, don't be disingenuous by showing 3 lines, the commit is many
> > pages
> > > > long of diff. All of it is -1 as it increases the code complexity,
> > increase
> > > > code duplication, and adds the anti-pattern of throwing
> > RuntimeException
> > > > more than once.
> > >
> > > My commit message could have been clearer, it actually reverted the
> > > commit 87e898fa [1] and replaced the reflection code with the snippet I
> > > mentioned. So the code doesn't come out of my hat, that's what
> > > commons-compress already had for years before.
> > >
> > > Adding a dependency such as commons-lang3 has a significant impact for
> > > the users. In my case it broke an application that was using
> > > commons-compress but didn't have commons-lang3 on the classpath.
> > >
> > > I understand your desire to reuse as much code as possible to ease the
> > > maintenance, but in this case the user's convenience outweighs the
> > > maintainer's convenience. commons-lang3 doesn't bring enough value in
> > > this case to justify its addition to the runtime dependencies.
> > >
> > > Emmanuel Bourg
> > >
> > > [1] https://github.com/apache/commons-compress/commit/87e898fa
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by Niall Pemberton <ni...@gmail.com>.
On Fri, 1 Nov 2024 at 13:22, Gary D. Gregory <gg...@apache.org> wrote:
> There has been no revert from the committer since the -1 on October 24, so
> I will revert. This affects 11 Java files. This will revert the copy and
> pasting of code patterns over and over.
Seems we’re now at an impasses as Emmanuel has vetoed the commits
introducing the dependencies earlier in the year. So are you now going to
revert those?
Niall
>
> Gary
>
> On 2024/10/24 21:17:41 Emmanuel Bourg wrote:
> > Le 24/10/2024 à 21:05, Gary Gregory a écrit :
> >
> > > Uh, no, don't be disingenuous by showing 3 lines, the commit is many
> pages
> > > long of diff. All of it is -1 as it increases the code complexity,
> increase
> > > code duplication, and adds the anti-pattern of throwing
> RuntimeException
> > > more than once.
> >
> > My commit message could have been clearer, it actually reverted the
> > commit 87e898fa [1] and replaced the reflection code with the snippet I
> > mentioned. So the code doesn't come out of my hat, that's what
> > commons-compress already had for years before.
> >
> > Adding a dependency such as commons-lang3 has a significant impact for
> > the users. In my case it broke an application that was using
> > commons-compress but didn't have commons-lang3 on the classpath.
> >
> > I understand your desire to reuse as much code as possible to ease the
> > maintenance, but in this case the user's convenience outweighs the
> > maintainer's convenience. commons-lang3 doesn't bring enough value in
> > this case to justify its addition to the runtime dependencies.
> >
> > Emmanuel Bourg
> >
> > [1] https://github.com/apache/commons-compress/commit/87e898fa
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by Richard Zowalla <ri...@zowalla.com>.
From the sideline: I am also concerned about adding dependendcies in low level libraries for replacing only a few lines of code.
From ASF policy perspective, the veto (-1) requires a technical explanation, cf. https://www.apache.org/foundation/voting.html#Veto
"To prevent vetoes from being used capriciously, the voter must provide with the veto a technical justification showing why the change is bad (opens a security exposure, negatively affects performance, etc. ). A veto without a justification is invalid and has no weight."
So IMHO asking for an explanation about the actual CnP is totally fine.
Gruß
Richard
Am 1. November 2024 15:22:07 MEZ schrieb Emmanuel Bourg <eb...@apache.org>:
>Hi Gary,
>
>Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :
>> There has been no revert from the committer since the -1 on October 24, so I will revert. This affects 11 Java files. This will revert the copy and pasting of code patterns over and over.
>
>I don't understand your "copy pasting" argument, the commit replaced exactly 8 occurrences of:
>
> int[] array = ArrayFill.fill(new int[length], value);
>
>with:
>
> int[] array = new int[length];
> Arrays.fill(array, value);
>
>
>It also replaced:
>
> SystemProperties.getOsName();
>
>with:
>
> System.getProperty("os.name");
>
>
>And two reflexive calls such as:
>
> FieldUtils.readField(object, fieldName, true);
>
>with:
>
> Field field = InputStream.class.getDeclaredField(fieldName);
> field.setAccessible(true);
> return field.get(object);
>
>
>These are not unreasonable substitutions to avoid a 660KB dependency. You make it sound like I copy-pasted tons of lines, but that's only 6 more lines of code overall.
>
>
>Let me put this differently, I'm -1 to your commits bf50b7d9 [1], 87e898fa [2] and 09031c91 [3] from earlier this year that introduced the dependency on Commons Lang in the first place. I'm especially unhappy with the first one, a mix of unrelated changes confusingly labeled as "Internal refactoring", where the dependency sneaked into without notice, neither mentioned in the commit message nor in the changelog. This dependency is an unnecessary bloat.
>
>I'm also concerned about the extra dependency on Commons Codec and Commons IO, also added this year. I'd like to return to the state of Commons Compress 1.25 with *zero* runtime dependency (besides the optional ones required for the various compression algorithms supported). For Codec and IO I suggest shading the classes as a compromise.
>
>Emmanuel Bourg
>
>[1] https://github.com/apache/commons-compress/commit/bf50b7d9
>[2] https://github.com/apache/commons-compress/commit/87e898fa
>[3] https://github.com/apache/commons-compress/commit/09031c91
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>For additional commands, e-mail: dev-help@commons.apache.org
>
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by sebb <se...@gmail.com>.
On Fri, 1 Nov 2024 at 14:22, Emmanuel Bourg <eb...@apache.org> wrote:
>
> Hi Gary,
>
> Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :
> > There has been no revert from the committer since the -1 on October 24, so I will revert. This affects 11 Java files. This will revert the copy and pasting of code patterns over and over.
>
> I don't understand your "copy pasting" argument, the commit replaced
> exactly 8 occurrences of:
>
> int[] array = ArrayFill.fill(new int[length], value);
>
> with:
>
> int[] array = new int[length];
> Arrays.fill(array, value);
>
>
> It also replaced:
>
> SystemProperties.getOsName();
>
> with:
>
> System.getProperty("os.name");
>
>
> And two reflexive calls such as:
>
> FieldUtils.readField(object, fieldName, true);
>
> with:
>
> Field field = InputStream.class.getDeclaredField(fieldName);
> field.setAccessible(true);
> return field.get(object);
>
>
> These are not unreasonable substitutions to avoid a 660KB dependency.
> You make it sound like I copy-pasted tons of lines, but that's only 6
> more lines of code overall.
>
>
> Let me put this differently, I'm -1 to your commits bf50b7d9 [1],
> 87e898fa [2] and 09031c91 [3] from earlier this year that introduced the
> dependency on Commons Lang in the first place. I'm especially unhappy
> with the first one, a mix of unrelated changes confusingly labeled as
> "Internal refactoring", where the dependency sneaked into without
> notice, neither mentioned in the commit message nor in the changelog.
> This dependency is an unnecessary bloat.
+1
> I'm also concerned about the extra dependency on Commons Codec and
> Commons IO, also added this year. I'd like to return to the state of
> Commons Compress 1.25 with *zero* runtime dependency (besides the
> optional ones required for the various compression algorithms
> supported). For Codec and IO I suggest shading the classes as a compromise.
>
> Emmanuel Bourg
>
> [1] https://github.com/apache/commons-compress/commit/bf50b7d9
> [2] https://github.com/apache/commons-compress/commit/87e898fa
> [3] https://github.com/apache/commons-compress/commit/09031c91
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Posted by Emmanuel Bourg <eb...@apache.org>.
Hi Gary,
Le 01/11/2024 à 14:22, Gary D. Gregory a écrit :
> There has been no revert from the committer since the -1 on October 24, so I will revert. This affects 11 Java files. This will revert the copy and pasting of code patterns over and over.
I don't understand your "copy pasting" argument, the commit replaced
exactly 8 occurrences of:
int[] array = ArrayFill.fill(new int[length], value);
with:
int[] array = new int[length];
Arrays.fill(array, value);
It also replaced:
SystemProperties.getOsName();
with:
System.getProperty("os.name");
And two reflexive calls such as:
FieldUtils.readField(object, fieldName, true);
with:
Field field = InputStream.class.getDeclaredField(fieldName);
field.setAccessible(true);
return field.get(object);
These are not unreasonable substitutions to avoid a 660KB dependency.
You make it sound like I copy-pasted tons of lines, but that's only 6
more lines of code overall.
Let me put this differently, I'm -1 to your commits bf50b7d9 [1],
87e898fa [2] and 09031c91 [3] from earlier this year that introduced the
dependency on Commons Lang in the first place. I'm especially unhappy
with the first one, a mix of unrelated changes confusingly labeled as
"Internal refactoring", where the dependency sneaked into without
notice, neither mentioned in the commit message nor in the changelog.
This dependency is an unnecessary bloat.
I'm also concerned about the extra dependency on Commons Codec and
Commons IO, also added this year. I'd like to return to the state of
Commons Compress 1.25 with *zero* runtime dependency (besides the
optional ones required for the various compression algorithms
supported). For Codec and IO I suggest shading the classes as a compromise.
Emmanuel Bourg
[1] https://github.com/apache/commons-compress/commit/bf50b7d9
[2] https://github.com/apache/commons-compress/commit/87e898fa
[3] https://github.com/apache/commons-compress/commit/09031c91
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org