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

[breun]

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]

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]

I believe this PR fixes the issue: #4345

[breun]

@diegomarquezp Is there anything I can do at this point to help get this merged? I'd consider this a bug fix rather than a feature request, because as soon as Maven 4 gets released for instance the method of specifying jib.to.image from the Jib docs will no longer work.

[diegomarquezp]

@ldetmer @mpeddada1 PTAL

[breun]

@diegomarquezp @ldetmer @mpeddada1 Maven 4.0.0-rc3 has been tagged, so this fix is getting more and more urgent. When Maven 4 is released before this PR gets merged, Jib users will start running into issues with the documented way of using Jib when they upgrade to Maven 4. Let me know if you need anything from me.

[ldetmer]

@breun thank you for the PR! technically this could be a breaking change since we are changing the order in which we retrieve properties. Can you provide a link to why maven 4 will cause an issue retrieving jib.to.image? Can it no longer be stored in the system properties?

[breun]

@ldetmer I believe the explanation is in MNG-8486 and its comments.

Jib's documentation says that you can specify a system property like jib.to.image on the Maven command line via -Djib.to.image=foo:bar, but -D is actually for setting a user property, not a system property. The reason that this works with Maven 3 is because Maven 3 copies user properties into the set of system properties, so when Jib retrieves the system properties, it will encounter the user property that was set. Maven 4 is stopping the (admittedly slightly strange) practice of including user properties in the set of system properties, and treating like the separate sets of properties that they are intended to be, and this means that Jib should also consult both sets of properties with the correct precedence.

I'd say the PR doesn't really change the order in which Jib retrieves properties, but it now correctly checks user properties first and only afterwards falls back to system properties. This should work on both Maven 3 and 4.

If you'd still consider merging this into the current Jib too risky for some reason, then only the next major version of Jib can be made compatible with Maven 4. But I'd consider any change in behavior a bug fix to how it was always intended to be, and I can't really think of a realistic example that would break, but it's always possible that I'm missing an angle.