Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Fix working with S3 blobstore #103

Closed
wants to merge 1 commit into from
Closed

Conversation

mpoindexter
Copy link
Contributor

@mpoindexter mpoindexter commented Mar 12, 2019

ArArchiveInputStream in commons-compress 1.18 has a bug that
prevents working with any source stream that does not implement
available() as returning the number of bytes remaining to be
read in the entire stream. Since socket based streams don't
behave like this, it does not work with the S3 blobstore.

commons-compress has fixed this upstream, but it's not in a
released version yet, so I've included a local copy of the
fixed class until this appears in an upstream release and can be
fixed by depending on a newer version.

ArArchiveInputStream in commons-compress 1.18 has a bug that
prevents working with any source stream that does not implement
available() as returning the number of bytes remaining to be
read in the entire stream. Since socket based streams don't
behave like this, it does not work with the S3 blobstore.

commons-compress has fixed this upstream, but it's not in a
released version yet, so I've included a local copy of the
fixed class until this appears in an upstream release and can be
fixed by depending on a newer version.
@mpoindexter
Copy link
Contributor Author

Should fix #43

@DarthHater - is this fine from a licensing perspective?

@DarthHater
Copy link
Member

@mpoindexter what method got changed for the fix? Might be easier to just override it in some smaller class etc...

Not 100% sure on the licensing aspect, but I surmise it's ok. I'll ask internally to see what the dealio is.

@mpoindexter
Copy link
Contributor Author

The code here is actually just the latest from commons-compress's git. That commit is https://gitbox.apache.org/repos/asf?p=commons-compress.git;a=commit;h=8e5338cd2d6edc347cfe2bcd72f15bf23b178f5b

@mpoindexter
Copy link
Contributor Author

I didn't want to override in some smaller class since the idea is that once commons-compress ships a release with this fix in, we can just bump the dependency version and get rid of this code.

@DarthHater
Copy link
Member

Yeah I can vibe on that, just like hate copying and pasting whole classes, etc... for licensing, blah blah blah issues.

@jlstephens89 you have any preference on dealing with this?

@j-s-3
Copy link
Contributor

j-s-3 commented Mar 18, 2019

@DarthHater I've added this as part of our work to internalize the APT plugin. I agree I'd rather not copy in a full class.

@mpoindexter
Copy link
Contributor Author

I don't love the full class either, just seemed like the least bad option at the moment since the method that controls this touches private members. Hopefully by the time you guys get this internalized commons-compress has done a release and you can just depend on it. Might be worth asking on their mailing list when they plan to do a release.

@bhamail
Copy link
Contributor

bhamail commented Sep 27, 2019

APT is now part of Nexus Repository Manager. Version 3.17.0 includes the APT plugin by default.
If this is still an issue if using 3.17.0 or later please file an issue at https://issues.sonatype.org/.
Links to the new source code location are in the top level README.md

@bhamail bhamail closed this Sep 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants