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

Added tasks for JDK's jmod and jlink tools. #80

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
4 participants
@craigpell
Copy link

commented Dec 7, 2018

Support for the jmod and jlink tools present in the JDK since Java 9. Now that Java 11 has no standalone JRE, officially, these tools are the only way to distribute client-side Java applications.

@asfgit

This comment has been minimized.

Copy link

commented Dec 7, 2018

Can one of the admins verify this patch?

1 similar comment
@asfgit

This comment has been minimized.

Copy link

commented Dec 7, 2018

Can one of the admins verify this patch?

@jaikiran

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2018

this is ok to test

@asfgit

This comment has been minimized.

Copy link

commented Dec 7, 2018

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

@asfgit

This comment has been minimized.

Copy link

commented Dec 7, 2018

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

@bodewig

This comment has been minimized.

Copy link
Member

commented Dec 9, 2018

This looks great Craig, many thanks!

I've got a short list of wishes, though:

  • please use a manual page that is not covered by the GPL inside of the tests - this is me probably being overly cautious but we really don't want to ship anything under the GPL as part of our releases
  • add yourself to CONTRIBUTORS and contributors.xml
  • add a blurb to WHATSNEW
  • add @since markers (with 1.10.6) to the new tasks and the new type - and do so to the manual pages as well.

I can do all or some of them myself if you prefer me to do it. In that case I'd ask you to confirm you want to be addressed as "Craig Pell" in the contributors files.

Many thanks again.

@craigpell

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

All done, except that I’m finding there aren’t many man pages on my Linux system which aren’t GPL. I’ve found a few which are under a BSD license; is that acceptable?

log("Executing: jmod " + String.join(" ", args), Project.MSG_VERBOSE);

ByteArrayOutputStream stdout = new ByteArrayOutputStream();
ByteArrayOutputStream stderr = new ByteArrayOutputStream();

This comment has been minimized.

Copy link
@jaikiran

jaikiran Dec 12, 2018

Contributor

Hi Craig, like Stefan already said, this patch is a really good addition to Ant. Thank you for that.

The only question/suggestion I have is, should we instead just pass System.out and System.err instead of creating a ByteArrayOutputStream? That way the actual output and errors are logged instead of we deciding to print it only when the tool execution fails.

This comment has been minimized.

Copy link
@craigpell

craigpell Dec 12, 2018

Author

That was what I tried first, but the output and error were never showing up at all in unit tests. (Also, my observation has been that the tool produces no output when it runs successfully.)

This comment has been minimized.

Copy link
@bodewig

bodewig Dec 13, 2018

Member

Test setup captures streams so it may become difficult. Would using LogOutputStreams do which would send the output directly to An't logging system?

@bodewig

This comment has been minimized.

Copy link
Member

commented Dec 12, 2018

BSD is perfectly fine, thanks.

VGR
Based on feedback, added self to contributors, added line to WHATSNEW,
added Ant version to docs, and replaced GPL man pages with BSD-licensed
man pages.
@asfgit

This comment has been minimized.

Copy link

commented Dec 13, 2018

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

@asfgit

This comment has been minimized.

Copy link

commented Dec 13, 2018

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

@asfgit asfgit closed this in 6b576f1 Dec 15, 2018

@bodewig

This comment has been minimized.

Copy link
Member

commented Dec 15, 2018

Many thanks again, @craigpell . I've merged this but would like to resolve the discussion about the log output.

With b74d5b3 I've committed a branch (logoutput-jmod-link) where I use LogOutputStream in link - I'm not sure why one would suppress the output at all, but I've never worked with the tool. A similar approach (or one that does away with the ByteArrayOutputStreams completely and removes the output from the exception and simply channels the output to Ant's log unconditionally) would work for jmod as well.

/cc @jaikiran

@craigpell

This comment has been minimized.

Copy link
Author

commented Dec 16, 2018

I restored the LogOutputStream usage and confirmed that the output is not visible in unit test results. It's not even captured in the system-out and system-err elements of the TEST-*.xml files.

I remember spending a good amount of time trying to find a method that would make the log show up, but I was never able to do it.

@jaikiran

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2018

I restored the LogOutputStream usage and confirmed that the output is not visible in unit test results. It's not even captured in the system-out and system-err elements of the TEST-*.xml files.

Hi @craigpell, the test(s) use the BuildFileRule[1] which as Stefan mentioned, redirect the output/error streams to StringBuffer(s)[2] which are stored in memory. Those logs are available (for tests) through the getFullLog and other such methods on the BuildFileRule.

[1] https://github.com/apache/ant/pull/80/files#diff-1bb76afc7ce4a535b0c580f9d040aff9R68
[2] https://github.com/apache/ant/blob/master/src/tests/junit/org/apache/tools/ant/BuildFileRule.java#L175
[3] https://github.com/apache/ant/blob/master/src/tests/junit/org/apache/tools/ant/BuildFileRule.java#L106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.