[prev in list] [next in list] [prev in thread] [next in thread] 

List:       jakarta-commons-dev
Subject:    =?utf-8?q?Re=3A_=5BPR=5D_COMPRESS-653=3A_Fix_split_archive_updating_incorrec?= =?utf-8?q?t_file_=5Bc
From:       kvr000_(via_GitHub) <git () apache ! org>
Date:       2023-12-30 22:15:14
Message-ID: PR_kwDOACdhIc5i2QMj-7b4196c7-5a28-41f5-a662-fdda60afb88c () gitbox ! apache ! org
[Download RAW message or body]


kvr000 commented on PR #455:
URL: https://github.com/apache/commons-compress/pull/455#issuecomment-1872616449

   > * Big picture: I am considering adding some random IO classes to Apache Commons \
IO like Apache PDFBox has, see \
https://github.com/apache/pdfbox/tree/trunk/io/src/main/java/org/apache/pdfbox/io  
   That totally makes sense, each library provides its own IOUtils currently.  I'd \
suggest few things:  1. FileChannel is typically more meaningful than \
SeekableByteChannel.  Even here in ZipFile and related, the SeekableByteChannel is \
used in a sense of FileChannel.  I provided fix for this a while ago which allowed \
lock-free reading from multiple ZipFile entries in parallel but there is the same \
work in other parts.  2. The JDK unfortunately misses interface for position_read and \
position_write methods which makes custom implementations more tricky.  I was \
addressing this issue in https://github.com/dryuf/dryuf-bigio (FlatChannel), you may \
consider this for inspiration when working on commons-io stuff.  3. The PdfBox in my \
opinion is quite complicated and may suffer from diamond inheritance problem.  The \
above may be more straightforward and more efficient.  
   
   > * In light of this, it is good that the random IO classes you've added are \
package-protected. This will let us possibly replace this implementation with this \
very code ported to Commons IO or with code from PDFBox ported to Commons IO. We do \
not want Commons Compress to depend on PDFBox, especially since PDFBox already \
depends on Commons IO.  > * To reduce the new public API surface, please move the new \
IOUtils method into package-protected place in the `zip` package.  
   That was done.
   
   > * I'll study all of this some more and see what is generic enough to add and \
reuse from Commons IO.  
   It may require more work to make it more generic.  For now the package private is \
probably better choice.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[prev in list] [next in list] [prev in thread] [next in thread]