Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] Changes for BZ-19516 #51

Merged
merged 1 commit into from Dec 13, 2017
Merged

Conversation

jaikiran
Copy link
Member

As suggested in https://bz.apache.org/bugzilla/show_bug.cgi?id=19516, the change in this PR uses java.io.BufferedInputStream which can take an underlying InputStream and provide mark support on the input stream. This should prevent loading a large amount of data into memory, in certain cases, in the Zip task.

If this PR is approved, I'll then backport this pretty straightforward change to 1.9.x branch too.

@jaikiran jaikiran closed this Dec 12, 2017
@jaikiran jaikiran reopened this Dec 12, 2017
@jaikiran
Copy link
Member Author

retest this please

@asfgit
Copy link

asfgit commented Dec 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant-Master%20Github-PR-Windows/2/

@jaikiran
Copy link
Member Author

retest this please

@asfgit
Copy link

asfgit commented Dec 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant-1.9.x%20Github-PR-Windows/7/

@asfgit
Copy link

asfgit commented Dec 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant-Master%20Github-PR-Windows/4/

@asfgit
Copy link

asfgit commented Dec 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant-1.9.x%20Github-PR-Windows/8/

@jaikiran jaikiran closed this Dec 12, 2017
@jaikiran jaikiran reopened this Dec 12, 2017
@asfgit
Copy link

asfgit commented Dec 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant-Master%20Github-PR-Windows/7/

@bodewig
Copy link
Member

bodewig commented Dec 12, 2017

an alternative would be to only wrap the stream inside of zipFile if mark wasn't supported. But I'm fine with the change as it is.

Thank you for dusting of really old issues.

@asfgit
Copy link

asfgit commented Dec 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Linux/1/

@jaikiran
Copy link
Member Author

an alternative would be to only wrap the stream inside of zipFile if mark wasn't supported.

@bodewig, I actually liked your idea. So I did an additional change in this commit (PR has been updated) to wrap the incoming stream into a stream that supports marking, if the incoming one doesn't. That way, it takes into account input streams that this Zip class doesn't construct and instead is passed to this method.

Please take a look at the updated PR and let me know what you think.

@asfgit
Copy link

asfgit commented Dec 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ant%20Github-PR-Windows/8/

@bodewig
Copy link
Member

bodewig commented Dec 13, 2017

Looks good to me.

@asfgit asfgit merged commit 17f06a9 into apache:master Dec 13, 2017
@jaikiran
Copy link
Member Author

Thanks for reviewing. Merged to master branch. Will backport it to 1.9.x too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants