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 version and mainClass attributes to <jar> task. #89

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

craigpell
Copy link

Added version and mainClass attributes to task, to set binary attributes of a modular jar file. For Ant 1.10.x. Addresses these tickets:
https://bz.apache.org/bugzilla/show_bug.cgi?id=62772
https://bz.apache.org/bugzilla/show_bug.cgi?id=62789

@asfgit
Copy link

asfgit commented Mar 23, 2019

Can one of the admins verify this patch?

3 similar comments
@asfgit
Copy link

asfgit commented Mar 23, 2019

Can one of the admins verify this patch?

@asfgit
Copy link

asfgit commented Mar 23, 2019

Can one of the admins verify this patch?

@asfgit
Copy link

asfgit commented Mar 23, 2019

Can one of the admins verify this patch?

execution entry point. Setting this does not affect the manifest in any way, and specifying
a Main-Class in the manifest will not cause this attribute to be set.
<strong>Note:</strong> If this is set to a class which isn't in the jar file, Java will
refuse to load the module at runtime.
Copy link
Contributor

@twogee twogee Mar 24, 2019

Choose a reason for hiding this comment

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

missing </td>

</tr>
<tr>
<td>build</td>
<td>Optional build number. Can contain any text.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing </td>

@@ -0,0 +1,59 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

The commit contains a vim swap file for this class; perhaps .gitignore must be updated?

@@ -0,0 +1,147 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

There is another implementation in org.apache.tools.ant.taskdefs.optional.depend.constantpool; perhaps that could be reused?

@jaikiran
Copy link
Member

this is ok to test

@asfgit
Copy link

asfgit commented Mar 25, 2019

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

@asfgit
Copy link

asfgit commented Mar 25, 2019

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

<td>version</td>
<td>The <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/module/ModuleDescriptor.Version.html">module version</a>
to embed into the jar file's module descriptor, if it has one. Not used in Java versions
older than Java 9.
Copy link
Member

Choose a reason for hiding this comment

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

Not used in Java versions older than Java 9.

I think this wording is confusing. Rephrasing it to "Not used in Java versions below Java 9", might be better.

@jaikiran
Copy link
Member

Thanks @craigpell, this is a very good enhancement and the code looks very good too. I do have some suggestions/questions about some of the new classes, but I haven't had a chance to fully review this PR. I'll take a more closer look at this one in the next few days and will merge or update this PR with my suggestions/questions.

@asfgit
Copy link

asfgit commented Mar 29, 2019

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

@asfgit
Copy link

asfgit commented Mar 29, 2019

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

@jaikiran
Copy link
Member

While you are here, a quick suggestion/question - given that these changes are related to Java modules in general and there probably will be more such enhancements for modules, maybe we should rename the newly added org.apache.tools.ant.util.jarattr package to org.apache.tools.ant.util.modules instead? I don't have a strong opinion on it, so if you want to use the current package name that you chose, that's fine by me.

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

Successfully merging this pull request may close these issues.

4 participants