You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Stephan202 <gi...@git.apache.org> on 2015/03/18 14:27:07 UTC
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
GitHub user Stephan202 opened a pull request:
https://github.com/apache/maven/pull/40
MNG-5786: Fix edge case in determining maven.multiModuleProjectDirectory.
See [MNG-5786](https://jira.codehaus.org/browse/MNG-5786) for details. The commit message explains the issue in different words:
MNG-5786: Fix maven.multiModuleProjectDirectory.
Fixes an edge case where maven.multiModuleProjectDirectory is set incorrectly.
Consider the following scenario:
/some/path/to/the/workspace/.mvn
/some/path/to/the/workspace/project/.mvn
/some/path/to/the/workspace/project/pom.xml
Prior to the fix, running Maven inside the *project* directory will cause
maven.multiModuleProjectDirectory to be set to the *workspace* directory. The
fix entails testing for the presence of .mvn before traversing one directory
up, rather than afterwards.
The loop termination condition was moved inside the loop, so as to ensure that
.mvn can also be placed in the file system root.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Stephan202/maven MNG-5786
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/maven/pull/40.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #40
----
commit 5374bfcaff59414ca9b580cefbab8880bf782d42
Author: Stephan Schroevers <st...@escaladagroup.com>
Date: 2015-03-18T13:18:18Z
MNG-5786: Fix maven.multiModuleProjectDirectory.
Fixes an edge case where maven.multiModuleProjectDirectory is set incorrectly.
Consider the following scenario:
/some/path/to/the/workspace/.mvn
/some/path/to/the/workspace/project/.mvn
/some/path/to/the/workspace/project/pom.xml
Prior to the fix, running Maven inside the *project* directory will cause
maven.multiModuleProjectDirectory to be set to the *workspace* directory. The
fix entails testing for the presence of .mvn before traversing one directory
up, rather than afterwards.
The loop termination condition was moved inside the loop, so as to ensure that
.mvn can also be placed in the file system root.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
Posted by jvanzyl <gi...@git.apache.org>.
Github user jvanzyl commented on the pull request:
https://github.com/apache/maven/pull/40#issuecomment-89769509
I'll take a look at try to incorporate into the 3.3.2 release which I'd like to get our next week.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
Posted by Stephan202 <gi...@git.apache.org>.
Github user Stephan202 commented on the pull request:
https://github.com/apache/maven/pull/40#issuecomment-89738913
I can't seem to comment on [MNG-5786](https://jira.codehaus.org/browse/MNG-5786) anymore (comment button missing), presumably because of the impending Codehaus shutdown. So I'll continue the discussion here.
@jvanzyl wrote:
> The intended use and behaviour is to have one `.mvn` at the root of the multi-module project. By design we currently do not expect there to be multiple `.mvn` directories within one project tree. So I consider this behaviour correct.
I understand wanting to keep the system simple and supporting only a single `.mvn` within the project tree. But as it stands, a "rogue" `.mvn` higher up, "outside" the project tree can silently impact the build. This can lead to hard to explain issues further down the line. As such, I suggest Maven either:
1. Warns if more than one `.mvn` is found in the path from CWD to root;
2. Fixes the code such that the "nearest" `.mvn` in the path from CWD to root always wins.
Option 2 is no more difficult to understand than "we support only one `.mvn`" (as one does not even need to think about error cases) and can be achieved with a trivial patch; that's precisely what this PR proposes.
Side note: if we feel uncomfortable considering any `.mvn` that may be outside the project tree, then the search logic should be extended with an extra termination condition: stop looking for a `.mvn` once the traversal up the directory tree reaches a directory that lacks a `pom.xml`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
Posted by Stephan202 <gi...@git.apache.org>.
Github user Stephan202 closed the pull request at:
https://github.com/apache/maven/pull/40
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
Posted by Stephan202 <gi...@git.apache.org>.
Github user Stephan202 commented on the pull request:
https://github.com/apache/maven/pull/40#issuecomment-140189697
A variation on the proposed fix was merged in #58. So I'll close this PR.
(I'll also delete by branch/fork, but the diff can still be viewed in [MNG-5786](https://issues.apache.org/jira/browse/MNG-5786).)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
[GitHub] maven pull request: MNG-5786: Fix edge case in determining maven.m...
Posted by ifedorenko <gi...@git.apache.org>.
Github user ifedorenko commented on the pull request:
https://github.com/apache/maven/pull/40#issuecomment-89838717
@Stephan202 what is your usecase for having multiple maven or jvm options for the same codebase? Is this to support svn (and similar) repositories that effectively host multiple codebases or something else? .mvn is meant to configure environment parameters like jvm -Xmx or builder and I can't think of a reason these would be different for different parts of the same codebase.
As for searching "true" multi-module project root, lack of `pom.xml` is not a good indicator traversal reached outside project tree. First, we wanted to support alternative pom formats with this feature. Second, even for projects that do use `pom.xml`, parent pom does not have to be at the root of the project codebase.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org