[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17825223#comment-17825223 ] ASF GitHub Bot commented on MSHARED-1285: - OLibutzki commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1987934181 > The release is still under vote. @gnodet Thanks for pushing that release ahead! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17824793#comment-17824793 ] ASF GitHub Bot commented on MSHARED-1285: - reckart commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1985714575 @OLibutzki the development process of Apache projects generally is public - you can help by testing and casting your vote as well - just reply to the voting mail. However, unless you are part of the project management committee, you vote will not count as binding. A release vote is successful with at least 3 +1 votes after at least 72h and no -1 vote. See https://www.apache.org/foundation/voting.html > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17824791#comment-17824791 ] ASF GitHub Bot commented on MSHARED-1285: - reckart commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1985709598 https://lists.apache.org/list?d...@maven.apache.org:2024-3:vote > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17824790#comment-17824790 ] ASF GitHub Bot commented on MSHARED-1285: - OLibutzki commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1985701031 > The release is still under vote. Thanks for the feedback... this is not a publicly visible process, right? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17824787#comment-17824787 ] ASF GitHub Bot commented on MSHARED-1285: - gnodet commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1985677119 > > I'm doing a review of maven shared projects now and will do a release for this project also. > > Any progress, @slachiewicz? > > There is a [3.3.2 tag](https://github.com/apache/maven-filtering/releases/tag/maven-filtering-3.3.2), but the release did not find its way to Maven Central. Do you know why @gnodet? The release is still under vote. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17824697#comment-17824697 ] ASF GitHub Bot commented on MSHARED-1285: - OLibutzki commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1985381882 > I'm doing a review of maven shared projects now and will do a release for this project also. Any progress, @slachiewicz? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17806006#comment-17806006 ] ASF GitHub Bot commented on MSHARED-1285: - slachiewicz closed pull request #83: [MSHARED-1285] - incremental build does not honor `BuildContext#isUptodate()` URL: https://github.com/apache/maven-filtering/pull/83 > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17806003#comment-17806003 ] ASF GitHub Bot commented on MSHARED-1285: - slachiewicz commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1888918418 I'm doing a review of maven shared projects now and will do a release for this project also. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17805985#comment-17805985 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-138328 > > Do you have any approx. arrival date for a release including this fix? > > We do not plan releases. Someone has to stand up as RM for this. Then the question is maybe is there someone who can drive the release and is willing to do so ... > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17805978#comment-17805978 ] ASF GitHub Bot commented on MSHARED-1285: - michael-o commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-129990 > Do you have any approx. arrival date for a release including this fix? We do not plan releases. Someone has to stand up as RM for this. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Assignee: Elliotte Rusty Harold >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17805013#comment-17805013 ] ASF GitHub Bot commented on MSHARED-1285: - OLibutzki commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1884404140 Do you have any approx. arrival date for a release including this fix? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > Fix For: maven-filtering-3.3.2 > > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17798678#comment-17798678 ] ASF GitHub Bot commented on MSHARED-1285: - elharo merged PR #77: URL: https://github.com/apache/maven-filtering/pull/77 > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17798637#comment-17798637 ] ASF GitHub Bot commented on MSHARED-1285: - akuhtz commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1863016485 Can this be merged? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794700#comment-17794700 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420419510 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -189,20 +190,44 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr // as destination // see MNG-1345 File outputDirectory = mavenResourcesExecution.getOutputDirectory(); -boolean outputExists = outputDirectory.exists(); -if (!outputExists && !outputDirectory.mkdirs()) { +if (!outputDirectory.mkdirs() && !outputDirectory.exists()) { throw new MavenFilteringException("Cannot create resource output directory: " + outputDirectory); } if (resource.isFiltering()) { isFilteringUsed = true; } -boolean ignoreDelta = !outputExists -|| buildContext.hasDelta(mavenResourcesExecution.getFileFilters()) -|| buildContext.hasDelta(getRelativeOutputDirectory(mavenResourcesExecution)); -LOGGER.debug("ignoreDelta " + ignoreDelta); -Scanner scanner = buildContext.newScanner(resourceDirectory, ignoreDelta); +boolean filtersFileChanges = buildContext.hasDelta(mavenResourcesExecution.getFileFilters()); Review Comment: Done > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794697#comment-17794697 ] ASF GitHub Bot commented on MSHARED-1285: - elharo commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420408887 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -189,20 +190,44 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr // as destination // see MNG-1345 File outputDirectory = mavenResourcesExecution.getOutputDirectory(); -boolean outputExists = outputDirectory.exists(); -if (!outputExists && !outputDirectory.mkdirs()) { +if (!outputDirectory.mkdirs() && !outputDirectory.exists()) { throw new MavenFilteringException("Cannot create resource output directory: " + outputDirectory); } if (resource.isFiltering()) { isFilteringUsed = true; } -boolean ignoreDelta = !outputExists -|| buildContext.hasDelta(mavenResourcesExecution.getFileFilters()) -|| buildContext.hasDelta(getRelativeOutputDirectory(mavenResourcesExecution)); -LOGGER.debug("ignoreDelta " + ignoreDelta); -Scanner scanner = buildContext.newScanner(resourceDirectory, ignoreDelta); +boolean filtersFileChanges = buildContext.hasDelta(mavenResourcesExecution.getFileFilters()); Review Comment: filtersFileChanged? since "changes" sounds like it's the actual changes instead of a boolean indicating whether things changed > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794693#comment-17794693 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420403517 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -321,19 +347,20 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr } /** - * Get the encoding to use when filtering the specified file. Properties files can be configured to use a different - * encoding than regular files. + * Get the encoding to use when filtering the specified file. Properties files + * can be configured to use a different encoding than regular files. * - * @param file The file to check - * @param encoding The encoding to use for regular files + * @param file The file to check Review Comment: I now reverted all unrelated javadoc changes > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794690#comment-17794690 ] ASF GitHub Bot commented on MSHARED-1285: - elharo commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420397992 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -321,19 +347,20 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr } /** - * Get the encoding to use when filtering the specified file. Properties files can be configured to use a different - * encoding than regular files. + * Get the encoding to use when filtering the specified file. Properties files + * can be configured to use a different encoding than regular files. * - * @param file The file to check - * @param encoding The encoding to use for regular files + * @param file The file to check Review Comment: Did spotless do that? That's new, i think, and a strike against spotless if so. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794687#comment-17794687 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420394454 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -321,19 +347,20 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr } /** - * Get the encoding to use when filtering the specified file. Properties files can be configured to use a different - * encoding than regular files. + * Get the encoding to use when filtering the specified file. Properties files + * can be configured to use a different encoding than regular files. * - * @param file The file to check - * @param encoding The encoding to use for regular files + * @param file The file to check Review Comment: I use `spoltless:apply` to format the file according to project rules, so not fully sure how to proceed, do you want me to revert the format changes from spotles? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794679#comment-17794679 ] ASF GitHub Bot commented on MSHARED-1285: - elharo commented on code in PR #77: URL: https://github.com/apache/maven-filtering/pull/77#discussion_r1420388359 ## src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java: ## @@ -321,19 +347,20 @@ public void filterResources(MavenResourcesExecution mavenResourcesExecution) thr } /** - * Get the encoding to use when filtering the specified file. Properties files can be configured to use a different - * encoding than regular files. + * Get the encoding to use when filtering the specified file. Properties files + * can be configured to use a different encoding than regular files. * - * @param file The file to check - * @param encoding The encoding to use for regular files + * @param file The file to check Review Comment: Probably better not to try to line these up. It's too hard to maintain. But we should make the initial letter lower case in keeping with Oracle guideline > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794607#comment-17794607 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1846852881 All checks have passed! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794601#comment-17794601 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1846830645 @lalmeras sounds reasonable, I have now adjust the code accordingly (and slightly restructured the flow), now my local build shows all test passing! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug > Components: maven-filtering >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794080#comment-17794080 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1844896414 I think ignoreDelta for this case is a well-founded behavior. It checks specifically filter files (not filtered files). If source for filters values is changed, I think it is expected that all resources are refreshed, hence the ignoreDelta. isUptodate usage will not allow you to to reproduce this behavior during the scanning time. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794034#comment-17794034 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1844620007 @lalmeras thanks for clarification, thinking more about it I wonder if `ignoreDelta` is actually useful for anything given we use `isUptodate()`, because for me it looks like `ignoreDelta` is some hack to try to solve the issues we see here in some cases and are now fixed by using an isUptodate scanner... > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17793854#comment-17793854 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1843346851 They are test-cases that handle filter modification. Expected behavior (checked by these tests) is to perform a full-build (this is the case where ignoreDelta := true is delta is found on filters). I address this issue in my previous work by adapting your original fix (restoring ignoreDelta behavior, using ignoreDelta to adapt file scan) : see this commit https://github.com/lalmeras/maven-filtering/commit/2525912aca62de2affd6e0d80616722454bd24a6 (I posted it on #83 :-( ) > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17793661#comment-17793661 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #83: URL: https://github.com/apache/maven-filtering/pull/83#issuecomment-1842713444 This are test-cases that handles filter modification. Expected behavior is to perform a full-build (this is the case where ignoreDelta := true is delta is found on filters). I address this issue in my previous work by adapting your original fix (restoring ignoreDelta behavior, using ignoreDelta to adapt file scan) : see this commit https://github.com/lalmeras/maven-filtering/commit/2525912aca62de2affd6e0d80616722454bd24a6 > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17793573#comment-17793573 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1842459505 It currently fails on test-time will need to investigate what it wants here... > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17793538#comment-17793538 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #83: URL: https://github.com/apache/maven-filtering/pull/83#issuecomment-1842270748 I have now rebased my PR on top of your commit: - https://github.com/apache/maven-filtering/pull/77 > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792380#comment-17792380 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #83: URL: https://github.com/apache/maven-filtering/pull/83#issuecomment-1837142122 @lalmeras thanks for the PR on working on test-case and @slawekjaranowski for aproving the workflow run, so this verifies that the described test-case fail with the current implementation, I'll create a follow-up PR! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792377#comment-17792377 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #83: URL: https://github.com/apache/maven-filtering/pull/83#issuecomment-1837140014 PR only contains test to exhibit the issue. You can check with CI build that the two added test fail. Once the issue demonstrated, proposal from @laeubi in #77 can be applied as a fix. Thanks to @laeubi for diagnostic and explanations. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792378#comment-17792378 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837140171 @laeubi PR #83 with failing test cases and updated commit messages. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792376#comment-17792376 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras opened a new pull request, #83: URL: https://github.com/apache/maven-filtering/pull/83 (this is a follow-up to #77) Current incremental build does not honor isUptodate(), so changed or removed target resources are not refreshed until sources are modified or a full build is performed. 2 new testcases exhibits this issue (one for missing target resource, one for modified target resource). As stub BuildContext provides appropriate isUptodate results, build should refresh this resources. Current implementation prevents BuildContext implementors to trigger appropriate resource refresh by tweaking isUptodate implementation. See javadoc in BuildContext#newScanner javadoc that advices that incremental build may be performed with a full resource scanning and a isUptodate call to refresh only needed resources. Following this checklist to help us incorporate your contribution quickly and easily: - [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MSHARED) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [x] Each commit in the pull request should have a meaningful subject line and body. - [x] Format the pull request title like `[MSHARED-XXX] - Fixes bug in ApproximateQuantiles`, where you replace `MSHARED-XXX` with the appropriate JIRA issue. Best practice is to use the JIRA issue title in the pull request title and in the first line of the commit message. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Run `mvn clean verify -Prun-its` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure please ask on the developers list. To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) you have to acknowledge this by using the following check-box. - [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792370#comment-17792370 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837123822 @lalmeras can you open a PR here so we see the test is currently failing? I'll then fetch the PR and add my one on top of it to prove it fixes the failing test! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792369#comment-17792369 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837123168 Thank you for your feedback. @laeubi Do you want to update your PR with my proposal so we now both have unit test demonstrating your use case (and failing with the current codebase) and your fix. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792360#comment-17792360 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837114983 @lalmeras I think you are right that m2e assumes that if the target is newer than the source it is up-todate... but I think that can be ignored for now as this is an implementation detail of the context, the important part is that if `isUptodate()` return `false`, the file should be copied again. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792357#comment-17792357 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837113152 @laeubi Can you check https://github.com/lalmeras/maven-filtering/commit/dbe13673489c61a5c277b507310083bdcaa87c92#diff-a52a7b8a0ff570c181fe18ab238b693c21243d40870dd2d8178340247755bba5R152 ? Pay attention that my test assumes that BuildContext can reply appropriately `#isUptodate = false` for the updated target file. My stub handles this correctly, but I think that eclipse inplementation of BuildContext will not provide the expected result : https://github.com/eclipse-m2e/m2e-core/blob/348ef88b0d92833eff038d721a98ccf50e9ef4fd/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/plexusbuildapi/EclipseIncrementalBuildContext.java#L230 Am I missing something ? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792317#comment-17792317 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1837037738 @lalmeras great to see you make progress on this :+1: For the second case assume the following: 1. A resource (e.g. `source.txt`) was recently copied to the output folder `output` 2. There is another resource (e.g. `source2.txt`) also copied to the output folder `output` 3. Now it happens that `output/source.txt` is modified e.g. by the user outside the scope of changes 4. Now `source2.txt` is modified and only this edit is part of the change scope What happens now is that `source2.txt` is copied correctly to folder `output` as it is part of the changed files, but `source.txt` is not copied even though it is out-of-date with `output/source.txt`. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17792200#comment-17792200 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1836797752 I update my branch https://github.com/lalmeras/maven-filtering/commits/MSHARED-1285 with the following change : * rebased on origin/master with the refactored tests * I added a test-case demonstrating the usecase described by @laeubi : if target files are removed, an incremental build fails to regenerate missing resources > Another case is that you modify the target file (but not the source) then the mojo will never update that file as long as one do not modify the source file. @laeubi I'm not sure to understand what is the current / expected behavior for this use-case. Can you give a more precise description ? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17791554#comment-17791554 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1833477761 Done ! #82 > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17791436#comment-17791436 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1833089113 > I do not open another PR; feel free to integrate my proposal in this PR. @lalmeras just open a PR for the testcases I think that can be easier to manage also we then have direct PR validation! > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17791384#comment-17791384 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1832907067 Here is a branch with my proposal : https://github.com/lalmeras/maven-filtering/commits/MSHARED-1285 * first commit is a test refactor applied to existing master codebase * second commit applies @laeubi proposal. I restore original ignoreDelta when filters or output directory trigger *hasDelta=true*, as this is an expected behaviour from tests Both commits built with success with java 8 / java 17 / mvn 3.8. I finally understand why old tests work. It is because they relied only on Scanner behavior, and not isUptodate / hasDelta methods. So they accommodate nicely with the buggy TestIncrementalBuildContext. No work done on this issue's testcase. I can work on it if this first proposal seems OK. I do not open another PR; feel free to integrate my proposal in this PR. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17790998#comment-17790998 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1831557819 Already done it (copy/fix TestIncrementalBuildContext). It doesn't work out of the box as there is other tests that rely on this bug to work. And another bug in getRelpath. But I agree the best option is to retrieve/fix/adapt TestIncrementalBuildContext so that the 4 tests that use it work and actually test the effective filtering behavior. Just not as simple as it seems. I plan to work on it. I'll keep this issue updated about my work on it. Once the tests fixed, I think it'll be easier to write a test exposing the current issue. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17790895#comment-17790895 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1831210021 @lalmeras many many thanks for looking into the test! > So the issue for this test failure is in TestIncrementalBuildContext (org.sonatype.plexus:plexus-build-api:tests). Not sure how to handle this. This is actually deprecated/removed in later versions exactly because no one can know what the test wants to assert... so the best would be to simply copy it in the plugin here and adjust as needed, or even better using a mock. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17790787#comment-17790787 ] ASF GitHub Bot commented on MSHARED-1285: - lalmeras commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1830852620 I dig into the test issue. Test verifies the following behavior: * two files with resource filtering are filtered; both files are copied to target with `${time}` replaced by `time` * one file is touched and an incremental build with time=notime is launched * expected behavior is that only the touched file is updated; other file keeps time value (instead of notime) * after proposed fix, both files are processed I think there is an issue with the test implementation. `TestIncrementalBuildContext.isUptodate(File, File)` (that is called by custom DirectoryScanner#isSelected, cf `buildContext.isUptodate(...)`) implementation always return false, because `hasDelta(target)` always return true, because hasDelta fails to resolve target path as a source path (cf getRelpath). Not sure why, but the original code does not trigger `TestIncrementalBuildContext.isUptodate()`. Other tests from project does not call this method either. So the issue for this test failure is in TestIncrementalBuildContext (org.sonatype.plexus:plexus-build-api:tests). Not sure how to handle this. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17788310#comment-17788310 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1820426240 > @laeubi would it be possible to add a unit test that showcases the issue and the proposed fix? It should be possible to reproduce the problem a testcase should be setup like this: 1. `BuildContext` reports an incremental build 2. There should be resource `A` + `B` 3. The resource target folder is empty 4. `BuildContext` only contains a change for `A` Result is only `A` is copied (as it has a change) but `B` is missing because the Mojo assumes that a file in resource target can never vanish and do assume it will get a change event for that, what can not be guaranteed as a file can always be deleted out of the control of maven / IDE /... Another case is that you *modify* the target file (but not the source) then the mojo will never update that file as long as one do not modify the source file. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17788309#comment-17788309 ] ASF GitHub Bot commented on MSHARED-1285: - mickaelistria commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1820418224 @laeubi would it be possible to add a unit test that showcases the issue and the proposed fix? > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17781394#comment-17781394 ] ASF GitHub Bot commented on MSHARED-1285: - pcdavid commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1787352103 FWIW, the (single) test failure I get with this PR is this one: ``` [ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.162 s <<< FAILURE! - in org.apache.maven.shared.filtering.IncrementalResourceFilteringTest [ERROR] org.apache.maven.shared.filtering.IncrementalResourceFilteringTest.testSimpleIncrementalFiltering Time elapsed: 0.031 s <<< FAILURE! junit.framework.ComparisonFailure: expected:<[]time> but was:<[no]time> at junit.framework.Assert.assertEquals(Assert.java:100) at junit.framework.Assert.assertEquals(Assert.java:107) at junit.framework.TestCase.assertEquals(TestCase.java:260) at org.apache.maven.shared.filtering.IncrementalResourceFilteringTest.assertTime(IncrementalResourceFilteringTest.java:158) at org.apache.maven.shared.filtering.IncrementalResourceFilteringTest.testSimpleIncrementalFiltering(IncrementalResourceFilteringTest.java:69) ``` I don't know anything about this project/codebase, but as an Eclipse user impacted by [this](https://github.com/eclipse-m2e/m2e-core/issues/1302) I was curious, and the last PR build is too old and the failure logs are not available anymore. > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17778392#comment-17778392 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1774104149 > Is there any way to do thsi that doesn't use plexus at all? E.g. something in commons-io instead? obviously not... > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17773241#comment-17773241 ] ASF GitHub Bot commented on MSHARED-1285: - OLibutzki commented on PR #77: URL: https://github.com/apache/maven-filtering/pull/77#issuecomment-1752774319 Any chances to get some progress into this PR? From an Eclipse user perspective this issue is really annoying as some resources are not copied to the target/classes folder. //cc @khmarbaise > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MSHARED-1285) DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes
[ https://issues.apache.org/jira/browse/MSHARED-1285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17740673#comment-17740673 ] ASF GitHub Bot commented on MSHARED-1285: - laeubi opened a new pull request, #77: URL: https://github.com/apache/maven-filtering/pull/77 Currently it could happen that the scanner misses changed files (because they are not part of the delta) or copies files even if they have not changed (e.g. because the output has changes). This uses now a different approach, instead of only handling the delta files, we scan all inputs and compare if they are up-to-date with the output. Following this checklist to help us incorporate your contribution quickly and easily: - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MSHARED) filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes. - [ ] Each commit in the pull request should have a meaningful subject line and body. - [ ] Format the pull request title like `[MSHARED-XXX] - Fixes bug in ApproximateQuantiles`, where you replace `MSHARED-XXX` with the appropriate JIRA issue. Best practice is to use the JIRA issue title in the pull request title and in the first line of the commit message. - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [ ] Run `mvn clean verify -Prun-its` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically. If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure please ask on the developers list. To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) you have to acknowledge this by using the following check-box. - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0) - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf). > DefaultMavenResourcesFiltering uses BuildContext in a way that fails sometimes > -- > > Key: MSHARED-1285 > URL: https://issues.apache.org/jira/browse/MSHARED-1285 > Project: Maven Shared Components > Issue Type: Bug >Reporter: Christoph Läubrich >Priority: Major > > The maven resources plugin uses > [https://github.com/apache/maven-filtering/blob/master/src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java] > to copy resources, but that component has some subtile flaws reported here: > [https://github.com/eclipse-m2e/m2e-core/discussions/1468] > The problematic part is the usage of BuildContext#newScanner that already > mentions in the javadoc that passing ignoreDelta to neScanner might not > reveal all required items +*for copy-resources*+ form A -> B and instead > BuildContext#isUpTodate should be used. > Just from a quick look I assume that part of code actually wants to use > something like this: > {code:java} > DirectoryScanner ds = new DirectoryScanner() { > @Override > protected boolean isSelected(String name, File file) { > if (file.isFile() && buildContext.isUptodate(getTargetFile(file), file)) > { > return false; > } > return true; > } > }; > ds.setBasedir(basedir); {code} > That way all the code that currently checks for if output directory existed > before can also be removed what is another issue because it leads to a full > resources copy even if source+target are already even if a single class file > is changed while the idea seems more that if the output was not there it > should not try to be incremental! -- This message was sent by Atlassian Jira (v8.20.10#820010)