Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
-1. No copypasta, please. I want to reduce code patterns that are duplicated all over the place. Gary On Thu, Oct 24, 2024 at 1:15 PM wrote: > This is an automated email from the ASF dual-hosted git repository. > > ebourg pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/commons-compress.git > > > The following commit(s) were added to refs/heads/master by this push: > new 23877f821 Removed the runtime dependency on commons-lang3 > 23877f821 is described below > > commit 23877f8216243df6b2f1dddac2a02b8403d2435d > Author: Emmanuel Bourg > AuthorDate: Thu Oct 24 19:13:47 2024 +0200 > > Removed the runtime dependency on commons-lang3 > --- > pom.xml| 3 +-- > src/changes/changes.xml| 1 + > .../compress/archivers/tar/TarArchiveEntry.java| 3 +-- > .../archivers/tar/TarArchiveOutputStream.java | 5 ++-- > .../commons/compress/archivers/zip/BinaryTree.java | 5 ++-- > .../compressors/deflate64/HuffmanDecoder.java | 4 ++-- > .../compressors/lz77support/LZ77Compressor.java| 6 ++--- > .../compress/harmony/unpack200/Archive.java| 2 +- > .../harmony/unpack200/Pack200UnpackerAdapter.java | 27 > -- > .../archivers/tar/TarArchiveOutputStreamTest.java | 4 ++-- > .../archivers/zip/ZipArchiveInputStreamTest.java | 5 ++-- > .../compress/archivers/zip/ZipFileTest.java| 4 ++-- > .../lz4/BlockLZ4CompressorOutputStreamTest.java| 5 ++-- > 13 files changed, 40 insertions(+), 34 deletions(-) > > diff --git a/pom.xml b/pom.xml > index 658864a0e..3ef7c6ebc 100644 > --- a/pom.xml > +++ b/pom.xml > @@ -65,8 +65,6 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, > arj. >javax.crypto.*;resolution:=optional, >org.apache.commons.io;resolution:=optional, >org.apache.commons.io.*;resolution:=optional, > - org.apache.commons.lang3;resolution:=optional, > - org.apache.commons.lang3.reflect;resolution:=optional, >org.apache.commons.codec;resolution:=optional, >org.apache.commons.codec.digest;resolution:=optional, >* > @@ -213,6 +211,7 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, > 7z, arj. >org.apache.commons >commons-lang3 >3.17.0 > + test > > > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index cb7711e3d..4bfd3dd0f 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -48,6 +48,7 @@ The type attribute can be add,update,fix,remove. > > due-to="Richard Blank, Gary Gregory">Better exception messages in > SeekableInMemoryByteChannel. >ZipArchiveOutputStream.addRawArchiveEntry() should check > is2PhaseSource #571. > + The runtime > dependency on commons-lang3 has been removed > > >Bump org.apache.commons:commons-parent from 72 to 78 #563, #567, > #574, #582, #587, #595. > diff --git > a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java > b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java > index e02eeb3a2..ad64263d9 100644 > --- > a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java > +++ > b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java > @@ -53,7 +53,6 @@ import org.apache.commons.compress.utils.IOUtils; > import org.apache.commons.compress.utils.ParsingUtils; > import org.apache.commons.compress.utils.TimeUtils; > import org.apache.commons.io.file.attribute.FileTimes; > -import org.apache.commons.lang3.SystemProperties; > > /** > * An entry in a https://www.gnu.org/software/tar/manual/html_node/Standard.html";>Tar > archive. > @@ -229,7 +228,7 @@ public class TarArchiveEntry implements ArchiveEntry, > TarConstants, EntryStreamO > */ > private static String normalizeFileName(String fileName, final > boolean preserveAbsolutePath) { > if (!preserveAbsolutePath) { > -final String property = SystemProperties.getOsName(); > +final String property = System.getProperty("os.name"); > if (property != null) { > final String osName = property.toLowerCase(Locale.ROOT); > > diff --git > a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java > b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java > index 4f3588d9b..f84064772 100644 > --- > a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java > +++ > b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java > @@ -32,6 +32,7 @@ import java.nio.file.LinkOption; > import java.nio.file.Path; > import java.nio.file.attribute.FileTime; > import java.time.Instant; > +import java.util.Arrays; > import java.util.HashMap; > import java.util.Map; > > @@ -43,7 +44,6 @@ import org.apache.commo
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
It looks to me like there are several different changes here. It seems to me that using standard library methods for ArrayFill and SystemProperties is a sensible change. I'm not sure about the other change as it does affect the behaviour, and does look more complicated. Perhaps it could be recast? On Thu, 24 Oct 2024 at 20:05, Gary Gregory wrote: > > Hello Emmanuel, > > 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. > > Please revert. > > Gary > > On Thu, Oct 24, 2024, 2:05 PM Emmanuel Bourg wrote: > > > Hi Gary, > > > > Le 24/10/2024 à 19:25, Gary Gregory a écrit : > > > > > No copypasta, please. I want to reduce code patterns that are duplicated > > > all over the place. > > > > What do you mean by "copypasta"? Are you concerned about the maintenance > > cost of the 3 lines of reflection code appearing twice in > > Pack200UnpackerAdapter? > > > >Field field = FilterInputStream.class.getDeclaredField("in"); > >field.setAccessible(true); > >return (InputStream) field.get(filterInputStream); > > > > Emmanuel Bourg > > > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
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-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Hello Emmanuel, 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. Please revert. Gary On Thu, Oct 24, 2024, 2:05 PM Emmanuel Bourg wrote: > Hi Gary, > > Le 24/10/2024 à 19:25, Gary Gregory a écrit : > > > No copypasta, please. I want to reduce code patterns that are duplicated > > all over the place. > > What do you mean by "copypasta"? Are you concerned about the maintenance > cost of the 3 lines of reflection code appearing twice in > Pack200UnpackerAdapter? > >Field field = FilterInputStream.class.getDeclaredField("in"); >field.setAccessible(true); >return (InputStream) field.get(filterInputStream); > > Emmanuel Bourg > > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3
Hi Gary, Le 24/10/2024 à 19:25, Gary Gregory a écrit : No copypasta, please. I want to reduce code patterns that are duplicated all over the place. What do you mean by "copypasta"? Are you concerned about the maintenance cost of the 3 lines of reflection code appearing twice in Pack200UnpackerAdapter? Field field = FilterInputStream.class.getDeclaredField("in"); field.setAccessible(true); return (InputStream) field.get(filterInputStream); Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org