You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Kristian Rosenvold <kr...@apache.org> on 2014/12/22 16:33:57 UTC

[compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

There are quite a few extension points in this class that make
changing it really hard.

I just committed  r1647329 which basically duplicates some code from
this class into another class. As much as I hate duplication, I wasn't
able to achieve what I wanted to without A) breaking some extension
capability of the existing class or B) duplicating the code.

We are not talking about the "bread and butter" usage of the class
here but rather intricate extensions I'm unsure of how many people
use. If it was up to me I'd break compatibility slightly an make a
JIRA that explains how to migrate forwards and hence move the class to
a somewhat better state. As things are right now I retained
compatibility by not changing the class itself.

WDYT ?

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2015-01-02, Kristian Rosenvold wrote:

> All fixed :)

Thanks a lot!

       Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@gmail.com>.
All fixed :)

I also gave up on the ParallelScatterZipCreator#createDeferred and
followed your suggestion.

Kristian


2015-01-02 16:17 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> Re backwards compatibility: ZipArchiveOutputStream's deflate method has
> been protected and now has gone.
>
> On 2014-12-31, Kristian Rosenvold wrote:
>
>> On a related note, I just added the *last* substantial change I intend
>> to do. I will do a tweak or two, and I'm sure that last class will
>> trigger a truckload of comments, even just the name of the class:)
>
> I'm notoriously bad at names, so I won't try to color that bikeshed.
>
> I'll comment on the commit itself.
>
>> Things on my todolist for c-compress:
>> A) Make a way to remove tempfiles written by
>> ScatterGatherBackingStore.
>
> +1
>
> Could you please also add a few sentences to
> <http://commons.apache.org/proper/commons-compress/zip.html>?
>
> Thanks!
>
>         Stefan
>
> ---------------------------------------------------------------------
> 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: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
Re backwards compatibility: ZipArchiveOutputStream's deflate method has
been protected and now has gone.

On 2014-12-31, Kristian Rosenvold wrote:

> On a related note, I just added the *last* substantial change I intend
> to do. I will do a tweak or two, and I'm sure that last class will
> trigger a truckload of comments, even just the name of the class:)

I'm notoriously bad at names, so I won't try to color that bikeshed.

I'll comment on the commit itself.

> Things on my todolist for c-compress:
> A) Make a way to remove tempfiles written by
> ScatterGatherBackingStore.

+1

Could you please also add a few sentences to
<http://commons.apache.org/proper/commons-compress/zip.html>?

Thanks!

        Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@apache.org>.
2014-12-31 11:50 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2014-12-30, Kristian Rosenvold wrote:
>> The whole ZipArchiveOutputStream class reminds me of a few of the
>> 3000+ LOC java classes I refactored in maven core; sometimes the only
>> acceptable solution is to extract *all* the logic into other classes
>> and just make the "old" class keep instantiate all the fields and pass
>> them on to the underlying "new" classes with cleaner
>> responsibilities. I'm not really saying this should be done, but the
>> class has that "distinct" feeling :)
>
> Well, when I started it more than thirteen years ago (had to look that
> up :-) it was only ~600 lines and then it grew from that.

Tell me about it. I get serious itches related to such classes :) But
not right now. Must n-o-t shave more yak.

>
> I'm not trying to defend anything, the class has outgrown its initial
> scope by far and backwards compatibility constraints have made it more
> or less impossible to refactor it properly.  This is one reason why I
> started the compress antlib as the same was true for Ant's <zip> task
> family.  Some day I'll find time to revive the currently dormant
> compress 2.0 effort ...

I am really not sure big breaks in compatibility are ever worth it. So
that means we just have to be even more clever :)

On a related note, I just added the *last* substantial change I intend
to do. I will do a tweak or two, and I'm sure that last class will
trigger a truckload of comments, even just the name of the class:)

The remaining stuff in
https://github.com/krosenvold/commons-compress/tree/trunk will now go
to plexus. Take a look at "ConcurrentJarCreator" in my git repo for a
full-scale sample of how to make a zip file.

Things on my todolist for c-compress:
A) Make a way to remove tempfiles written by
ScatterGatherBackingStore. I suspect this will involve renaming
existing "close" method to something like "reverse" and making close
actually "close" (and delete) everything.
B) Optimize LFH generation. I want this in the concurrent scatter
part. The problem with this is that to do this I will need to make a
thread-safe ZipArchiveEntry implementation. Which I have been trying
to avoid. But there's no way around that.

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-30, Kristian Rosenvold wrote:

> I fixed all comments and reinstated the protected Deflater.

Thanks, looks good.

> The whole ZipArchiveOutputStream class reminds me of a few of the
> 3000+ LOC java classes I refactored in maven core; sometimes the only
> acceptable solution is to extract *all* the logic into other classes
> and just make the "old" class keep instantiate all the fields and pass
> them on to the underlying "new" classes with cleaner
> responsibilities. I'm not really saying this should be done, but the
> class has that "distinct" feeling :)

Well, when I started it more than thirteen years ago (had to look that
up :-) it was only ~600 lines and then it grew from that.

I'm not trying to defend anything, the class has outgrown its initial
scope by far and backwards compatibility constraints have made it more
or less impossible to refactor it properly.  This is one reason why I
started the compress antlib as the same was true for Ant's <zip> task
family.  Some day I'll find time to revive the currently dormant
compress 2.0 effort ...

> I took Gary's "cheap" solution to the StreamCompressor "subclass"
> issue; I just made the whole streamcompressor package private and not
> a part of any public api. Given that it's not really meant to be an
> extension point I think this is adequate.

+1

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@gmail.com>.
2014-12-29 15:05 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2014-12-29, Kristian Rosenvold wrote:
>
>> The refactoring os ZipArchiveOutputStream to use StreamCompressor is now done in
>> the branch https://github.com/krosenvold/commons-compress
>
> Some code comments:
>
> * the fields writtenToOutputStream, sourcePayloadLength and actualCrc in
>   StreamCompressor can probably be made private
> * inside the streams we usually write to the underlying stream first and
>   update the count later - so the count is never bigger than what has
>   been written if the write throws an exception.  I don't think this
>   also applied to StreamCompressor's writeCounted but may be nice for
>   consistency.
> * ZipArchiveOutputStream's Deflater has been protected and is now
>   removed.  We'll need to discuss whether we all can accept the
>   potentially breaking change.
I fixed all comments and reinstated the protected Deflater.
>
>> As refactorings come it doesn't "feel" too good (extracting all the
>> methods to create zip headers to a different class would probably be a
>> refactoring with a lot better *feeling* to it....). This may be due to
>> the overall largeness of the file in question, it may also be because
>> of the "leaks" between ZipArchiveOutputStream (fields raf & out) and
>> StreamCompressor.
>
> You could probably remove the out field if you added a flush method to
> the stream compressor.  But the raf field remains for rewriting sizes in
> seekable output - I don't see how this could be moved to
> StreamCompressor without feeling strange.

I think the only way to make this good is to avoid putting all the
seek stuff into the streamcompressor; that's what I did. The whole
ZipArchiveOutputStream class reminds me of a few of the 3000+ LOC java
classes I refactored in maven core; sometimes the only acceptable
solution is to extract *all* the logic into other classes and just
make the "old" class keep instantiate all the fields and pass them on
to the underlying "new" classes with cleaner responsibilities. I'm not
really saying this should be done, but the class has that "distinct"
feeling :)

I took Gary's "cheap" solution to the StreamCompressor "subclass"
issue; I just made the whole streamcompressor package private and not
a part of any public api. Given that it's not really meant to be an
extension point I think this is adequate. Alternately I would actually
consider going back to the old "if" statement that used to be there
before I introduced all the OO political correctness.

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-29, Kristian Rosenvold wrote:

> The refactoring os ZipArchiveOutputStream to use StreamCompressor is now done in
> the branch https://github.com/krosenvold/commons-compress

Some code comments:

* the fields writtenToOutputStream, sourcePayloadLength and actualCrc in
  StreamCompressor can probably be made private
* inside the streams we usually write to the underlying stream first and
  update the count later - so the count is never bigger than what has
  been written if the write throws an exception.  I don't think this
  also applied to StreamCompressor's writeCounted but may be nice for
  consistency.
* ZipArchiveOutputStream's Deflater has been protected and is now
  removed.  We'll need to discuss whether we all can accept the
  potentially breaking change.

> As refactorings come it doesn't "feel" too good (extracting all the
> methods to create zip headers to a different class would probably be a
> refactoring with a lot better *feeling* to it....). This may be due to
> the overall largeness of the file in question, it may also be because
> of the "leaks" between ZipArchiveOutputStream (fields raf & out) and
> StreamCompressor.

You could probably remove the out field if you added a flush method to
the stream compressor.  But the raf field remains for rewriting sizes in
seekable output - I don't see how this could be moved to
StreamCompressor without feeling strange.

I'd like to see the OutputStream and RandomAccessFile versions of
StreamCompressor#create go away in favour of BackingStore
implementations, this could remove the inner subclasses at the same
time.  The RandomAccessFile case could probably be merged into
FileBasedBackingStore where the BackingStore preferred RandomAccessFile
when available and falls back to FileOutputStream.  Unfortunately that
would just duplicate the logic from ZipArchiveOutputStream since that
one still needed access to the RandomAccessFile.

Yes, I can see how splitting out the code that writes the metadata parts
of the archive from those that write actual file data might help.  The
later part could again be separated for DEFLATED and STORED.  What I
don't see is how to do it in a backwards compatible manner.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@apache.org>.
The refactoring os ZipArchiveOutputStream to use StreamCompressor is now done in
the branch https://github.com/krosenvold/commons-compress

As refactorings come it doesn't "feel" too good (extracting all the
methods to create zip headers to a different class would probably be a
refactoring with a lot better *feeling* to it....). This may be due to
the overall largeness of the file in question, it may also be because
of the "leaks" between ZipArchiveOutputStream (fields raf & out) and
StreamCompressor.

It removes the duplication and is probably better since it clearly
decouples one aspect of the algorithm from the rest. I just wish I
could get it even better .... Feedback welcome :)

Kristian



2014-12-26 18:54 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2014-12-23, Stefan Bodewig wrote:
>
>> Your commit message calls out writeOut as a particilar problem.
>> Fortunately this is one is final, so we don't have to retain the effect
>> of subclasses overriding it.  Wouldn't it be possible to have writeOut
>> call the StreamCompressor's writeOut method so it still worked as
>> before?
>
> I see you've already started to implement that in your github fork. :-)
>
>> [As an aside I'd prefer the StreamCompressor being passed into the
>> stream so we don't need subclasses just to replace the compressor.]
>
> At least for the ScatterZipOutputStream I've started to experiment a
> little:
> <https://github.com/bodewig/commons-compress/compare/scatter-backing-store>
>
> As for
>
>> One problem of the zip package is that it doesn't support all the
>> other compression methods possible for ZIPs, this might be a possible
>> extension point.
>
> I'd like to see where you are going to take the compressor before I try
> to dive in.  Do I need to look at your plexus-archiver fork?
>
> Stefan
>
> ---------------------------------------------------------------------
> 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: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-28, Kristian Rosenvold wrote:

> 2014-12-28 11:35 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
>> On 2014-12-26, Kristian Rosenvold wrote:

>>> D) Look at performance in the "gather" phase. It's too slow right now,
>>> even with my last commit on trunk. Consider moving the creation of the
>>> LFH to the multithreaded parts and actually copy parts to the central
>>> directory upon merging. Ideally, the "gather" phase should be just IO.
>>> Maybe make the creation of the central directory a Deferred stream
>>> too, preferably of the offloading type.

>> As long as you only write to the central directory stream after you've
>> completely written the entry (otherwise you may not know the sizes
>> depending on the method and whether the backing store is seekable) this
>> should be doable.

> The nice thing about using ScatterZipOutputStream first is that we
> know *everything* about sizes and crcs before we start writing the zip
> file.

Ah, yes, re-read ScatterZipOutputStrean again.

>> May I ask sneaking in "prefer using delegation over inheritance where
>> possible"? ;-)

> OMG; I seem to have gone full circle on this. I have been deeply in
> the "prefer delegation" camp for the last 15 years or so. For the last
> year or so I have been exposed to "inheritance"-based extensions in
> @dayjob, and it's rubbing off on me....

Welcome back :-)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@gmail.com>.
2014-12-28 11:35 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2014-12-26, Kristian Rosenvold wrote:
>> A) Think about manifest handling in genreal
>
> This applies to plexus since CC isn't doing any manifest handling at all
> right now.
Yes, I'm thinking along the lines of just making it "easy" for the
client to do this task. I'll try to keep the manifest-specific stuff
out of CC.

>> D) Look at performance in the "gather" phase. It's too slow right now,
>> even with my last commit on trunk. Consider moving the creation of the
>> LFH to the multithreaded parts and actually copy parts to the central
>> directory upon merging. Ideally, the "gather" phase should be just IO.
>> Maybe make the creation of the central directory a Deferred stream
>> too, preferably of the offloading type.
>
> As long as you only write to the central directory stream after you've
> completely written the entry (otherwise you may not know the sizes
> depending on the method and whether the backing store is seekable) this
> should be doable.

The nice thing about using ScatterZipOutputStream first is that we
know *everything* about sizes and crcs before we start writing the zip
file. So the only thing we really need to know for the central
directory is the offset, which is also known up-front as long as
scatter is fully complete before gather. So at some point I'll also be
looking at single-pass zip writing in ZipArchiveOutputStream (when
using raw methods), but that's on the "nice to have"" list.

>
> May I ask sneaking in "prefer using delegation over inheritance where
> possible"? ;-)

OMG; I seem to have gone full circle on this. I have been deeply in
the "prefer delegation" camp for the last 15 years or so. For the last
year or so I have been exposed to "inheritance"-based extensions in
@dayjob, and it's rubbing off on me....

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-26, Kristian Rosenvold wrote:

> Yup; I'm taking care of the duplication in trunk on my github fork.
> The other interesting branch to look at is the somewhat stale
> "concurrentSupport" branch and in particular the class
> ConcurrentZipCreator. This is my primary goal, I just went off on a
> round of yak-shaving first. I seem to have finished with the yaks now,
> so
> I aim to rebase this branch to trunk soon.

Thanks.

> Right now the problem of multiple algorithms is not "my" itch; I have
> enough other itches right now.

Understood.

> My todo list right now look like this:
> A) Think about manifest handling in genreal

This applies to plexus since CC isn't doing any manifest handling at all
right now.

> B) Investigate manifest merging in ConcurrentZipCreator. ATM I'm just
> thinking of exposing all the added manifests as a collection and
> leaving this to the client to merge stuff when multiple manfests are
> present.

Since you see ConcurrentZipCreator coming to CC we should try to void
special casing the manifest.  ZipFile already has an API that exposes
all entries of a given name, the same may be possible for
ConcurrentZipCreator.

> D) Look at performance in the "gather" phase. It's too slow right now,
> even with my last commit on trunk. Consider moving the creation of the
> LFH to the multithreaded parts and actually copy parts to the central
> directory upon merging. Ideally, the "gather" phase should be just IO.
> Maybe make the creation of the central directory a Deferred stream
> too, preferably of the offloading type.

As long as you only write to the central directory stream after you've
completely written the entry (otherwise you may not know the sizes
depending on the method and whether the backing store is seekable) this
should be doable.

May I ask sneaking in "prefer using delegation over inheritance where
possible"? ;-)

Cheers

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Kristian Rosenvold <kr...@gmail.com>.
Yup; I'm taking care of the duplication in trunk on my github fork.
The other interesting branch to look at is the somewhat stale
"concurrentSupport" branch and in particular the class
ConcurrentZipCreator. This is my primary goal, I just went off on a
round of yak-shaving first. I seem to have finished with the yaks now,
so
I aim to rebase this branch to trunk soon.

I suppose I will be committing the "ConcurrentZipCreator" to
c-compress. Originally I had this in plexus-archiver, but I moved it
over once the use case was sufficiently well understood. So right now
I have no code in plexus-archiver, other than a branch or two with
early versions of ConcurrentZipCreator in it.

Right now the problem of multiple algorithms is not "my" itch; I have
enough other itches right now.

My todo list right now look like this:
A) Think about manifest handling in genreal
B) Investigate manifest merging in ConcurrentZipCreator. ATM I'm just
thinking of exposing all the added manifests as a collection and
leaving this to the client to merge stuff when multiple manfests are
present.
C) Decide on the ultimate thread-safety issues in the client front-end
(ConcurrentZipCreator). I'm walking the line right now (quite a risky
model), and I'd like to make it clearer.
D) Look at performance in the "gather" phase. It's too slow right now,
even with my last commit on trunk. Consider moving the creation of the
LFH to the multithreaded parts and actually copy parts to the central
directory upon merging. Ideally, the "gather" phase should be just IO.
Maybe make the creation of the central directory a Deferred stream
too, preferably of the offloading type.

I suppose that about sums it up. When these things are done I'm ready :)

Kristian



2014-12-26 18:54 GMT+01:00 Stefan Bodewig <bo...@apache.org>:
> On 2014-12-23, Stefan Bodewig wrote:
>
>> Your commit message calls out writeOut as a particilar problem.
>> Fortunately this is one is final, so we don't have to retain the effect
>> of subclasses overriding it.  Wouldn't it be possible to have writeOut
>> call the StreamCompressor's writeOut method so it still worked as
>> before?
>
> I see you've already started to implement that in your github fork. :-)
>
>> [As an aside I'd prefer the StreamCompressor being passed into the
>> stream so we don't need subclasses just to replace the compressor.]
>
> At least for the ScatterZipOutputStream I've started to experiment a
> little:
> <https://github.com/bodewig/commons-compress/compare/scatter-backing-store>
>
> As for
>
>> One problem of the zip package is that it doesn't support all the
>> other compression methods possible for ZIPs, this might be a possible
>> extension point.
>
> I'd like to see where you are going to take the compressor before I try
> to dive in.  Do I need to look at your plexus-archiver fork?
>
> Stefan
>
> ---------------------------------------------------------------------
> 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: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-23, Stefan Bodewig wrote:

> Your commit message calls out writeOut as a particilar problem.
> Fortunately this is one is final, so we don't have to retain the effect
> of subclasses overriding it.  Wouldn't it be possible to have writeOut
> call the StreamCompressor's writeOut method so it still worked as
> before?

I see you've already started to implement that in your github fork. :-)

> [As an aside I'd prefer the StreamCompressor being passed into the
> stream so we don't need subclasses just to replace the compressor.]

At least for the ScatterZipOutputStream I've started to experiment a
little:
<https://github.com/bodewig/commons-compress/compare/scatter-backing-store>

As for

> One problem of the zip package is that it doesn't support all the
> other compression methods possible for ZIPs, this might be a possible
> extension point.

I'd like to see where you are going to take the compressor before I try
to dive in.  Do I need to look at your plexus-archiver fork?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Stefan Bodewig <bo...@apache.org>.
On 2014-12-22, Kristian Rosenvold wrote:

> There are quite a few extension points in this class that make
> changing it really hard.

ACK

> I just committed  r1647329 which basically duplicates some code from
> this class into another class. As much as I hate duplication, I wasn't
> able to achieve what I wanted to without A) breaking some extension
> capability of the existing class or B) duplicating the code.

I'm not sure adding the features of ScatterZipOutputStream to
ZipArchiveOutputStream would be such a good idea.  Maybe it is not too
bad they are two separate clases.  Most likely you are not suggesting
that but rather rewriting ZipArchiveOutputStream in terms of
StreamCompressor to reduce duplication.

Your commit message calls out writeOut as a particilar problem.
Fortunately this is one is final, so we don't have to retain the effect
of subclasses overriding it.  Wouldn't it be possible to have writeOut
call the StreamCompressor's writeOut method so it still worked as
before?

[As an aside I'd prefer the StreamCompressor being passed into the
stream so we don't need subclasses just to replace the compressor.  One
problem of the zip package is that it doesn't support all the other
compression methods possible for ZIPs, this might be a possible
extension point.]

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [compress] Importance of retaining exact compatibility for ZipArchiveOutputStream ?

Posted by Gary Gregory <ga...@gmail.com>.
You could talk about what a compress2 version that breaks BC would look
like. Are there other changes that would be good in a 2.0 version?

Gary

On Mon, Dec 22, 2014 at 10:33 AM, Kristian Rosenvold <kr...@apache.org>
wrote:
>
> There are quite a few extension points in this class that make
> changing it really hard.
>
> I just committed  r1647329 which basically duplicates some code from
> this class into another class. As much as I hate duplication, I wasn't
> able to achieve what I wanted to without A) breaking some extension
> capability of the existing class or B) duplicating the code.
>
> We are not talking about the "bread and butter" usage of the class
> here but rather intricate extensions I'm unsure of how many people
> use. If it was up to me I'd break compatibility slightly an make a
> JIRA that explains how to migrate forwards and hence move the class to
> a somewhat better state. As things are right now I retained
> compatibility by not changing the class itself.
>
> WDYT ?
>
> Kristian
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory