Skip to content

MNG-5889 - adding logic that looks for the file argument and starts t…#93

Closed
rpatrick00 wants to merge 2 commits intoapache:masterfrom
rpatrick00:master
Closed

MNG-5889 - adding logic that looks for the file argument and starts t…#93
rpatrick00 wants to merge 2 commits intoapache:masterfrom
rpatrick00:master

Conversation

@rpatrick00
Copy link
Copy Markdown

…he search for the .mvn directory at the location of the specified POM when present

…he search for the .mvn directory at the location of the specified POM when present
@rpatrick00 rpatrick00 closed this Sep 6, 2016
@rpatrick00
Copy link
Copy Markdown
Author

Fixing a small problem...will resubmit

@rpatrick00 rpatrick00 reopened this Sep 6, 2016
@jochenw
Copy link
Copy Markdown

jochenw commented Sep 15, 2016

I wonder, whether this couldn't be handled at the Java level? If so, there would be no need to change script files, making them even more complex than they already are.

@rpatrick00
Copy link
Copy Markdown
Author

Because the maven shell scripts are finding the .mvn directory before starting java in order to find the relevant .mvn/jvm.config to process any java command-line options that may be found in it. This location is passed into the Java process so that the Java code does not look for the .mvn directory again...

@jochenw
Copy link
Copy Markdown

jochenw commented Sep 15, 2016

Then possibly a good first step would be to move that logic from the scripts to the Java Main class.

@rpatrick00
Copy link
Copy Markdown
Author

rpatrick00 commented Sep 15, 2016

Did you not understand what I said?

.mvn/jvm.config might look like this:

-Xmx2048m
-Xms1024m
-XX:MaxPermSize=512m
-Djava.awt.headless=true

If I move the processing of that file into Java, how am I going to set those options on the Java command-line used to start Maven? There is a chicken-and-egg problem in that I need to add these options to the Java command-line so if I process the file in Java, it is too late (without forking and execing a Java process which seems worse than the shell script additions). See the JVM and Command Line Options section at https://maven.apache.org/docs/3.3.1/release-notes.html

Note that I did not create this .mvn directory feature or contribute to its implementation. I am simply trying to fix it so that the Jenkins maven-plugin will work for Maven projects that rely on the .mvn directory feature. Since Jenkins does not execute the Maven build from the ${project.basedir} and relies on running Maven with the "-f /path/to/project/basedir/pom.xml" option, Jenkins builds that rely on the .mvn directory feature fail. The mvn shell scripts were already locating the .mvn directory--all I did was correct a flaw in the shell-script logic that assumed the current execution directory was in ${project.basedir} or one of its sub-directories.

You might want to re-read the discussion in https://issues.apache.org/jira/browse/MNG-5889

@Tibor17
Copy link
Copy Markdown
Contributor

Tibor17 commented Sep 15, 2016

IIUC this might be useful in CI because you may run mvn deploy -f /path/to/project/pom.xml and the discovery would be made in /path/to/project/.mvn as well. AFAIK Maven is not 2 processes running but one and you need to pass jvm options like this then.

@aheritier
Copy link
Copy Markdown
Member

+1000 for this change (as far as it is tested/validated)
This problem of using -f on CI servers (and non only) is very common and it makes the usage of .mvn folders useless.
The problem is to be sure that we are fixing this for linux and windows

@rpatrick00
Copy link
Copy Markdown
Author

I tested this on both Linux and Windows, but most of the testing was on Windows simply because Windows CMD scripts are challenging to get right. The sh script is much simpler...

@Tibor17
Copy link
Copy Markdown
Contributor

Tibor17 commented Sep 15, 2016

I want to test it as well and then push it to master.

@Tibor17
Copy link
Copy Markdown
Contributor

Tibor17 commented Sep 15, 2016

@rpatrick00 pls squash it to one commit.

@rpatrick00 rpatrick00 closed this Sep 15, 2016
@rpatrick00
Copy link
Copy Markdown
Author

Submitting a new pull request...

#94

@rpatrick00
Copy link
Copy Markdown
Author

@Tibor17, have you looked at the pull request? Any issues with merging it?

pzygielo pushed a commit to pzygielo/maven that referenced this pull request Jan 4, 2023
gnodet pushed a commit to gnodet/maven that referenced this pull request Nov 20, 2024
…ilure point

Use a fresh, preconfigured verifier and do not clear CLI options because
on clear all preconfigurations, especially global settings are gone.

This issue can be observed when Maven is ran with MNG-4645 where
preconfigured Maven Central is lost in global settings.

This closes apache#93
@jira-importer
Copy link
Copy Markdown

Resolve #7889

gnodet added a commit to gnodet/maven that referenced this pull request Nov 7, 2025
- Added PR apache#93 for convenience utility methods
- Updated remaining tasks status
- All domtrip enhancement PRs now submitted
- Migration fully complete, waiting for PR reviews
gnodet added a commit to gnodet/maven that referenced this pull request Nov 8, 2025
Major milestone reached:
- PR apache#91 (plugin management methods) approved by cstamas and ready for merge
- PR apache#93 (convenience methods) has CI passing and awaiting review
- All Maven tests continue to pass (214 mvnup + 371 maven-cli)
- Full build successful without any JDOM2 dependencies

The DomTrip migration is 100% complete and fully functional.
Next step is waiting for the approved PR to be merged.
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.

6 participants