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

[MNG-7774] Maven config and command line interpolation #1098

Merged
merged 10 commits into from
May 5, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented May 2, 2023

Reuse as much as possible from master, but keep existing stuff like multiModuleProjectDirectory alone.

Changes:

  • interpolate user properties and arguments
  • introduce session.topDirectory and session.rootDirectory expressions (for interpolation only)
  • Maven fails to start if any of the new properties are undefined but their use is attempted
  • leave everything else untouched

https://issues.apache.org/jira/browse/MNG-7774

Reuse as much as possible from master, but keep existing stuff like
multiModuleProjectDirectory alone.

Changes:
* interpolate user properties and arguments
* introduce session.topDirectory and session.rootDirectory
* Maven fails to start if any of the new properties are undefined but their use is attempted
@cstamas cstamas requested a review from michael-o May 3, 2023 07:36
try {
topDirectory = topDirectory.toAbsolutePath().toRealPath();
} catch (IOException e) {
System.err.println("Error computing real path from " + topDirectory);
Copy link
Member

Choose a reason for hiding this comment

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

Would it be easier to troubleshoot the problem if the exception information was printed out here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, included IOEx message as well

@cstamas cstamas changed the title [MNG-7774] Maven config and command line interpolation (take two) [MNG-7774] Maven config and command line interpolation May 4, 2023
@michael-o
Copy link
Member

Does this actually supersede #1062?

@cstamas
Copy link
Member Author

cstamas commented May 4, 2023

Does this actually supersede #1062?

No, that one is for master and builds on top of already present new mvn4 features not present in 3.9.x. This PR is for 3.9.x and "mildly follows" the one you refer to, with reduced functionalities. For example, that one uses RootLocator (already on master) while we have no such thing. Master also uses two strategies to identify "root", but it requires consumer POM feature, again not available here, etc. This PR provides "minimum viable change" to provide in 3.9.x:

  • config and command line interpolation
  • two new properties (but for interpolation ONLY), unlike on master, where these two bubble up to session and project
  • the UT is intentionally 1:1 (so it IS copy pasted from referenced PR)

@cstamas
Copy link
Member Author

cstamas commented May 4, 2023

Anyone else?

@CrazyHZM
Copy link
Member

CrazyHZM commented May 4, 2023

@cstamas I think the description of adding this command in mvn -help is still missing.

@cstamas
Copy link
Member Author

cstamas commented May 4, 2023

@cstamas I think the description of adding this command in mvn -help is still missing.

Which command? No command was added or modified in Maven CLI

@CrazyHZM
Copy link
Member

CrazyHZM commented May 4, 2023

@cstamas I think the description of adding this command in mvn -help is still missing.

Which command? No command was added or modified in Maven CLI

My mistake was that I didn't see it clearly. There was nothing wrong with 3.9.x. There are too many maven versions for my local development. 😢 @cstamas

Copy link
Member

@CrazyHZM CrazyHZM left a comment

Choose a reason for hiding this comment

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

LGTM

I had a change at one point I forgot about (Hamcrest matcher "is" vs "endsWith").
Implementation now behaves same as on master, so proper is "is".
@cstamas cstamas merged commit 79556dd into apache:maven-3.9.x May 5, 2023
@cstamas cstamas deleted the maven-3.9.x-MNG-7774-take-two branch May 5, 2023 12:01
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.

3 participants