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.

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

Refresh component for January 2023 #37

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Conversation

basil
Copy link
Member

@basil basil commented Feb 16, 2023

See jenkinsci/pom#387 and jenkinsci/plugin-pom#689. We cannot update Maven Enforcer Plugin to the latest version until this component is recompiled against recent Maven APIs.

I tested each component manually (with Enforcer 3.2.1):

  • RequireExtensionVersion with valid version
  • RequireExtensionVersion with invalid version
  • Unincrementalified a plugin, stepped through the modified code in IncrementalifyMojo in a debugger, verified the changes to pom.xml were as expected
  • Stepped through the modified code in UpdateMojo in a debugger, verified the mojo completed without any errors
  • Jenkins core build with these changes
  • Text Finder plugin build with these changes

CC @jglick

@basil basil requested a review from a team as a code owner February 16, 2023 19:20
Comment on lines +18 to +20
<artifactId>enforcer-api</artifactId>
<version>3.2.1</version>
<scope>provided</scope>
Copy link
Member Author

Choose a reason for hiding this comment

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

Depending on the public API as recommended upstream rather than on internal implementation details.


/**
* Verifies that {@code git-changelist-maven-extension}, if present, is sufficiently new.
*/
public class RequireExtensionVersion extends AbstractVersionEnforcer {
@Named("requireExtensionVersion")
Copy link
Member Author

Choose a reason for hiding this comment

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

As recommended upstream. This means we'll need to change <rule implementation="io.jenkins.tools.incrementals.enforcer.RequireExtensionVersion"> to <requireExtensionVersion> in consumers, which looks nicer anyway.


/**
* Verifies that {@code git-changelist-maven-extension}, if present, is sufficiently new.
*/
public class RequireExtensionVersion extends AbstractVersionEnforcer {
@Named("requireExtensionVersion")
public class RequireExtensionVersion extends AbstractEnforcerRule {
Copy link
Member Author

Choose a reason for hiding this comment

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

AbstractVersionEnforcer is now a private API, so depend only on the public API and copy/paste in everything else we need from the API that is now private.

Comment on lines +53 to +66
/**
* Specify the required version. Some examples are:
* <ul>
* <li><code>2.0.4</code> Version 2.0.4 and higher (different from Maven meaning)</li>
* <li><code>[2.0,2.1)</code> Versions 2.0 (included) to 2.1 (not included)</li>
* <li><code>[2.0,2.1]</code> Versions 2.0 to 2.1 (both included)</li>
* <li><code>[2.0.5,)</code> Versions 2.0.5 and higher</li>
* <li><code>(,2.0.5],[2.1.1,)</code> Versions up to 2.0.5 (included) and 2.1.1 or higher</li>
* </ul>
*
* @see {@link #setVersion(String)}
* @see {@link #getVersion()}
*/
private String version;
Copy link
Member Author

Choose a reason for hiding this comment

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

Copypasta.

Comment on lines +68 to +73
private final PlexusContainer container;

@Inject
public RequireExtensionVersion(PlexusContainer container) {
this.container = Objects.requireNonNull(container);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Adapting to the new recommended way of injecting components via constructor (EnforcerRuleHelper is now deprecated).

Comment on lines +79 to +81
@Inject public IncrementalifyMojo(RepositorySystem repositorySystem, org.eclipse.aether.RepositorySystem aetherRepositorySystem, Map<String, Wagon> wagonMap, Map<String, ChangeRecorder> changeRecorders) {
super(repositorySystem, aetherRepositorySystem, wagonMap, changeRecorders);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Adapting to the new recommended method for injecting components via constructor.

Comment on lines +90 to +98
try {
gclmeVersions = getHelper().lookupArtifactVersions(getHelper().createDependencyArtifact("io.jenkins.tools.incrementals", "git-changelist-maven-extension", "[0,)", "type", null, null), true);
} catch (VersionRetrievalException x) {
throw new MojoExecutionException(x.getMessage(), x);
}
VersionRange any;
try {
any = VersionRange.createFromVersionSpec("[0,)");
gclmeVersions = getHelper().lookupArtifactVersions(getHelper().createDependencyArtifact("io.jenkins.tools.incrementals", "git-changelist-maven-extension", any, "type", null, null), true);
} catch (ArtifactMetadataRetrievalException | InvalidVersionSpecificationException x) {
} catch (InvalidVersionSpecificationException x) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Adapting to random compilation failures. Stepped through the new code in the debugger to make sure it was working.

Comment on lines +163 to +166
PomHelper.setElementValue(pom, "/project/scm", "tag", "${scmTag}");
PomHelper.setElementValue(pom, "/project/scm", "connection", connectionRGHR.interpolableText);
PomHelper.setElementValue(pom, "/project/scm", "developerConnection", developerConnectionRGHR.interpolableText);
PomHelper.setElementValue(pom, "/project/scm", "url", urlRGHR.interpolableText);
Copy link
Member Author

Choose a reason for hiding this comment

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

Adapting to random API breakage as was done upstream by using the new method.

Comment on lines +169 to +178
/**
* Gets the parent artifact from the pom.
*
* @param pom The pom.
* @param helper The helper (used to create the artifact).
* @return The parent artifact or <code>null</code> if no parent is specified.
* @throws XMLStreamException if something went wrong.
*/
private static Artifact getProjectParent( final ModifiedPomXMLEventReader pom, VersionsHelper helper )
throws XMLStreamException
Copy link
Member Author

Choose a reason for hiding this comment

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

Copypasta, this was deleted upstream with no explanation. Inlining the last working version of it before it was deleted.

@@ -133,7 +141,7 @@ private void update(ModifiedPomXMLEventReader pom, List<Dependency> dependencies
}

private void updateProperties(ModifiedPomXMLEventReader pom, UpdateChecker checker) throws Exception {
PROPERTY: for (Map.Entry<Property, PropertyVersions> entry : getHelper().getVersionPropertiesMap(getProject(), /* TODO */ new Property[0], /* TODO */ null, null, true).entrySet()) {
PROPERTY: for (Map.Entry<Property, PropertyVersions> entry : getHelper().getVersionPropertiesMap(VersionsHelper.VersionPropertiesMapRequest.builder().withMavenProject(getProject()).build()).entrySet()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Adapting to random upstream breakage as was done in the original commit. Stepped through this in the debugger to make sure it worked.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Thanks! Will run a release.

@jglick jglick added the dependencies Pull requests that update a dependency file label Feb 16, 2023
@jglick jglick merged commit 0980fbf into jenkinsci:master Feb 16, 2023
@jglick
Copy link
Member

jglick commented Feb 16, 2023

@basil basil deleted the refresh branch February 16, 2023 20:09
basil added a commit to jenkinsci/pom that referenced this pull request Feb 16, 2023
basil added a commit to jenkinsci/plugin-pom that referenced this pull request Feb 16, 2023
basil added a commit to jenkinsci/plugin-pom that referenced this pull request Feb 16, 2023
basil added a commit to jenkinsci/plugin-pom that referenced this pull request Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants