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

MASSEMBLY-617 : add ability to set fileMappers in FileSet in maven as… #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tcollignon
Copy link

@tcollignon tcollignon commented Jul 26, 2018

…sembly plugin

new version of PR #4 for https://issues.apache.org/jira/browse/MASSEMBLY-617

@tcollignon
Copy link
Author

Hi

This need certainly to add test but I don't know where :(

And in MDO it"s wrong in version and again I don't know what i must do in it

Thx

Copy link

@spiritualops spiritualops left a comment

Choose a reason for hiding this comment

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

Don't use SNAPSHOTs, please.

pom.xml Outdated
@@ -138,7 +138,7 @@ under the License.
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-archiver</artifactId>
<version>3.6.0</version>
<version>3.6.1-SNAPSHOT</version>

Choose a reason for hiding this comment

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

Please don't use SNAPSHOT versions, as this could introduce instability at any given time.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @spiritualops , yes of course, I'm just waiting plexus release (maybe it's already done) to change this with the correct release version

And I need help to maybe add some test, and for MDO

Thx

Choose a reason for hiding this comment

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

The Plexus Archiver has been released and the assembly plugin is already updated to use a newer version so you can just rebase your branch.

<name>fileMappers</name>
<version>3.2.0+</version>
<association>
<type>FileMapper</type>

Choose a reason for hiding this comment

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

You can't use the FileMapper from Plexus IO directly. You need to declare all types (except "simple" types such as String) in the MDO. So you have to declare this type in MDO as well and then use it to configure the actual FileMappers.

@plamentotev
Copy link

@tcollignon I've added comment about the MDO inline. hope it helps but if its too vague I'll try to find some time to add an example.

About the tests take a look at src/it/projects/file-sets - those are the integration tests for the file sets. One of those should be suitable to add file matter and then verify it. If not you can use one as base to crate a new one. Also it may be worth to check if src/test/java/org/apache/maven/plugins/assembly/archive/task/AddFileSetsTaskTest.java and src/test/java/org/apache/maven/plugins/assembly/archive/phase/FileSetAssemblyPhaseTest.java need to be updated.

@tcollignon
Copy link
Author

thx for your answer @plamentotev , I will try to finish this when I can

@tcollignon
Copy link
Author

So @plamentotev I have try to do something.
Can you tell my what you think please?
I'm not very comfortable with MDO, I'm sorry.
I have added a large amount of code in AddDirectoryTask, and I think it's not very good with the beautiful il else ..., if you have better please tell me

@tcollignon
Copy link
Author

Note : This PR could close https://issues.apache.org/jira/browse/MASSEMBLY-886 too

@tcollignon
Copy link
Author

Hi any news here?
@plamentotev Can you look at this please?

thx for your time

@plamentotev
Copy link

Hi @tcollignon,

The problem is not that much with the verbosity of the code, but that is the list of file mappers is practically hard coded. If you want to add another one you'll need to change the AddDirectoryTask as well. You can get a FileMapper instance from Plexus using its role so you can avoid hard coding the instances. The bigger problem is how to configure them. Unfortunately the interface does not contain means to configure them so the only option is through reflection. You can take a look how Archiver is configured - org.apache.maven.plugins.assembly.archive.DefaultAssemblyArchiver#configureArchiver. You need something similar for MDO, but I'm not too familiar with it myself. I guess you can add field of type DOM that can contain arbitrary XML so you can use it to configure the FileMapper instances.

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.

3 participants