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

craigpell
Copy link

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
Copy link

asfgit commented Dec 7, 2018

Can one of the admins verify this patch?

1 similar comment
@asfgit
Copy link

asfgit commented Dec 7, 2018

Can one of the admins verify this patch?

@jaikiran
Copy link
Member

jaikiran commented Dec 7, 2018

this is ok to test

@asfgit
Copy link

asfgit 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
Copy link

asfgit 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
Copy link
Member

bodewig 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
Copy link
Author

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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

bodewig commented Dec 12, 2018

BSD is perfectly fine, thanks.

added Ant version to docs, and replaced GPL man pages with BSD-licensed
man pages.
@asfgit
Copy link

asfgit 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
Copy link

asfgit 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
Copy link
Member

bodewig 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
Copy link
Author

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
Copy link
Member

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
Labels
None yet
Projects
None yet
4 participants