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

Jib Maven Plugin 3.4.4 doesn't consider user properties properly (breaks on Maven 4) #4344

Open
breun opened this issue Jan 7, 2025 · 2 comments · May be fixed by #4345
Open

Jib Maven Plugin 3.4.4 doesn't consider user properties properly (breaks on Maven 4) #4344

breun opened this issue Jan 7, 2025 · 2 comments · May be fixed by #4345

Comments

@breun
Copy link

breun commented Jan 7, 2025

Environment:

  • Jib version: 3.4.4
  • Build tool: Maven 4.0.0-rc-2
  • OS: macOS 15.2 arm64

Description of the issue:

I have a working project with jib-maven-plugin 3.4.4 using Maven 3.9.9, but when I tried it with Maven 4.0.0-rc-2, I noticed that specifying -Djib.to.image=foo:bar on the Maven command line was no longer being picked up.

At first I thought this was a bug in Maven 4.0.0-rc-2, so I created a bug report for Maven, but after a Maven developer looked into this, he concluded that this is actually an issue with the way that jib-maven-plugin reads properties. See this comment for the full analysis, but the short summary is that jib-maven-plugin currently doesn't consider user properties (as specified via -Dfoo=bar on the Maven command line) correctly, and this will actually break with Maven 4.

But it looks like there is a simple fix for this, by checking session.getUserProperties() with the highest precendence before project and system properties.

Expected behavior:

I expected specifying -Djib.to.image=foo:bar to keep working on Maven 4.0.0-rc-2, and according to the Maven developers that should be possible if jib-maven-plugin makes the suggested change. That more robust method of reading configuration properties should also work with Maven 3, so this should be a backwards compatible change.

Steps to reproduce:

See my reproducer project with instructions at https://github.com/breun/reproducer-for-MNG-8486

@cstamas
Copy link

cstamas commented Jan 8, 2025

Proposed change (just use MavenSession and MavenProject):

 public static String getProperty( 
     String propertyName, @Nullable MavenProject project, @Nullable MavenSession session) { 
   if (session != null && session.getUserProperties().containsKey(propertyName)) { 
     return session.getUserProperties().getProperty(propertyName); 
   } 
   if (project != null && project.getProperties().containsKey(propertyName)) { 
     return project.getProperties().getProperty(propertyName); 
   } 
   if (session != null && session.getSystemProperties().containsKey(propertyName)) { 
     return session.getSystemProperties().getProperty(propertyName); 
   } 
   return null; 
 } 

Method would be most correct if changed to this:

  • maven user properties "wins" always
  • then project propeties are checked
  • finally maven system properties are checked

Current code problem is that is NOT checking user properties, while Maven 4 now cleanly separates them.

@breun
Copy link
Author

breun commented Jan 8, 2025

I believe this PR fixes the issue: #4345

@breun breun changed the title Jib Maven Plugin 3.4.4 doesn't consider user properties properly Jib Maven Plugin 3.4.4 doesn't consider user properties properly (breaks on Maven 4) Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants