You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by GitBox <gi...@apache.org> on 2020/07/30 15:32:06 UTC
[GitHub] [ant] maartenc opened a new pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
maartenc opened a new pull request #134:
URL: https://github.com/apache/ant/pull/134
Hi,
I want to create a jar containing an INDEX.LIST, but I need another jar name mapping algorithm than the default one provided by Ant. I thought it would be a good idea to add the possibility to specify a (optional) filename mapper within the <indexjars> element to be able to provide another algorithm.
Something like this:
```
<jar>
<indexjars>
<fileset dir="build/lib/*.jar"/>
<globmapper from="build/lib/*.jar" to="bin/*.jar"/>
</indexjars>
</jar>
```
What do you think, would such an enhancement make sense?
And do you think the code changes are a good start? (it still needs some javadocs, junit testing, ...)
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-671460358
Thx for the feedback @bodewig .
I'll try to update the PR.
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-671366647
Many thanks @maartenc I believe your second alternative sounds like what I meant. It feels as if the impact of the change would be smaller overall - so please give it a try if you've got the time to do so.
Your current patch would be binary incompatible (code compiled against an older version of Ant that called `addConfiguredIndexJars` with a `Path` would now no longer find the method it was expecting and had to be recompiled). It would even be source incompatible for a hypothetic subclass of `Jar` that overrode `addConfiguredIndexJars` but I doubt such a thing exists.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc edited a comment on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc edited a comment on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-671239026
Hi @bodewig ,
You are right, here is a bit more context of the problem.
Our applications are installed like this:
```
<some root dir>
/Shared
/org/apache/commons-io/commons-io-2.4.jar
/org/apache/commons-lang/commons-lang-2.6.jar
...
/ApplicationA
main.jar
lib1.jar
lib2.jar
...
/ApplicationB
main.jar
lib1.jar
lib3.jar
...
```
Our biggest application uses about 500 jars. To speed up classloading, we are adding an INDEX.LIST to main.jar.
The INDEX.LIST spec says that the jar file paths in the INDEX.LIST should be relative to main.jar. So the INDEX.LIST should look like:
```
JarIndex-Version: 1.0
main.jar
[...]
lib1.jar
[...]
../Shared/org/apache/commons-io/commons-io-2.4.jar
[...]
../Shared/org/apache/commons-lang/commons-lang-2.6.jar
[...]
```
With the current Ant `<jar>` task, there are 2 possibilities to configure the jar file paths in the INDEX.LIST.
1. if main.jar doesn't contain a `Class-Path` attribute (our use-case), all jar file paths are flattened.
2. if main.jar does contain a `Class-Path` attribute, the jar file path is taken from that `Class-Path` attribute.
Since we don't have a `Class-Path` attribute (which is intentionaly), we end up with a flatten path names in INDEX.LIST.
A workaround could probably be to:
1. add a `Class-Path` attribute to the MANIFEST.MF with the wanted mapping for INDEX.LIST
2. generate the INDEX.LIST
3. remove the `Class-Path` attribute from the MANIFEST.MF
This is off course a suboptimal solution making the build script needlessly complex.
The attached patch tries to address the issue by making it possible to specify a filename-mapper **inside** the `<indexjars>` element. If this mapper isn't specified, the default mapping (as specified above) is used.
```
<jar ...>
<indexjars>
<fileset dir="build/lib/**/*.jar"/>
<firstmatchmapper>
<globmapper from="${basedir}/build/lib/${application}/*" to="*" handledirsep="true" />
<globmapper from="${basedir}/build/lib/Shared/*" to="../Shared/*" handledirsep="true"/>
</firstmatchmapper>
</indexjars>
</jar>
```
An alternative patch could be to add an extra mapper subelement to the `<jar>` task directly (and not inside the `<indexjars>` element), something like:
```
<jar ...>
<indexjars>
<fileset dir="build/lib/**/*.jar"/>
</indexjars>
<indexjarsmapper>
<firstmatchmapper>
<globmapper from="${basedir}/build/lib/${application}/*" to="*" handledirsep="true" />
<globmapper from="${basedir}/build/lib/Shared/*" to="../Shared/*" handledirsep="true"/>
</firstmatchmapper>
</indexjarsmapper>
</jar>
```
I think the first solutions results in a cleaner build.xml. But with perhaps a little bit more complex java code (and with a not 100% backwards compatible API change). But the second solution is also a possibility, I can create a new patch if you prefer that solution.
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-667850214
Thanks for the feedback @bodewig ,
I don't think a `mappedresources` would help here, because the index-logic inside the `jar` task will transform the name of the jar to the basename of the jar or the name in the ClassPath manifest entry.
In my use-case, I don't have a ClassPath entry, but I don't want the jar basename in the index.list, I need another name mapping scheme which is based on the final location of the jars in our application, not a flat mapping because not all jars are located in the same directory. I thought using a filenamemapper for this would be an elegant solution.
One thing I'm not sure of is the removal of the replacement of `addConfiguredIndexJars(Path p)` with `addConfiguredIndexJars(ResourceCollection rc)`. I think I can't keep the method with the Path argument, because otherwise that method might be chosen to configure the task (and I need the new addConfiguredIndexJars method). So I replaced it with the ResourceCollection variant for backwards compatibility, but I'm not sure this is good enough to match the Ant guidelines because client libraries that are using that method directly will need to be recompiled (existing build scripts will keep working I think).
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-687096390
you should be able to merge this PR yourself.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-687081344
Looks good to me, @maartenc . You may want to undo the glob import of `org.apache.tools.ant.types.*`, though :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc merged pull request #134: Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc merged pull request #134:
URL: https://github.com/apache/ant/pull/134
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-671239026
Hi @bodewig ,
You are right, here is a bit more context of the problem.
Our applications are installed like this:
```
<some root dir>
/Shared
/org/apache/commons-io/commons-io-2.4.jar
/org/apache/commons-lang/commons-lang-2.6.jar
...
/ApplicationA
main.jar
lib1.jar
lib2.jar
...
/ApplicationB
main.jar
lib1.jar
lib3.jar
...
```
Our biggest application uses about 500 jars. To speed up classloading, we are adding an INDEX.LIST to main.jar.
The INDEX.LIST spec says that the jar file paths in the INDEX.LIST should be relative to main.jar. So the INDEX.LIST should look like:
```
JarIndex-Version: 1.0
main.jar
[...]
lib1.jar
[...]
../Shared/org/apache/commons-io/commons-io-2.4.jar
[...]
../Shared/org/apache/commons-lang/commons-lang-2.6.jar
[...]
```
With the current Ant <jar> task, there are 2 possibilities to configure the jar file paths in the INDEX.LIST.
1. if main.jar doesn't contain a Class-Path attribute (our use-case), all jar file paths are flattened.
2. if main.jar does contain a Class-Path attribute, the jar file path is taken from that Class-Path attribute.
Since we don't have a Class-Path attribute (which is intentionaly), we end up with a flatten path names in INDEX.LIST.
A workaround could probably be to:
1. add a Class-Path attribute to the MANIFEST.MF with the wanted mapping for INDEX.LIST
2. generate the INDEX.LIST
3. remove the Class-Path attribute from the MANIFEST.MF
This is off course a suboptimal solution making the build script needlessly complex.
The attached patch tries to address the issue by making it possible to specify a filename-mapper **inside** the <indexjars> element. If this mapper isn't specified, the default mapping (as specified above) is used.
```
<jar ...>
<indexjars>
<fileset dir="build/lib/**/*.jar"/>
<firstmatchmapper>
<globmapper from="${basedir}/build/lib/${application}/*" to="*" handledirsep="true" />
<globmapper from="${basedir}/build/lib/Shared/*" to="../Shared/*" handledirsep="true"/>
</firstmatchmapper>
</indexjars>
</jar>
```
An alternative patch could be to add an extra mapper subelement to the <jar> task directly (and not inside the <indexjars> element), something like:
```
<jar ...>
<indexjars>
<fileset dir="build/lib/**/*.jar"/>
</indexjars>
<indexjarsmapper>
<firstmatchmapper>
<globmapper from="${basedir}/build/lib/${application}/*" to="*" handledirsep="true" />
<globmapper from="${basedir}/build/lib/Shared/*" to="../Shared/*" handledirsep="true"/>
</firstmatchmapper>
</indexjarsmapper>
</jar>
```
I think the first solutions results in a cleaner build.xml. But with perhaps a little bit more complex java code (and with a not 100% backwards compatible API change). But the second solution is also a possibility, I can create a new patch if you prefer that solution.
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-687093433
Oops, fixed! :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-667647783
Wouldn't https://ant.apache.org/manual/Types/resources.html#mappedresources do what you need?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-671044385
I overlooked you've been doing more than "just" mapping the file name, sorry. Actually I'm still not sure I fully understand what you want to achieve and could probably use a description of the problem rather than a proposed solution :-)
I'm trying to wrap my head around jar indexing and what the task actually does right now.
FWIU you've got a `Path` of jars that will be available in certain locations when the jar you are creating is loaded and you want to create a jar-index for the jars of this `Path` and use the "certain locations" rather than the current behaviour of using the base name for each Path-element. Right?
Would it be enough to add a `FileNameMapper` that is used in https://github.com/apache/ant/blob/6022806ecd6a47c8034713303db97a3c1a2b7f0c/src/main/org/apache/tools/ant/taskdefs/Jar.java#L1030 with `FlattenMapper` as default?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] maartenc commented on pull request #134: [WIP] Add the possibility to specify a custom filename mapper when creating an INDEX.LIST
Posted by GitBox <gi...@apache.org>.
maartenc commented on pull request #134:
URL: https://github.com/apache/ant/pull/134#issuecomment-686415058
Hi @bodewig , I've updated this PR with a new attempt. Could you take a look?
This supports the following construct:
```
<jar ...>
<indexjars>
...
</indexjars>
<indexjarsmapper>
...
</indexjarsmapper>
</jar>
```
Thanks,
Maarten
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org