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/04/29 14:39:30 UTC
[GitHub] [maven-release] cstamas opened a new pull request, #118: Catch up
cstamas opened a new pull request, #118:
URL: https://github.com/apache/maven-release/pull/118
So, nudged the neglected plugin:
* off plexus XML (it had _logic_ in there!), fully to JSR330
* off ancient sonatype mvn 3.0 packages to mvn 3.2.5
* off from Junit3 PlexusTestCase w/ XMLs to modern(er) Junit 4 and no XMLs
* fixed all the "tweaks" happened in well hidden XML
* reformat
* cheers!
All but two UTs pass that I cannot interpret:
https://github.com/apache/maven-release/blob/85d973c87724aa2bc0698e6773dd9ce2a0fc8701/maven-release-manager/src/test/java/org/apache/maven/shared/release/versions/DefaultVersionInfoTest.java#L235-L239
https://github.com/apache/maven-release/blob/85d973c87724aa2bc0698e6773dd9ce2a0fc8701/maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/GenerateReleasePomsPhaseTest.java
Former currently does not even run (maven version != 3.0), while latter is most probably due bad diff, probably diff logic in UT needs more "no change" detection.
All but one IT pass:
[INFO] Building: projects/prepare/oddeven-policy/pom.xml
[INFO] The build exited with code 1. See /home/cstamas/Worx/apache-maven/maven-release/maven-release-plugin/target/it/projects/prepare/oddeven-policy/build.log for details.
[INFO] projects/prepare/oddeven-policy/pom.xml .......... FAILED (1.2 s)
But this is due aether-util missing from classpath, will be fixed.
--
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-release] michael-o commented on pull request #118: [MRELEASE-1087] Upgrade Maven to 3.2.5 (and de-plexus)
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1119311196
> LGTM: a good pass at cleanup that will prepare for next ones I hope M6 will be the last milestone and we can after go to final
I assume that SCM 2 will be in MRELEASE 4 only, no?
--
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-release] olamy commented on a diff in pull request #118: [MRELEASE-1087] Upgrade Maven to 3.2.5 (and de-plexus)
Posted by GitBox <gi...@apache.org>.
olamy commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r866438748
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
+
+ private final Map<String, Strategy> strategies;
/**
* The available phases.
*/
- @Requirement
- private Map<String, ReleasePhase> releasePhases;
+ private final Map<String, ReleasePhase> releasePhases;
/**
* The configuration storage.
*/
- @Requirement( hint = "properties" )
- private ReleaseDescriptorStore configStore;
+ private final AtomicReference<ReleaseDescriptorStore> configStore;
Review Comment:
oh ok I saw that too late https://github.com/apache/maven-release/pull/118#discussion_r863535359
seems still to be over complex this AtomicRef but if it works...
--
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-release] cstamas commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1114655576
There is one problem: all UT and IT pass OK **except this one that is disabled (hence build is green)**: [GenerateReleasePomsPhaseTest](https://github.com/apache/maven-release/pull/118/commits/3c6319f95ec5f5384fa33407be8800a81da25095)
I have hard time even to understand what this UT is about, and even less clues WHY it fails, seems it has to do how POM is persisted and some change probably between 3.0 and 3.2 "ways of doing it". So here I'd expect some explanation and help from @hboutemy @rfscholte and @michael-o to figure it out.
--
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-release] slawekjaranowski commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1117963562
`maven-model-builder/src/main/resources/org/apache/maven/model/pom-4.0.0.xml` was changed so we need change `expected-*-pom.xml` for UT.
--
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-release] cstamas commented on pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1118960807
@rfscholte or @hboutemy would be good if any of you would bless this PR so we can merge. Again, as before, if there is any other PR scheduled, please merge it BEFORE this one, I will handle conflicts.
But, this PR clearly shows severe issues with m-release-p:
* why does release redo Maven CLI? (see use of commons-cli), this is obviously a duplication of logic here
* why does release resolve anything at all? Several of us have hard time to understand what is happening here
All in all, I still think what I said before: "this plugin does WAAY more than it should", and hence, is very fragile (the plugin but it's process as well).
Anyway, let's move on, and we can tinker more about these later.
--
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-release] olamy commented on a diff in pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
olamy commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r866433670
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
+
+ private final Map<String, Strategy> strategies;
/**
* The available phases.
*/
- @Requirement
- private Map<String, ReleasePhase> releasePhases;
+ private final Map<String, ReleasePhase> releasePhases;
/**
* The configuration storage.
*/
- @Requirement( hint = "properties" )
- private ReleaseDescriptorStore configStore;
+ private final AtomicReference<ReleaseDescriptorStore> configStore;
Review Comment:
what is the need for using `AtomicReference`?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
Review Comment:
nit. any reason to use a different pattern for this one?
another change use `private final Logger logger = LoggerFactory.getLogger( getClass() );`
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractInputVariablesPhase.java:
##########
@@ -0,0 +1,309 @@
+package org.apache.maven.shared.release.phase;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.maven.artifact.ArtifactUtils;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.scm.manager.NoSuchScmProviderException;
+import org.apache.maven.scm.provider.ScmProvider;
+import org.apache.maven.scm.repository.ScmRepository;
+import org.apache.maven.scm.repository.ScmRepositoryException;
+import org.apache.maven.shared.release.ReleaseExecutionException;
+import org.apache.maven.shared.release.ReleaseResult;
+import org.apache.maven.shared.release.config.ReleaseDescriptor;
+import org.apache.maven.shared.release.env.ReleaseEnvironment;
+import org.apache.maven.shared.release.policy.PolicyException;
+import org.apache.maven.shared.release.policy.naming.NamingPolicy;
+import org.apache.maven.shared.release.policy.naming.NamingPolicyRequest;
+import org.apache.maven.shared.release.scm.ReleaseScmRepositoryException;
+import org.apache.maven.shared.release.scm.ScmRepositoryConfigurator;
+import org.apache.maven.shared.release.util.ReleaseUtil;
+import org.codehaus.plexus.components.interactivity.Prompter;
+import org.codehaus.plexus.components.interactivity.PrompterException;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.Interpolator;
+import org.codehaus.plexus.interpolation.PrefixAwareRecursionInterceptor;
+import org.codehaus.plexus.interpolation.PrefixedPropertiesValueSource;
+import org.codehaus.plexus.interpolation.RecursionInterceptor;
+import org.codehaus.plexus.interpolation.StringSearchInterpolator;
+import org.codehaus.plexus.util.StringUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Input any variables that were not yet configured.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public abstract class AbstractInputVariablesPhase
+ extends AbstractReleasePhase
+{
+ /**
+ * Component used to prompt for input.
+ */
+ private final AtomicReference<Prompter> prompter;
+
+ /**
+ * Tool that gets a configured SCM repository from release configuration.
+ */
+ private final ScmRepositoryConfigurator scmRepositoryConfigurator;
+
+ /**
+ * Component used for custom or default naming policy
+ */
+ private final Map<String, NamingPolicy> namingPolicies;
+
+ /**
+ * Whether this is a branch or a tag operation.
+ */
+ private final boolean branchOperation;
+
+ /**
+ * The default naming policy to apply, if any
+ */
+ private final String defaultNamingPolicy;
+
+ protected AbstractInputVariablesPhase( Prompter prompter, ScmRepositoryConfigurator scmRepositoryConfigurator,
+ Map<String, NamingPolicy> namingPolicies, boolean branchOperation,
+ String defaultNamingPolicy )
+ {
+ this.prompter = new AtomicReference<>( requireNonNull( prompter ) );
+ this.scmRepositoryConfigurator = requireNonNull( scmRepositoryConfigurator );
+ this.namingPolicies = requireNonNull( namingPolicies );
+ this.branchOperation = branchOperation;
+ this.defaultNamingPolicy = defaultNamingPolicy;
+ }
+
+ /**
+ * For easier testing only!
+ */
+ public void setPrompter( Prompter prompter )
+ {
+ this.prompter.set( prompter );
+ }
+
+ boolean isBranchOperation()
+ {
+ return branchOperation;
+ }
+
+ /**
+ * <p>getScmProvider.</p>
+ *
+ * @param releaseDescriptor a {@link ReleaseDescriptor} object
+ * @param releaseEnvironment a {@link ReleaseEnvironment} object
+ * @return a {@link ScmProvider} object
+ * @throws ReleaseScmRepositoryException if any.
+ * @throws ReleaseExecutionException if any.
+ */
+ protected ScmProvider getScmProvider( ReleaseDescriptor releaseDescriptor, ReleaseEnvironment releaseEnvironment )
+ throws ReleaseScmRepositoryException, ReleaseExecutionException
+ {
+ try
+ {
+ ScmRepository repository =
+ scmRepositoryConfigurator.getConfiguredRepository( releaseDescriptor,
+ releaseEnvironment.getSettings() );
+
+ return scmRepositoryConfigurator.getRepositoryProvider( repository );
+ }
+ catch ( ScmRepositoryException e )
+ {
+ throw new ReleaseScmRepositoryException(
+ e.getMessage() + " for URL: " + releaseDescriptor.getScmSourceUrl(), e.getValidationMessages() );
+ }
+ catch ( NoSuchScmProviderException e )
+ {
+ throw new ReleaseExecutionException( "Unable to configure SCM repository: " + e.getMessage(), e );
+ }
+ }
+
+ @Override
+ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnvironment releaseEnvironment,
+ List<MavenProject> reactorProjects )
+ throws ReleaseExecutionException
+ {
+ ReleaseResult result = new ReleaseResult();
+
+ // get the root project
+ MavenProject project = ReleaseUtil.getRootProject( reactorProjects );
+
+ String tag = releaseDescriptor.getScmReleaseLabel();
+
+ if ( tag == null )
+ {
+ // Must get default version from mapped versions, as the project will be the incorrect snapshot
+ String key = ArtifactUtils.versionlessKey( project.getGroupId(), project.getArtifactId() );
+ String releaseVersion = releaseDescriptor.getProjectReleaseVersion( key );
+ if ( releaseVersion == null )
+ {
+ throw new ReleaseExecutionException( "Project tag cannot be selected if version is not yet mapped" );
+ }
+
+ String suggestedName;
+ String scmTagNameFormat = releaseDescriptor.getScmTagNameFormat();
+ if ( releaseDescriptor.getProjectNamingPolicyId() != null )
+ {
+ try
+ {
+ suggestedName =
+ resolveSuggestedName( releaseDescriptor.getProjectNamingPolicyId(), releaseVersion,
+ project );
+ }
+ catch ( PolicyException e )
+ {
+ throw new ReleaseExecutionException( e.getMessage(), e );
+ }
+ }
+ else if ( scmTagNameFormat != null )
+ {
+ Interpolator interpolator = new StringSearchInterpolator( "@{", "}" );
+ List<String> possiblePrefixes = java.util.Arrays.asList( "project", "pom" );
Review Comment:
nit no need of full package name
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractInputVariablesPhase.java:
##########
@@ -0,0 +1,309 @@
+package org.apache.maven.shared.release.phase;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.maven.artifact.ArtifactUtils;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.scm.manager.NoSuchScmProviderException;
+import org.apache.maven.scm.provider.ScmProvider;
+import org.apache.maven.scm.repository.ScmRepository;
+import org.apache.maven.scm.repository.ScmRepositoryException;
+import org.apache.maven.shared.release.ReleaseExecutionException;
+import org.apache.maven.shared.release.ReleaseResult;
+import org.apache.maven.shared.release.config.ReleaseDescriptor;
+import org.apache.maven.shared.release.env.ReleaseEnvironment;
+import org.apache.maven.shared.release.policy.PolicyException;
+import org.apache.maven.shared.release.policy.naming.NamingPolicy;
+import org.apache.maven.shared.release.policy.naming.NamingPolicyRequest;
+import org.apache.maven.shared.release.scm.ReleaseScmRepositoryException;
+import org.apache.maven.shared.release.scm.ScmRepositoryConfigurator;
+import org.apache.maven.shared.release.util.ReleaseUtil;
+import org.codehaus.plexus.components.interactivity.Prompter;
+import org.codehaus.plexus.components.interactivity.PrompterException;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.Interpolator;
+import org.codehaus.plexus.interpolation.PrefixAwareRecursionInterceptor;
+import org.codehaus.plexus.interpolation.PrefixedPropertiesValueSource;
+import org.codehaus.plexus.interpolation.RecursionInterceptor;
+import org.codehaus.plexus.interpolation.StringSearchInterpolator;
+import org.codehaus.plexus.util.StringUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Input any variables that were not yet configured.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public abstract class AbstractInputVariablesPhase
+ extends AbstractReleasePhase
+{
+ /**
+ * Component used to prompt for input.
+ */
+ private final AtomicReference<Prompter> prompter;
Review Comment:
why `AtomicReference`?
--
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-release] cstamas commented on pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1118261446
I updated/fixed things reported so far, @slawekjaranowski did re-enable the failing UT GenerateReleasePomsPhaseTest (while he did disable several tests in it's parent class), but also there is that DefaultVersionInfoTest as well...
So, I'd like to see some consensus about these changes and finally make this PR merged.
But please note, that if there are some other pending PRs for upcoming release, better would be to merge those and update this PR, as am pretty sure this PR will render all existing PRs as "conflicted".
--
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-release] slawekjaranowski commented on a diff in pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865711326
##########
maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/AbstractReleaseTestCase.java:
##########
@@ -213,7 +213,13 @@ public FileVisitResult visitFile( Path file, BasicFileAttributes attrs )
for ( MavenProject project : reactorProjects )
{
MavenProject resolvedProject = projectBuilder.build( project.getFile(), buildingRequest ).getProject();
-
+
+ // FIXME ... setDependencyArtifacts - set direct dependencies ...
+ // but in org.apache.maven.shared.release.phase.GenerateReleasePomsPhase.createReleaseDependencies
+ // getArtifacts is used ...
+
+ // so we probably also need call setResolvedArtifacts
+
Review Comment:
And why we need use `setDependencyArtifacts` directly ...
code is also strange for me we check:
```
if ( project.getDependencyArtifacts() == null )
```
and next on different object
```
resolvedProject.setDependencyArtifacts(....)
```
I will check what happens if I call
```
resolvedProject.setResolvedArtifacts( resolvedProject.getDependencyArtifacts() )
```
of course I don't understand it very well
--
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-release] cstamas commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1114643720
I am aware this PR is really BIG and nothing else describes it fully than vague "catch up", but we need to perform this jump, as current master is really lagging and IMHO has way too much debt than would be somewhat IMHO acceptable.
--
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-release] cstamas commented on a diff in pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865630699
##########
maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/AbstractReleaseTestCase.java:
##########
@@ -213,7 +213,13 @@ public FileVisitResult visitFile( Path file, BasicFileAttributes attrs )
for ( MavenProject project : reactorProjects )
{
MavenProject resolvedProject = projectBuilder.build( project.getFile(), buildingRequest ).getProject();
-
+
+ // FIXME ... setDependencyArtifacts - set direct dependencies ...
+ // but in org.apache.maven.shared.release.phase.GenerateReleasePomsPhase.createReleaseDependencies
+ // getArtifacts is used ...
+
+ // so we probably also need call setResolvedArtifacts
+
Review Comment:
For example, I have hard time to understand why release needs to resolve anything at all...
--
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-release] cstamas commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865600675
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java:
##########
@@ -530,25 +537,25 @@ else if ( mappedVersion.equals( propertyValue ) )
{
// this property may have been updated during processing a sibling.
logInfo( result, " Ignoring artifact version update for expression " + rawVersion
- + " because it is already updated" );
+ + " because it is already updated" );
}
else if ( !mappedVersion.equals( rawVersion ) )
{
if ( mappedVersion.matches( "\\$\\{project.+\\}" )
- || mappedVersion.matches( "\\$\\{pom.+\\}" )
- || "${version}".equals( mappedVersion ) )
+ || mappedVersion.matches( "\\$\\{pom.+\\}" )
+ || "${version}".equals( mappedVersion ) )
Review Comment:
BTW, this is yet another indicator release plugin does too much: it seems to duplicate too much logic that is in core...
--
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-release] hboutemy commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1116971073
we need a MNG Jira issue with an update of every commit message to be able to track the change in the future
--
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-release] cstamas merged pull request #118: [MRELEASE-1087] Upgrade Maven to 3.2.5 (and de-plexus)
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #118:
URL: https://github.com/apache/maven-release/pull/118
--
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-release] cstamas commented on a diff in pull request #118: [MRELEASE-1087] Upgrade Maven to 3.2.5 (and de-plexus)
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r866524011
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
Review Comment:
Unified all loggers, they are obtained in same way everywhere
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractInputVariablesPhase.java:
##########
@@ -0,0 +1,309 @@
+package org.apache.maven.shared.release.phase;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.maven.artifact.ArtifactUtils;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.scm.manager.NoSuchScmProviderException;
+import org.apache.maven.scm.provider.ScmProvider;
+import org.apache.maven.scm.repository.ScmRepository;
+import org.apache.maven.scm.repository.ScmRepositoryException;
+import org.apache.maven.shared.release.ReleaseExecutionException;
+import org.apache.maven.shared.release.ReleaseResult;
+import org.apache.maven.shared.release.config.ReleaseDescriptor;
+import org.apache.maven.shared.release.env.ReleaseEnvironment;
+import org.apache.maven.shared.release.policy.PolicyException;
+import org.apache.maven.shared.release.policy.naming.NamingPolicy;
+import org.apache.maven.shared.release.policy.naming.NamingPolicyRequest;
+import org.apache.maven.shared.release.scm.ReleaseScmRepositoryException;
+import org.apache.maven.shared.release.scm.ScmRepositoryConfigurator;
+import org.apache.maven.shared.release.util.ReleaseUtil;
+import org.codehaus.plexus.components.interactivity.Prompter;
+import org.codehaus.plexus.components.interactivity.PrompterException;
+import org.codehaus.plexus.interpolation.InterpolationException;
+import org.codehaus.plexus.interpolation.Interpolator;
+import org.codehaus.plexus.interpolation.PrefixAwareRecursionInterceptor;
+import org.codehaus.plexus.interpolation.PrefixedPropertiesValueSource;
+import org.codehaus.plexus.interpolation.RecursionInterceptor;
+import org.codehaus.plexus.interpolation.StringSearchInterpolator;
+import org.codehaus.plexus.util.StringUtils;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Input any variables that were not yet configured.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public abstract class AbstractInputVariablesPhase
+ extends AbstractReleasePhase
+{
+ /**
+ * Component used to prompt for input.
+ */
+ private final AtomicReference<Prompter> prompter;
+
+ /**
+ * Tool that gets a configured SCM repository from release configuration.
+ */
+ private final ScmRepositoryConfigurator scmRepositoryConfigurator;
+
+ /**
+ * Component used for custom or default naming policy
+ */
+ private final Map<String, NamingPolicy> namingPolicies;
+
+ /**
+ * Whether this is a branch or a tag operation.
+ */
+ private final boolean branchOperation;
+
+ /**
+ * The default naming policy to apply, if any
+ */
+ private final String defaultNamingPolicy;
+
+ protected AbstractInputVariablesPhase( Prompter prompter, ScmRepositoryConfigurator scmRepositoryConfigurator,
+ Map<String, NamingPolicy> namingPolicies, boolean branchOperation,
+ String defaultNamingPolicy )
+ {
+ this.prompter = new AtomicReference<>( requireNonNull( prompter ) );
+ this.scmRepositoryConfigurator = requireNonNull( scmRepositoryConfigurator );
+ this.namingPolicies = requireNonNull( namingPolicies );
+ this.branchOperation = branchOperation;
+ this.defaultNamingPolicy = defaultNamingPolicy;
+ }
+
+ /**
+ * For easier testing only!
+ */
+ public void setPrompter( Prompter prompter )
+ {
+ this.prompter.set( prompter );
+ }
+
+ boolean isBranchOperation()
+ {
+ return branchOperation;
+ }
+
+ /**
+ * <p>getScmProvider.</p>
+ *
+ * @param releaseDescriptor a {@link ReleaseDescriptor} object
+ * @param releaseEnvironment a {@link ReleaseEnvironment} object
+ * @return a {@link ScmProvider} object
+ * @throws ReleaseScmRepositoryException if any.
+ * @throws ReleaseExecutionException if any.
+ */
+ protected ScmProvider getScmProvider( ReleaseDescriptor releaseDescriptor, ReleaseEnvironment releaseEnvironment )
+ throws ReleaseScmRepositoryException, ReleaseExecutionException
+ {
+ try
+ {
+ ScmRepository repository =
+ scmRepositoryConfigurator.getConfiguredRepository( releaseDescriptor,
+ releaseEnvironment.getSettings() );
+
+ return scmRepositoryConfigurator.getRepositoryProvider( repository );
+ }
+ catch ( ScmRepositoryException e )
+ {
+ throw new ReleaseScmRepositoryException(
+ e.getMessage() + " for URL: " + releaseDescriptor.getScmSourceUrl(), e.getValidationMessages() );
+ }
+ catch ( NoSuchScmProviderException e )
+ {
+ throw new ReleaseExecutionException( "Unable to configure SCM repository: " + e.getMessage(), e );
+ }
+ }
+
+ @Override
+ public ReleaseResult execute( ReleaseDescriptor releaseDescriptor, ReleaseEnvironment releaseEnvironment,
+ List<MavenProject> reactorProjects )
+ throws ReleaseExecutionException
+ {
+ ReleaseResult result = new ReleaseResult();
+
+ // get the root project
+ MavenProject project = ReleaseUtil.getRootProject( reactorProjects );
+
+ String tag = releaseDescriptor.getScmReleaseLabel();
+
+ if ( tag == null )
+ {
+ // Must get default version from mapped versions, as the project will be the incorrect snapshot
+ String key = ArtifactUtils.versionlessKey( project.getGroupId(), project.getArtifactId() );
+ String releaseVersion = releaseDescriptor.getProjectReleaseVersion( key );
+ if ( releaseVersion == null )
+ {
+ throw new ReleaseExecutionException( "Project tag cannot be selected if version is not yet mapped" );
+ }
+
+ String suggestedName;
+ String scmTagNameFormat = releaseDescriptor.getScmTagNameFormat();
+ if ( releaseDescriptor.getProjectNamingPolicyId() != null )
+ {
+ try
+ {
+ suggestedName =
+ resolveSuggestedName( releaseDescriptor.getProjectNamingPolicyId(), releaseVersion,
+ project );
+ }
+ catch ( PolicyException e )
+ {
+ throw new ReleaseExecutionException( e.getMessage(), e );
+ }
+ }
+ else if ( scmTagNameFormat != null )
+ {
+ Interpolator interpolator = new StringSearchInterpolator( "@{", "}" );
+ List<String> possiblePrefixes = java.util.Arrays.asList( "project", "pom" );
Review Comment:
Fixed
--
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-release] cstamas commented on pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1118957409
Cool, thanks @slawekjaranowski !
--
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-release] michael-o commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r863541722
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
+
+ private final Map<String, Strategy> strategies;
/**
* The available phases.
*/
- @Requirement
- private Map<String, ReleasePhase> releasePhases;
+ private final Map<String, ReleasePhase> releasePhases;
/**
* The configuration storage.
*/
- @Requirement( hint = "properties" )
- private ReleaseDescriptorStore configStore;
+ private final AtomicReference<ReleaseDescriptorStore> configStore;
private static final int PHASE_SKIP = 0, PHASE_START = 1, PHASE_END = 2, GOAL_END = 12, ERROR = 99;
+ @Inject
+ public DefaultReleaseManager( Map<String, Strategy> strategies,
+ Map<String, ReleasePhase> releasePhases,
+ @Named( "properties" ) ReleaseDescriptorStore configStore )
+ {
+ this.strategies = requireNonNull( strategies );
+ this.releasePhases = requireNonNull( releasePhases );
+ this.configStore = new AtomicReference<>( requireNonNull( configStore ) );
Review Comment:
Thanks...
--
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-release] cstamas commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865600455
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java:
##########
@@ -530,25 +537,25 @@ else if ( mappedVersion.equals( propertyValue ) )
{
// this property may have been updated during processing a sibling.
logInfo( result, " Ignoring artifact version update for expression " + rawVersion
- + " because it is already updated" );
+ + " because it is already updated" );
}
else if ( !mappedVersion.equals( rawVersion ) )
{
if ( mappedVersion.matches( "\\$\\{project.+\\}" )
- || mappedVersion.matches( "\\$\\{pom.+\\}" )
- || "${version}".equals( mappedVersion ) )
+ || mappedVersion.matches( "\\$\\{pom.+\\}" )
+ || "${version}".equals( mappedVersion ) )
Review Comment:
Added
--
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-release] cstamas commented on a diff in pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865623124
##########
maven-release-manager/pom.xml:
##########
@@ -98,14 +116,25 @@
<dependency>
<groupId>commons-cli</groupId>
<artifactId>commons-cli</artifactId>
- <version>1.2</version>
+ <version>1.5.0</version>
Review Comment:
See 820e774bbe892d2f1f0d77b8355216ad87d264ec
--
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-release] slawekjaranowski commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865414828
##########
maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/AbstractReleaseTestCase.java:
##########
@@ -213,7 +213,13 @@ public FileVisitResult visitFile( Path file, BasicFileAttributes attrs )
for ( MavenProject project : reactorProjects )
{
MavenProject resolvedProject = projectBuilder.build( project.getFile(), buildingRequest ).getProject();
-
+
+ // FIXME ... setDependencyArtifacts - set direct dependencies ...
+ // but in org.apache.maven.shared.release.phase.GenerateReleasePomsPhase.createReleaseDependencies
+ // getArtifacts is used ...
+
+ // so we probably also need call setResolvedArtifacts
+
Review Comment:
Probable cause of ignore tests in AbstractRewritingReleasePhaseTestCase
--
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-release] cstamas commented on a diff in pull request #118: [MRELEASE-1087] Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r865604908
##########
maven-release-manager/pom.xml:
##########
@@ -98,14 +116,25 @@
<dependency>
<groupId>commons-cli</groupId>
<artifactId>commons-cli</artifactId>
- <version>1.2</version>
+ <version>1.5.0</version>
Review Comment:
Yup, look into org.apache.maven.shared.release.exec.InvokerMavenExecutor
--
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-release] hboutemy commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1118218500
Jira issue created https://issues.apache.org/jira/browse/MRELEASE-1087
--
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-release] michael-o commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r863513916
##########
maven-release-manager/pom.xml:
##########
@@ -59,14 +60,14 @@
</dependency>
<dependency>
- <groupId>org.sonatype.plexus</groupId>
+ <groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-sec-dispatcher</artifactId>
- <version>1.3</version>
+ <version>2.0</version>
</dependency>
<dependency>
- <groupId>org.sonatype.plexus</groupId>
+ <groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-cipher</artifactId>
- <version>1.7</version>
+ <version>2.0</version>
Review Comment:
Perfect thank you!
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
+
+ private final Map<String, Strategy> strategies;
/**
* The available phases.
*/
- @Requirement
- private Map<String, ReleasePhase> releasePhases;
+ private final Map<String, ReleasePhase> releasePhases;
/**
* The configuration storage.
*/
- @Requirement( hint = "properties" )
- private ReleaseDescriptorStore configStore;
+ private final AtomicReference<ReleaseDescriptorStore> configStore;
private static final int PHASE_SKIP = 0, PHASE_START = 1, PHASE_END = 2, GOAL_END = 12, ERROR = 99;
+ @Inject
+ public DefaultReleaseManager( Map<String, Strategy> strategies,
+ Map<String, ReleasePhase> releasePhases,
+ @Named( "properties" ) ReleaseDescriptorStore configStore )
+ {
+ this.strategies = requireNonNull( strategies );
+ this.releasePhases = requireNonNull( releasePhases );
+ this.configStore = new AtomicReference<>( requireNonNull( configStore ) );
Review Comment:
Can you explain why this requires an atomic ref?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/PropertiesReleaseDescriptorStore.java:
##########
@@ -101,21 +103,21 @@ public ReleaseDescriptorBuilder read( ReleaseDescriptorBuilder mergeDescriptor,
}
catch ( FileNotFoundException e )
{
- getLogger().debug( file.getName() + " not found - using empty properties" );
+ LOGGER.debug( file.getName() + " not found - using empty properties" );
}
catch ( IOException e )
{
throw new ReleaseDescriptorStoreException(
- "Error reading properties file '" + file.getName() + "': " + e.getMessage(), e );
+ "Error reading properties file '" + file.getName() + "': " + e.getMessage(), e );
Review Comment:
I hate this ugly duplication of exception messages, but this is not part of this PR.
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/PropertiesReleaseDescriptorStore.java:
##########
@@ -101,21 +103,21 @@ public ReleaseDescriptorBuilder read( ReleaseDescriptorBuilder mergeDescriptor,
}
catch ( FileNotFoundException e )
{
- getLogger().debug( file.getName() + " not found - using empty properties" );
+ LOGGER.debug( file.getName() + " not found - using empty properties" );
}
catch ( IOException e )
{
throw new ReleaseDescriptorStoreException(
- "Error reading properties file '" + file.getName() + "': " + e.getMessage(), e );
+ "Error reading properties file '" + file.getName() + "': " + e.getMessage(), e );
}
-
+
try
{
- decryptProperties( properties );
+ mavenCrypto.decryptProperties( properties );
}
- catch ( IllegalStateException | SecDispatcherException | PlexusCipherException e )
+ catch ( MavenCryptoException e )
{
- getLogger().debug( e.getMessage() );
+ LOGGER.debug( e.getMessage() );
Review Comment:
Ha ha ha, crypto fails, but hey let's continue anyway.
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java:
##########
@@ -530,25 +537,25 @@ else if ( mappedVersion.equals( propertyValue ) )
{
// this property may have been updated during processing a sibling.
logInfo( result, " Ignoring artifact version update for expression " + rawVersion
- + " because it is already updated" );
+ + " because it is already updated" );
}
else if ( !mappedVersion.equals( rawVersion ) )
{
if ( mappedVersion.matches( "\\$\\{project.+\\}" )
- || mappedVersion.matches( "\\$\\{pom.+\\}" )
- || "${version}".equals( mappedVersion ) )
+ || mappedVersion.matches( "\\$\\{pom.+\\}" )
+ || "${version}".equals( mappedVersion ) )
Review Comment:
Can you add a comment here that `pom.` and prefixless is gone in Maven 4?
##########
maven-release-manager/pom.xml:
##########
@@ -98,14 +116,25 @@
<dependency>
<groupId>commons-cli</groupId>
<artifactId>commons-cli</artifactId>
- <version>1.2</version>
+ <version>1.5.0</version>
Review Comment:
I don't know how much is needed here, but @mthmulders put some effect to change Maven Code code to 1.5.0 for those APIs which are deprecated. Does this apply here as well?
--
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-release] michael-o commented on pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #118:
URL: https://github.com/apache/maven-release/pull/118#issuecomment-1115788966
Will try to go through today/tomorrow.
--
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-release] cstamas commented on a diff in pull request #118: Catch up
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #118:
URL: https://github.com/apache/maven-release/pull/118#discussion_r863535359
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java:
##########
@@ -37,38 +42,56 @@
import org.apache.maven.shared.release.phase.ReleasePhase;
import org.apache.maven.shared.release.phase.ResourceGenerator;
import org.apache.maven.shared.release.strategy.Strategy;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
-import org.codehaus.plexus.logging.AbstractLogEnabled;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
/**
* Implementation of the release manager.
*
* @author <a href="mailto:brett@apache.org">Brett Porter</a>
*/
-@Component( role = ReleaseManager.class )
+@Singleton
+@Named
public class DefaultReleaseManager
- extends AbstractLogEnabled
implements ReleaseManager
{
- @Requirement
- private Map<String, Strategy> strategies;
+ private static final Logger LOGGER = LoggerFactory.getLogger( DefaultReleaseManager.class );
+
+ private final Map<String, Strategy> strategies;
/**
* The available phases.
*/
- @Requirement
- private Map<String, ReleasePhase> releasePhases;
+ private final Map<String, ReleasePhase> releasePhases;
/**
* The configuration storage.
*/
- @Requirement( hint = "properties" )
- private ReleaseDescriptorStore configStore;
+ private final AtomicReference<ReleaseDescriptorStore> configStore;
private static final int PHASE_SKIP = 0, PHASE_START = 1, PHASE_END = 2, GOAL_END = 12, ERROR = 99;
+ @Inject
+ public DefaultReleaseManager( Map<String, Strategy> strategies,
+ Map<String, ReleasePhase> releasePhases,
+ @Named( "properties" ) ReleaseDescriptorStore configStore )
+ {
+ this.strategies = requireNonNull( strategies );
+ this.releasePhases = requireNonNull( releasePhases );
+ this.configStore = new AtomicReference<>( requireNonNull( configStore ) );
Review Comment:
A few lines lower there is a **setter** for config store, that is used in UT ONLY. Simply put: for sanity sake, to keep ctor injection and keep all member final, BUT to not completely rewrite tests, I did it like this. The setter has comment on it.
This pattern is applied on several places, as sadly UTs were written with "plexus on mind", so they lookup component from container and then change members to some mocks... changed to pure ctor injection where I could, but there are some complicated UTs that I just gave up, and added setter + atomic ref to save my sanity :smile:
All this should not bother anything at "runtime" (in prod), when plugin runs in build.
--
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