You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/06/24 09:17:01 UTC
[GitHub] [maven-ear-plugin] santik opened a new pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
santik opened a new pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9
In plugin instead of correct EAR archiver JAR archiver was used.
In this PR application will use correct archiver.
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] pzygielo commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r444763785
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -692,9 +693,9 @@ public String getMappedFileName( String pName )
*
* @return the archiver
*/
- protected JarArchiver getJarArchiver()
+ protected EarArchiver getEarArchiver()
Review comment:
Perhaps `link` in doc could be updated and import of `JarAchiver` removed?
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695957296
@hboutemy sure! I've just resolved conflicts, but if you have anything to add- do it
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695957296
@hboutemy sure! I've just resolved conflicts, but if you have anything to add- do it
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695358931
question: what does an ear archiver do differently from a jar archiver that brings concrete benefit, please?
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494063438
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
given there is a test later that checks and fail if the descriptor does not exist, it seems this case is not really useful, isn't it?
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
I tested and found that starting with JavaEE 5, descriptor is not required: but currently, EarArchiver is implemented in a way that does not support this condition
ok, now I see how we must either improve EarArchiver, either do the workaround you did: I'll merge and document the workaround
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-648743428
> On [fb2a47c](https://github.com/apache/maven-ear-plugin/commit/fb2a47cf8cdbc23639b49d6f892a9a0e2fc766d2) I got
>
> ```
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-ear-plugin:3.1.0-SNAPSHOT:ear (default-ear) on project maven-ear-plugin-test-project-054: Error assembling EAR: Deployment descriptor: .../maven-ear-plugin/target/test-classes/projects/project-054/target/maven-ear-plugin-test-project-054-99.0/META-INF/application.xml does not exist. -> [Help 1]
> org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-ear-plugin:3.1.0-SNAPSHOT:ear (default-ear) on project maven-ear-plugin-test-project-054: Error assembling EAR
> ...
> [ERROR] Errors:
> [ERROR] EarMojoIT.testProject054:652->AbstractEarPluginIT.doTestProject:184->AbstractEarPluginIT.doTestProject:169->AbstractEarPluginIT.doTestProject:144->AbstractEarPluginIT.executeMojo:128->AbstractEarPluginIT.executeMojo:97 » Verification
> ```
You are right. Sometimes we still need to use JAR archiver. Updated the code.
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695759498
@hboutemy it is described in the ticket as showing incorrect message
however I think it is better to use correct implementation instead of generic jar archiver when it is possible
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494830755
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
ok
I fixed conflicts again
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494063438
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
given there is a test later that checks and fail if the descriptor does not exist, it seems this case is not really useful, isn't it?
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695923376
ok, now I get that the message is written by the archiver, not by the plugin
adn from a pure theoretical perspective, a more dedicated archiver can help in some cases but given the plugin worked perfectly with simple jar, I need to clarify if any ear archiver feature won't conflict
but sure, I'll rework the provided patch to fix the current conflict with master, if you don't beat me at it :)
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494092160
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
file is not required for the jar archiver, but required for ear
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-695923376
ok, now I get that the message is written by the archiver, not by the plugin
adn from a pure theoretical perspective, a more dedicated archiver can help in some cases but given the plugin worked perfectly with simple jar, I need to clarify if any ear archiver feature won't conflict
but sure, I'll rework the provided patch to fix the current conflict with master, if you don't beat me at it :)
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494624818
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
I tested and found that starting with JavaEE 5, descriptor is not required: but currently, EarArchiver is implemented in a way that does not support this condition
ok, now I see how we must either improve EarArchiver, either do the workaround you did: I'll merge and document the workaround
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r444767125
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -692,9 +693,9 @@ public String getMappedFileName( String pName )
*
* @return the archiver
*/
- protected JarArchiver getJarArchiver()
+ protected EarArchiver getEarArchiver()
Review comment:
done
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] santik commented on a change in pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
santik commented on a change in pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#discussion_r494092160
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
file is not required for the jar archiver, but required for ear
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -289,12 +296,25 @@ public void execute()
final File earFile;
final MavenArchiver archiver;
final Date reproducibleLastModifiedDate;
+ File ddFile = new File( getWorkDirectory(), APPLICATION_XML_URI );
try
{
earFile = getEarFile( outputDirectory, finalName, classifier );
archiver = new EarMavenArchiver( getModules() );
- getLog().debug( "Jar archiver implementation [" + jarArchiver.getClass().getName() + "]" );
- archiver.setArchiver( jarArchiver );
+ JarArchiver theArchiver;
+ if ( ddFile.exists() )
+ {
+ final EarArchiver earArchiver = getEarArchiver();
+ earArchiver.setAppxml( ddFile );
+ theArchiver = earArchiver;
+ }
+ else
+ {
+ theArchiver = getJarArchiver();
Review comment:
ok
I fixed conflicts again
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] pzygielo commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-648726740
On fb2a47cf8cdbc23639b49d6f892a9a0e2fc766d2 I got
```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-ear-plugin:3.1.0-SNAPSHOT:ear (default-ear) on project maven-ear-plugin-test-project-054: Error assembling EAR: Deployment descriptor: .../maven-ear-plugin/target/test-classes/projects/project-054/target/maven-ear-plugin-test-project-054-99.0/META-INF/application.xml does not exist. -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-ear-plugin:3.1.0-SNAPSHOT:ear (default-ear) on project maven-ear-plugin-test-project-054: Error assembling EAR
...
[ERROR] Errors:
[ERROR] EarMojoIT.testProject054:652->AbstractEarPluginIT.doTestProject:184->AbstractEarPluginIT.doTestProject:169->AbstractEarPluginIT.doTestProject:144->AbstractEarPluginIT.executeMojo:128->AbstractEarPluginIT.executeMojo:97 » Verification
```
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy commented on pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9#issuecomment-699462405
merged and updated to document the unexpectedly complex logic for an issue that we supposed was up-for-grabs... :)
thank you everybody for your help and efforts
----------------------------------------------------------------
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
[GitHub] [maven-ear-plugin] hboutemy closed pull request #9: [MEAR-194] Use ear archiver instead of jar archiver
Posted by GitBox <gi...@apache.org>.
hboutemy closed pull request #9:
URL: https://github.com/apache/maven-ear-plugin/pull/9
----------------------------------------------------------------
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