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

Update internal maven version #182

Merged
merged 1 commit into from
Jun 16, 2021
Merged

Conversation

alecharp
Copy link
Member

@alecharp alecharp commented Jun 4, 2021

This PR is to update the internal version of Maven used by the plugin.
This is to be able to use the latest patches of the tool.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

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

given how this plugin interacts with the underlying maven and that generally libraries are backward but not forward compatible what testing has been performed here?

Has this been manually tested that projects with older versions of Maven still work?

The ATH only tests with maven 3.6.3 which is possibly a bit too new given this plugin aims to support some ancient versions of maven...

Other than that code looks perfectly sane

@alecharp
Copy link
Member Author

alecharp commented Jun 7, 2021

given how this plugin interacts with the underlying maven and that generally libraries are backward but not forward compatible what testing has been performed here?

I haven't seen any removal from API nor binary incompatible changes. But I haven't used the plugin "locally" to see if a project once built with the previous version could still be loaded and build with this new version.

Has this been manually tested that projects with older versions of Maven still work?

Not sure to understand correctly. Those changes don't change the fact that users can use whatever version of Maven they want. Like they could use Maven 3.0.5 even though the pom.xml pointed to 3.5.4.

@olamy
Copy link
Member

olamy commented Jun 8, 2021

my concern is the usage of http repo in pom (can definitely happen in internal company builds when repository are hosted internally). Does it still work especially with the embedder? (i.e first phase of the build when building graph and dependencies modules)

@alecharp
Copy link
Member Author

alecharp commented Jun 9, 2021

You are correct @olamy, http repositories are not working.

I tried to build https://github.com/jenkinsci/google-container-registry-auth-plugin, making sure to only and directly use the repository declared in the pom.xml and I got

When using Maven 3.8.1 to build the project

Parsing POMs
Failed to transfer Could not find artifact org.jenkins-ci.plugins:plugin:pom:1.596.2 in central (https://repo.maven.apache.org/maven2)
ERROR: Failed to parse POMs
org.apache.maven.project.ProjectBuildingException: Some problems were encountered while processing the POMs:
[FATAL] Non-resolvable parent POM for org.jenkins-ci.plugins:google-container-registry-auth:0.4-SNAPSHOT: Could not transfer artifact org.jenkins-ci.plugins:plugin:pom:1.596.2 from/to maven-default-http-blocker (http://0.0.0.0/): Blocked mirror for repositories: [repo.jenkins-ci.org (http://repo.jenkins-ci.org/public/, default, releases+snapshots)] and 'parent.relativePath' points at no local POM @ line 18, column 11

	at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:397)
	at hudson.maven.MavenEmbedder.buildProjects(MavenEmbedder.java:370)
	at hudson.maven.MavenEmbedder.readProjects(MavenEmbedder.java:340)
	at hudson.maven.MavenModuleSetBuild$PomParser.invoke(MavenModuleSetBuild.java:1330)
	at hudson.maven.MavenModuleSetBuild$PomParser.invoke(MavenModuleSetBuild.java:1124)
	at hudson.FilePath.act(FilePath.java:1076)
	at hudson.FilePath.act(FilePath.java:1059)
	at hudson.maven.MavenModuleSetBuild$MavenModuleSetBuildExecution.parsePoms(MavenModuleSetBuild.java:985)
	at hudson.maven.MavenModuleSetBuild$MavenModuleSetBuildExecution.doRun(MavenModuleSetBuild.java:689)
	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:513)
	at hudson.model.Run.execute(Run.java:1907)
	at hudson.maven.MavenModuleSetBuild.run(MavenModuleSetBuild.java:543)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:429)

When using Maven 3.5.4 to build the project

ERROR: Failed to parse POMs
org.apache.maven.project.ProjectBuildingException: Some problems were encountered while processing the POMs:
[FATAL] Non-resolvable parent POM for org.jenkins-ci.plugins:google-container-registry-auth:0.4-SNAPSHOT: org.jenkins-ci.plugins:plugin:pom:1.596.2 failed to transfer from http://0.0.0.0/ during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of maven-default-http-blocker has elapsed or updates are forced. Original error: Could not transfer artifact org.jenkins-ci.plugins:plugin:pom:1.596.2 from/to maven-default-http-blocker (http://0.0.0.0/): Blocked mirror for repositories: [repo.jenkins-ci.org (http://repo.jenkins-ci.org/public/, default, releases+snapshots)] and 'parent.relativePath' points at no local POM @ line 18, column 11

	at org.apache.maven.project.DefaultProjectBuilder.build(DefaultProjectBuilder.java:397)
	at hudson.maven.MavenEmbedder.buildProjects(MavenEmbedder.java:370)
	at hudson.maven.MavenEmbedder.readProjects(MavenEmbedder.java:340)
	at hudson.maven.MavenModuleSetBuild$PomParser.invoke(MavenModuleSetBuild.java:1330)
	at hudson.maven.MavenModuleSetBuild$PomParser.invoke(MavenModuleSetBuild.java:1124)
	at hudson.FilePath.act(FilePath.java:1076)
	at hudson.FilePath.act(FilePath.java:1059)
	at hudson.maven.MavenModuleSetBuild$MavenModuleSetBuildExecution.parsePoms(MavenModuleSetBuild.java:985)
	at hudson.maven.MavenModuleSetBuild$MavenModuleSetBuildExecution.doRun(MavenModuleSetBuild.java:689)
	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:513)
	at hudson.model.Run.execute(Run.java:1907)
	at hudson.maven.MavenModuleSetBuild.run(MavenModuleSetBuild.java:543)
	at hudson.model.ResourceController.execute(ResourceController.java:97)
	at hudson.model.Executor.run(Executor.java:429)

@alecharp
Copy link
Member Author

The problem is in fact less problematic. I was using the same settings.xml in both scenario: the one from Maven 3.8.1 distribution. This meant that I was in fact using a configuration which blocked the http requests.

As soon as I used the settings.xml of the Maven 3.5.4 distribution, which doesn't block http requests, I had no problem building the project.

Side note: even if I'm using a http URI for the repository, all the repositories I found are redirecting the traffic to the https and so, first download is requested through http but then it's downloaded though https and next downloads are all done through https.

@MRamonLeon
Copy link

There are a bunch of spotbugs, a test error and a Jacoco report generation failure. The two latter seem fishy, maybe it could be affected by the maven change?

@rsandell
Copy link
Member

@olamy are we good?

Perhaps we (sh/c)ould ping some other maintainers as well
@jglick @olivergondza @aheritier

@aheritier
Copy link
Member

it looks good to me even if I am always afraid to change anything in this plugin :-)
What I don't get is the new blocked attribute in the publisher and the relation with the upgrade

Comment on lines +210 to +213
<exclusion>
<groupId>org.eclipse.sisu</groupId>
<artifactId>org.eclipse.sisu.plexus</artifactId>
</exclusion>
Copy link
Member

Choose a reason for hiding this comment

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

This sort of POM hacking is a good sign that you should be writing a RealJenkinsRule test to prove that the basic functionality of the plugin still works.

pom.xml Show resolved Hide resolved
@olamy
Copy link
Member

olamy commented Jun 16, 2021

it looks good to me even if I am always afraid to change anything in this plugin :-)
What I don't get is the new blocked attribute in the publisher and the relation with the upgrade

because of this https://github.com/apache/maven/blob/d6c9614fa922027be8453c507c948f3727a91327/maven-artifact/src/main/java/org/apache/maven/artifact/repository/ArtifactRepository.java#L76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants