Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSHARED-1080] Parent POM 36, Java8, drop legacy. #38

Merged
merged 6 commits into from
Jun 11, 2022
Merged

[MSHARED-1080] Parent POM 36, Java8, drop legacy. #38

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dedca2a
[MSHARED-1080] Parent POM 36, Java8, drop legacy.
cstamas Jun 8, 2022
d026df4
WIP
cstamas Jun 9, 2022
e0f0ed6
WIP
cstamas Jun 9, 2022
b21a566
Fix UT
cstamas Jun 9, 2022
4e16d70
Raise from the dead, neglect the smell.
cstamas Jun 10, 2022
0158eb9
Applied PR comments
cstamas Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 28 additions & 31 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
<parent>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-shared-components</artifactId>
<version>34</version>
<version>36</version>
<relativePath/>
</parent>

<artifactId>maven-filtering</artifactId>
Expand Down Expand Up @@ -55,7 +56,8 @@

<properties>
<mavenVersion>3.2.5</mavenVersion>
<javaVersion>7</javaVersion>
<slf4jVersion>1.7.36</slf4jVersion>
<javaVersion>8</javaVersion>
cstamas marked this conversation as resolved.
Show resolved Hide resolved
<project.build.outputTimestamp>2020-08-05T15:18:01Z</project.build.outputTimestamp>
</properties>

Expand All @@ -66,20 +68,33 @@
</contributors>

<dependencies>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>${slf4jVersion}</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-model</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-settings</artifactId>
<version>${mavenVersion}</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.eclipse.sisu</groupId>
Expand All @@ -90,15 +105,10 @@
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-annotations</artifactId>
</dependency>
<dependency>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-shared-utils</artifactId>
<version>3.3.4</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.3.0</version><!-- Java 7 -->
<version>3.4.2</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
Expand All @@ -108,25 +118,13 @@
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.6</version><!-- Java 7 -->
</dependency>
<dependency>
<groupId>org.sonatype.plexus</groupId>
<artifactId>plexus-build-api</artifactId>
<version>0.0.7</version>
</dependency>
<dependency>
<groupId>org.sonatype.plexus</groupId>
<artifactId>plexus-build-api</artifactId>
<version>0.0.7</version>
<scope>test</scope>
<classifier>tests</classifier>
<version>2.11.0</version>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>2.28.2</version><!-- Java 7 -->
<version>4.5.1</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -141,21 +139,20 @@
<version>2.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>${slf4jVersion}</version>
<scope>test</scope>
</dependency>

</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-metadata</artifactId>
<executions>
<execution>
<goals>
<goal>generate-metadata</goal>
</goals>
</execution>
</executions>
<groupId>org.eclipse.sisu</groupId>
<artifactId>sisu-maven-plugin</artifactId>
michael-o marked this conversation as resolved.
Show resolved Hide resolved
</plugin>
</plugins>
</build>
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/apache/maven/shared/filtering/BaseFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.maven.execution.MavenSession;
import org.apache.maven.project.MavenProject;
import org.apache.maven.settings.Settings;
import org.apache.maven.shared.utils.StringUtils;
import org.apache.maven.shared.utils.io.FileUtils;
import org.codehaus.plexus.interpolation.InterpolationPostProcessor;
import org.codehaus.plexus.interpolation.Interpolator;
Expand Down Expand Up @@ -195,7 +194,7 @@ void loadProperties( Properties filterProperties, File basedir, List<String> pro

for ( String filterFile : propertiesFilePaths )
{
if ( StringUtils.isEmpty( filterFile ) )
if ( filterFile == null || filterFile.trim().isEmpty() )
{
// skip empty file name
continue;
Expand Down
25 changes: 14 additions & 11 deletions src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
* under the License.
*/

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;

import java.io.File;
import java.io.IOException;
import java.util.List;
Expand All @@ -27,24 +31,25 @@
import org.apache.maven.project.MavenProject;
import org.apache.maven.shared.utils.io.FileUtils;
import org.apache.maven.shared.utils.io.FileUtils.FilterWrapper;
import org.codehaus.plexus.component.annotations.Component;
import org.codehaus.plexus.component.annotations.Requirement;
import org.sonatype.plexus.build.incremental.BuildContext;

import static java.util.Objects.requireNonNull;

/**
* @author Olivier Lamy
*/
@Component( role = org.apache.maven.shared.filtering.MavenFileFilter.class, hint = "default" )
@Singleton
@Named
public class DefaultMavenFileFilter
extends BaseFilter
implements MavenFileFilter
{
private final MavenReaderFilter readerFilter;

@Requirement
private MavenReaderFilter readerFilter;
michael-o marked this conversation as resolved.
Show resolved Hide resolved

@Requirement
private BuildContext buildContext;
@Inject
public DefaultMavenFileFilter( MavenReaderFilter readerFilter )
{
this.readerFilter = requireNonNull( readerFilter );
}

@Override
public void copyFile( File from, File to, boolean filtering, MavenProject mavenProject, List<String> filters,
Expand Down Expand Up @@ -105,8 +110,6 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt
}
FileUtils.copyFile( from, to, encoding, new FileUtils.FilterWrapper[0], overwrite );
}

buildContext.refresh( to );
Copy link
Member

Choose a reason for hiding this comment

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

isn't something used by m2e? or not anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

This comes from a repo that is last touched 12 years ago, and is archived. IMHO is dead, parrot dead, but we can tinker about it if you want 😄

Copy link
Member

Choose a reason for hiding this comment

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

a quick search on github shows this https://github.com/search?q=org%3Aeclipse-m2e+BuildContext&type=code
for sure this very old repo is just interface which doesn't need much changes so that's probably the reason.

disclaimer i'm not using m2e so... 🤣

Copy link
Member Author

Choose a reason for hiding this comment

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

You are missing the point: this parrot is dead. Very. Also, whatever it is, this was first attempt to implement incremental build, followed by two other attempts (by same person), that finally materialized in something completely different. So, yes, this may look "stable", the very same way dead parrot looks "stable" (well, not moving, is it?), but is superseded by several other attempts...

Copy link
Member

Choose a reason for hiding this comment

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

maybe but I can see references of it in the m2e code.
Is it really used or not? I have no idea as I'm not a contributor of m2e.
My opinion is just it worth to ask those folks as we may or not break something.
if not perfect but it worth the courtesy of simply asking.

Copy link
Member

Choose a reason for hiding this comment

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

it's not a rant. you point to a repo mentioning it's the logical successor.
whereas my point is by removing this you will affect all m2e users which is probably hundreds thousands even million(s) of users.
By pointing the repo I'm just saying m2e has a lot of contribution/maintenance and users compared to the repo you mention.
Even if I agree this was a bad idea/design (and as the guy who started the maven-filtering I didn't like it long time ago neither but it has been done...) but now it's here and use by a lot of people.
so we live with that or we discuss with m2e folks a replacement solution or at least ask them what are the impacts removing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆

Copy link
Member

Choose a reason for hiding this comment

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

If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆

no sorry but you do not read correctly, the most important is the number of users impacted.
and here in m2e is a lot!
what is better it's not the question here.
again I'm just saying the impacted users number is really huge!

Copy link
Member Author

Choose a reason for hiding this comment

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

frankly looking at commit history m2e has more maintenance than https://github.com/takari/io.takari.incrementalbuild (last commit almost 2 yo ago).

And who said this then?

Anyway, lets bury the boomerang, sisu build api is raised from the dead

Copy link
Member Author

Choose a reason for hiding this comment

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

... and my 5 cents to whole this topic: I never saw a "plain" (explained later) Maven project that is fairly more complex than "hello world" and works OOTB in m2e. Work, as in no "tweaks", "m2e profile in POM" needed etc. OTOH, I did saw Maven project using takari lifecycle that WORK perfectly and OOTB in m2e.

But alas, takari lifecycle use the "inferior" incrementalbuild and not buildcontext API. Oh, and it also replaces many things from Maven itself, and even ditches this maven-filtering and replaces it's with it's own to be functional 😄

("plain" as Maven build w/o any extension, while takari-lifecycle is an extension and replaces a LOT from maven, like core copmponets but also shared stuff like this maven-filtering and many plugins as well)

}
catch ( IOException e )
{
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/org/apache/maven/shared/filtering/DefaultMavenReaderFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@
* under the License.
*/

import javax.inject.Named;
import javax.inject.Singleton;

import java.io.Reader;
import java.util.Collections;
import java.util.List;

import org.apache.maven.execution.MavenSession;
import org.apache.maven.project.MavenProject;
import org.apache.maven.shared.utils.io.FileUtils.FilterWrapper;
import org.codehaus.plexus.component.annotations.Component;

/**
* @author Kristian Rosenvold
*/
@Component( role = org.apache.maven.shared.filtering.MavenReaderFilter.class, hint = "default" )
@Singleton
@Named
public class DefaultMavenReaderFilter
extends BaseFilter
implements MavenReaderFilter
Expand Down
Loading