[GitHub] [maven-filtering] sebthom opened a new pull request #15: MRESOURCES-268 Report path of offending file on copy/filter error.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] sebthom opened a new pull request #15: MRESOURCES-268 Report path of offending file on copy/filter error.

GitBox

sebthom opened a new pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15


   Changes DefaultMavenFileFilter#copyFile to contain the path to the
   offending file as part of the thrown MavenFilteringException.
   Without this change end-users won't see which file caused the error.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] michael-o commented on a change in pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox

michael-o commented on a change in pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#discussion_r556037237



##########
File path: src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
##########
@@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
         }
         catch ( IOException e )
         {
-            throw new MavenFilteringException( e.getMessage(), e );
+            throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to "

Review comment:
       What is the purpose to duplicate information like here?




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] sebthom commented on a change in pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox
In reply to this post by GitBox

sebthom commented on a change in pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#discussion_r556049602



##########
File path: src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
##########
@@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
         }
         catch ( IOException e )
         {
-            throw new MavenFilteringException( e.getMessage(), e );
+            throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to "

Review comment:
       What do you mean with duplicate information? When maven-resources-plugin currently fails, it only displays a cryptic error message like `[ERROR] Failed to execute goal org.apache.maven.plugins:maven-resources-plugin:3.2.0:resources (default-resources) on project test: Input length = 1 -> [Help 1]` without **any** context information. Even when using `mvn -e` the displayed stacktrace does not show any information about the file that failed during the operation.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] michael-o commented on a change in pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox
In reply to this post by GitBox

michael-o commented on a change in pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#discussion_r556067456



##########
File path: src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
##########
@@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
         }
         catch ( IOException e )
         {
-            throw new MavenFilteringException( e.getMessage(), e );
+            throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to "

Review comment:
       The the root cause does not bubble up, then this is crap. Likely the log message is not ok. You should not try for this the symptom here. Please look in MRESOURCES where this exception is handled.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] sebthom commented on a change in pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox
In reply to this post by GitBox

sebthom commented on a change in pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#discussion_r556529636



##########
File path: src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
##########
@@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
         }
         catch ( IOException e )
         {
-            throw new MavenFilteringException( e.getMessage(), e );
+            throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to "

Review comment:
       The maven resources plugin is delegating the iteration over files to MavenResourcesFiltering, thus is not aware of the file for which filtering fails and therefore cannot report the file path.
   
   Adding the file paths to the error message here has the advantage that all potential code paths to DefaultMavenFileFilter will be covered.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] sebthom commented on a change in pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox
In reply to this post by GitBox

sebthom commented on a change in pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#discussion_r556530488



##########
File path: src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
##########
@@ -110,7 +110,8 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
         }
         catch ( IOException e )
         {
-            throw new MavenFilteringException( e.getMessage(), e );
+            throw new MavenFilteringException( ( filtering ? "filtering " : "copying " ) + from.getPath() + " to "

Review comment:
       To illustrate the effect of this PR some screenshots:
   
   1) error message before this PR
   
   ![image](https://user-images.githubusercontent.com/426959/104460261-23e7db00-55ae-11eb-8597-4ea95c5098dc.png)
   
   2) error message with this PR
   
   ![image](https://user-images.githubusercontent.com/426959/104460295-33672400-55ae-11eb-801a-091a62190773.png)
   




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-filtering] sebthom commented on pull request #15: [MRESOURCES-268] - Report path of offending file on copy/filter error.

GitBox
In reply to this post by GitBox

sebthom commented on pull request #15:
URL: https://github.com/apache/maven-filtering/pull/15#issuecomment-766320217


   @khmarbaise @hboutemy any thoughts?


----------------------------------------------------------------
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:
[hidden email]