Re: (commons-compress) branch master updated: Removed the runtime dependency on commons-lang3

2024-10-24 Thread Gary Gregory
-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

2024-10-24 Thread sebb
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

2024-10-24 Thread Emmanuel Bourg

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

2024-10-24 Thread Gary Gregory
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

2024-10-24 Thread Emmanuel Bourg

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