Skip to content

[MNG-8694] Fix version interpolation and ternary operator #2272

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 30, 2025
Merged

[MNG-8694] Fix version interpolation and ternary operator #2272

merged 7 commits into from
Apr 30, 2025

Conversation

kamilkrzywanski
Copy link
Contributor

@kamilkrzywanski kamilkrzywanski commented Apr 23, 2025 •
edited
Loading

Fix for https://issues.apache.org/jira/browse/MNG-8694
@slawekjaranowski as mentioned today on jug challenge accepted!

gnodet
gnodet requested changes Apr 24, 2025
View reviewed changes
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

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

I don't think the behavior is complete.
It could be solved by a loop, but preferably by using the DefaultInterpolator service.

@gnodet gnodet added this to the 4.0.0-rc-4 milestone Apr 24, 2025
@kamilkrzywanski
Copy link
Contributor Author

@gnodet Removed unused regex as well, can you approve again?

@gnodet gnodet changed the title MNG-8694 - fix for resolve version from nested props [MNG-8694] fix for resolve version from nested props Apr 24, 2025
@gnodet
Copy link
Contributor

gnodet commented Apr 24, 2025

I still think this only partially address the use case from MNG-8694, so IMHO we'd need to extend the test to support a similar use case. Probably not using a profile, since it won't work, but using a value such as ${coreVersion}${release:--DEVELOPER} or something like that.

@kamilkrzywanski
Copy link
Contributor Author

kamilkrzywanski commented Apr 24, 2025 •
edited
Loading

@gnodet Test case extended

@gnodet gnodet changed the title [MNG-8694] fix for resolve version from nested props [MNG-8694] Fix version interpolation and ternary operator Apr 25, 2025
@gnodet
Copy link
Contributor

gnodet commented Apr 25, 2025 •
edited
Loading

@gnodet Test case extended

Thx, I slightly modified it as one of the problem was the generation of an empty string when release is defined to true (which is what happens when using -Drelease on the command line for example).

This PR now fixes the ternary operator to support expressions such as ${release:+${foo}:-${bar}} or ${versionCore}${release:+-DEVELOPER}.

The actual solution for the original use case is to use ${versionCore}${release:+:--DEVELOPER} which will evaluate to 1.0 if release is defined, and 1.0-DEVELOPER if undefined.

@gnodet
Fix ternary operator processing in DefaultInterpolator
a8c77ed 
When processing ternary expressions like ${release:+${foo}:-${bar}} or ${versionCore}${release:+-DEVELOPER}, the DefaultInterpolator was incorrectly continuing to process subsequent operators after a decision had been made on the first operator.

This fix adds a break statement after each successful operator evaluation to stop processing any remaining operators once a decision has been made, ensuring that:

1. When 'release' is true and 'foo' is empty,  correctly evaluates to an empty string
2. When 'release' is true,  correctly evaluates to just

Added test cases to verify both scenarios.
@gnodet gnodet merged commit fffdf0e into apache:master Apr 30, 2025
13 checks passed
@jira-importer
Copy link

Resolve #9611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnodet gnodet gnodet approved these changes

@slawekjaranowski slawekjaranowski slawekjaranowski approved these changes

Assignees

@gnodet gnodet

Labels
maintenance
Projects
None yet
Milestone
4.0.0-rc-4
Development

Successfully merging this pull request may close these issues.
4 participants
@kamilkrzywanski @gnodet @jira-importer @slawekjaranowski