You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/07/07 10:12:50 UTC
[GitHub] [maven-install-plugin] cstamas opened a new pull request, #32: Streamline the plugin
cstamas opened a new pull request, #32:
URL: https://github.com/apache/maven-install-plugin/pull/32
Original plugin made hoops and loops, instead to perform what it needed to perform. Partly to blame this was unfinished state of MAT API (it was able to install project only).
Installing project is needed in InstallMojo, but InstallFileMojo was forced to make hoops and loops due this, as it was passed one file (and maybe pomFile), and it was forced to create "fake" project, decorate and fake setup it with all whistle and bells, only to get it via MAT to resolver that would "decompose" it back into set of artifacts needing a deploy. So it went this file-artifact-project-artifact route, that made all the logic fragile and overly complicated.
This PR completely reworks m-install-p making it (almost trivially) simple: it does what it needs to do, without any fuss, and does it in streamlined way: InstallMojo will create a list of artifacts out of project and pass it to repository system for deploy, while InstallFileMojo literally prepares just a deployment request, nothing more. No fuss, no magic, no fake project building etc.
Note: the code in AbstractInstallMojo may or may not need to be reusable, but definitely smells like some "Maven API-ish thing".
Problems: InstallFileMojo implicitly implemented ID validation (thru fake project building), and it revealed the problem that Maven ID (groupId, artifactId) and version validation is deeply buried into maven-model-builder and is NOT reusable at all, hence a light copy of logic (rules for ID allowed characters and version forbidden characters) are copied over here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] cstamas commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917761382
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -158,48 +165,36 @@
@Parameter( property = "localRepositoryPath" )
private File localRepositoryPath;
- /**
- * Used for attaching the artifacts to install to the project.
- */
- @Component
- private MavenProjectHelper projectHelper;
-
- /**
- * Used for creating the project to which the artifacts to install will be attached.
- */
- @Component
- private ProjectBuilder projectBuilder;
-
- /**
- * Used to install the project created.
- */
- @Component
- private ProjectInstaller installer;
-
- /**
- * @see org.apache.maven.plugin.Mojo#execute()
- */
+ @Override
public void execute()
throws MojoExecutionException, MojoFailureException
{
-
if ( !file.exists() )
{
String message = "The specified file '" + file.getPath() + "' does not exist";
getLog().error( message );
throw new MojoFailureException( message );
}
- ProjectBuildingRequest buildingRequest = session.getProjectBuildingRequest();
-
- // ----------------------------------------------------------------------
- // Override the default localRepository variable
- // ----------------------------------------------------------------------
+ RepositorySystemSession repositorySystemSession = session.getRepositorySession();
if ( localRepositoryPath != null )
{
- buildingRequest = repositoryManager.setLocalRepositoryBasedir( buildingRequest, localRepositoryPath );
-
- getLog().debug( "localRepoPath: " + repositoryManager.getLocalRepositoryBasedir( buildingRequest ) );
+ // "clone" repository session and replace localRepository
+ DefaultRepositorySystemSession newSession = new DefaultRepositorySystemSession(
+ session.getRepositorySession() );
+ // Clear cache, since we're using a new local repository
+ newSession.setCache( new DefaultRepositoryCache() );
+ // keep same repositoryType
+ String contentType = newSession.getLocalRepository().getContentType();
+ if ( "enhanced".equals( contentType ) )
+ {
+ contentType = "default";
+ }
Review Comment:
https://issues.apache.org/jira/browse/MRESOLVER-267
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] cstamas merged pull request #32: [MINSTALL-177] Streamline the plugin
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] cstamas commented on pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#issuecomment-1179428535
Ping? Anyone?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] cstamas commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917684531
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -158,48 +165,36 @@
@Parameter( property = "localRepositoryPath" )
private File localRepositoryPath;
- /**
- * Used for attaching the artifacts to install to the project.
- */
- @Component
- private MavenProjectHelper projectHelper;
-
- /**
- * Used for creating the project to which the artifacts to install will be attached.
- */
- @Component
- private ProjectBuilder projectBuilder;
-
- /**
- * Used to install the project created.
- */
- @Component
- private ProjectInstaller installer;
-
- /**
- * @see org.apache.maven.plugin.Mojo#execute()
- */
+ @Override
public void execute()
throws MojoExecutionException, MojoFailureException
{
-
if ( !file.exists() )
{
String message = "The specified file '" + file.getPath() + "' does not exist";
getLog().error( message );
throw new MojoFailureException( message );
}
- ProjectBuildingRequest buildingRequest = session.getProjectBuildingRequest();
-
- // ----------------------------------------------------------------------
- // Override the default localRepository variable
- // ----------------------------------------------------------------------
+ RepositorySystemSession repositorySystemSession = session.getRepositorySession();
if ( localRepositoryPath != null )
{
- buildingRequest = repositoryManager.setLocalRepositoryBasedir( buildingRequest, localRepositoryPath );
-
- getLog().debug( "localRepoPath: " + repositoryManager.getLocalRepositoryBasedir( buildingRequest ) );
+ // "clone" repository session and replace localRepository
+ DefaultRepositorySystemSession newSession = new DefaultRepositorySystemSession(
+ session.getRepositorySession() );
+ // Clear cache, since we're using a new local repository
+ newSession.setCache( new DefaultRepositoryCache() );
+ // keep same repositoryType
+ String contentType = newSession.getLocalRepository().getContentType();
+ if ( "enhanced".equals( contentType ) )
+ {
+ contentType = "default";
+ }
Review Comment:
This is resolver (actually one of oldest Sonatype Aether) thing: the "enhanced" repository was introduced after the "simple" repository and was promoted as default (so priority-wise both act on "default" content type, but enhanced has higher priority), and enhanced repo reports content type enhanced. Is mess and inconsistent, but this is like it due legacy (in resolver).
Refs:
https://github.com/apache/maven-resolver/blob/master/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManagerFactory.java#L98
https://github.com/apache/maven-resolver/blob/master/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java#L79
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] slawekjaranowski commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917258610
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -158,48 +165,36 @@
@Parameter( property = "localRepositoryPath" )
private File localRepositoryPath;
- /**
- * Used for attaching the artifacts to install to the project.
- */
- @Component
- private MavenProjectHelper projectHelper;
-
- /**
- * Used for creating the project to which the artifacts to install will be attached.
- */
- @Component
- private ProjectBuilder projectBuilder;
-
- /**
- * Used to install the project created.
- */
- @Component
- private ProjectInstaller installer;
-
- /**
- * @see org.apache.maven.plugin.Mojo#execute()
- */
+ @Override
public void execute()
throws MojoExecutionException, MojoFailureException
{
-
if ( !file.exists() )
{
String message = "The specified file '" + file.getPath() + "' does not exist";
getLog().error( message );
throw new MojoFailureException( message );
}
- ProjectBuildingRequest buildingRequest = session.getProjectBuildingRequest();
-
- // ----------------------------------------------------------------------
- // Override the default localRepository variable
- // ----------------------------------------------------------------------
+ RepositorySystemSession repositorySystemSession = session.getRepositorySession();
if ( localRepositoryPath != null )
{
- buildingRequest = repositoryManager.setLocalRepositoryBasedir( buildingRequest, localRepositoryPath );
-
- getLog().debug( "localRepoPath: " + repositoryManager.getLocalRepositoryBasedir( buildingRequest ) );
+ // "clone" repository session and replace localRepository
+ DefaultRepositorySystemSession newSession = new DefaultRepositorySystemSession(
+ session.getRepositorySession() );
+ // Clear cache, since we're using a new local repository
+ newSession.setCache( new DefaultRepositoryCache() );
+ // keep same repositoryType
+ String contentType = newSession.getLocalRepository().getContentType();
+ if ( "enhanced".equals( contentType ) )
+ {
+ contentType = "default";
+ }
Review Comment:
in comments `keep same repositoryType` but change from `enhanced` to `default`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] cstamas commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917692432
##########
src/main/java/org/apache/maven/plugins/install/InstallMojo.java:
##########
@@ -153,31 +163,90 @@ private boolean allProjectsMarked()
return true;
}
- private void installProject( MavenProject pir )
- throws MojoFailureException, MojoExecutionException
+ private void installProject( MavenProject project ) throws MojoExecutionException, MojoFailureException
{
try
{
- installer.install( session.getProjectBuildingRequest(), new ProjectInstallerRequest().setProject( pir ) );
+ repositorySystem.install( session.getRepositorySession(), processProject( project ) );
}
- catch ( IOException e )
+ catch ( IllegalArgumentException e )
{
- throw new MojoFailureException( "IOException", e );
+ throw new MojoFailureException( e.getMessage(), e );
Review Comment:
Hm, we need to clean up this I guess.... (as now I see that I used the two inconsistently).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] slawekjaranowski commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917772366
##########
src/main/java/org/apache/maven/plugins/install/InstallMojo.java:
##########
@@ -153,31 +163,90 @@ private boolean allProjectsMarked()
return true;
}
- private void installProject( MavenProject pir )
- throws MojoFailureException, MojoExecutionException
+ private void installProject( MavenProject project ) throws MojoExecutionException, MojoFailureException
{
try
{
- installer.install( session.getProjectBuildingRequest(), new ProjectInstallerRequest().setProject( pir ) );
+ repositorySystem.install( session.getRepositorySession(), processProject( project ) );
}
- catch ( IOException e )
+ catch ( IllegalArgumentException e )
{
- throw new MojoFailureException( "IOException", e );
+ throw new MojoFailureException( e.getMessage(), e );
Review Comment:
I've started discussion on dev list about it 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-install-plugin] slawekjaranowski commented on a diff in pull request #32: Streamline the plugin
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #32:
URL: https://github.com/apache/maven-install-plugin/pull/32#discussion_r917258353
##########
src/main/java/org/apache/maven/plugins/install/InstallMojo.java:
##########
@@ -153,31 +163,90 @@ private boolean allProjectsMarked()
return true;
}
- private void installProject( MavenProject pir )
- throws MojoFailureException, MojoExecutionException
+ private void installProject( MavenProject project ) throws MojoExecutionException, MojoFailureException
{
try
{
- installer.install( session.getProjectBuildingRequest(), new ProjectInstallerRequest().setProject( pir ) );
+ repositorySystem.install( session.getRepositorySession(), processProject( project ) );
}
- catch ( IOException e )
+ catch ( IllegalArgumentException e )
{
- throw new MojoFailureException( "IOException", e );
+ throw new MojoFailureException( e.getMessage(), e );
Review Comment:
why not `MojoExecutionException`
I must find a discussion, but as I remember `MojoFailureException` should not be used ....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org