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

MRELEASE-875 #17

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@imod
Contributor

imod commented Jul 20, 2014

Even though I have commit rights myself, I would like to have other to take a look at this first.
This PR fixes an issue voted my a couple of people https://jira.codehaus.org/browse/MRELEASE-875
The fix will allow to do a release of artifacts/repos not having a pom.xml in the root of the git-repository.

The PR fixes the issue for maven-scm-provider-jgit and maven-scm-provider-gitexe

given the follwing directory layout:

.git
subdir1/
subdir1/subdir2/pom.xml
subdir1/subdir2/module1/pom.xml

the following will be possible:
in subdir2:

$ mvn -f pom.xml release:prepare release:perform 

in subdir1:

$ mvn -f subdir2/pom.xml release:prepare release:perform 

in root dir:

$ mvn -f subdir1/subdir2/pom.xml release:prepare release:perform 
@imod

This comment has been minimized.

Show comment
Hide comment
@imod

imod Jul 22, 2014

Contributor

@olamy @rfscholte what do you think about this PR?

Contributor

imod commented Jul 22, 2014

@olamy @rfscholte what do you think about this PR?

@rfscholte

This comment has been minimized.

Show comment
Hide comment
@rfscholte

rfscholte Jul 24, 2014

Contributor

I did a quick scan and I wonder if this is the right approach. IIUC the main problem is that release:perform doesn't calculate the working directory correctly. Sounds like a general issue, so my current idea is to solve this in the maven-release-plugin itself.

Contributor

rfscholte commented Jul 24, 2014

I did a quick scan and I wonder if this is the right approach. IIUC the main problem is that release:perform doesn't calculate the working directory correctly. Sounds like a general issue, so my current idea is to solve this in the maven-release-plugin itself.

@rlyons

This comment has been minimized.

Show comment
Hide comment
@rlyons

rlyons Jul 25, 2014

So close, so far away... I have a build issue with this exact problem... If this fixes it, I would be interested... perhaps it can be fixed in both places instead of one?

rlyons commented Jul 25, 2014

So close, so far away... I have a build issue with this exact problem... If this fixes it, I would be interested... perhaps it can be fixed in both places instead of one?

@rfscholte

This comment has been minimized.

Show comment
Hide comment
@rfscholte

rfscholte Jul 25, 2014

Contributor

@rlyons I don't think any scm-provider should be aware of requirements of the maven-release-plugin or any other plugin. The scm project is already complex, let's not introduce this kind of workarounds here.
Just to be fully correct: besides the pom.xml not being in the root, the other problem is that release:prepare release:perform are called at once. If this was done in 2 separate steps, you could call release:perform -DpomFileName=path/to/pom.xml.

Contributor

rfscholte commented Jul 25, 2014

@rlyons I don't think any scm-provider should be aware of requirements of the maven-release-plugin or any other plugin. The scm project is already complex, let's not introduce this kind of workarounds here.
Just to be fully correct: besides the pom.xml not being in the root, the other problem is that release:prepare release:perform are called at once. If this was done in 2 separate steps, you could call release:perform -DpomFileName=path/to/pom.xml.

@imod

This comment has been minimized.

Show comment
Hide comment
@imod

imod Jul 26, 2014

Contributor

@rlyons I agree with @rfscholte if we can fix it in the release plugin, then we should - but on the other hand, I never managed to get the workaround with the pomFileName to work - it always failed at release:prepare in every single try I gave to it. So I deffinetly think this must be fixed.

Even though we will not integrate this PR, I think it actually contains 80% of all whats required to fix the issue in the m-r-p (including test repos)

Contributor

imod commented Jul 26, 2014

@rlyons I agree with @rfscholte if we can fix it in the release plugin, then we should - but on the other hand, I never managed to get the workaround with the pomFileName to work - it always failed at release:prepare in every single try I gave to it. So I deffinetly think this must be fixed.

Even though we will not integrate this PR, I think it actually contains 80% of all whats required to fix the issue in the m-r-p (including test repos)

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 10, 2014

Contributor

I'm primarily seeing this as a prepare failure, it makes pomFileName useless. So I'd rather have a fix than not.

Contributor

bimargulies commented Sep 10, 2014

I'm primarily seeing this as a prepare failure, it makes pomFileName useless. So I'd rather have a fix than not.

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 14, 2014

Contributor

If you look at MRELEASE-875, you will see that I can't repro a problem here with the current trunk of the release plugin plus scm 1.9.1, using either git 1.8.5.2 or 2.1.0. Have you got any suggestions as to what might explain this?

Contributor

bimargulies commented Sep 14, 2014

If you look at MRELEASE-875, you will see that I can't repro a problem here with the current trunk of the release plugin plus scm 1.9.1, using either git 1.8.5.2 or 2.1.0. Have you got any suggestions as to what might explain this?

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 14, 2014

Contributor

Hang on, is 'mvn -f' the critical issue here? I always cd into the directory in question.

Also, in the test repo you checked in, sub1/sub2/pom.xml isn't a pom, it's just a file with a pathname.

Contributor

bimargulies commented Sep 14, 2014

Hang on, is 'mvn -f' the critical issue here? I always cd into the directory in question.

Also, in the test repo you checked in, sub1/sub2/pom.xml isn't a pom, it's just a file with a pathname.

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 14, 2014

Contributor

@imod

As per remarks I've added to MRELEASE-875, I can see things broken with 2.5.0 and I see things working fine it I change the m-r-p to use SCM 1.9.1. If I've missed a repro process, pleaseplease add it to the jira. Elsewise I think I should close this PR.

Contributor

bimargulies commented Sep 14, 2014

@imod

As per remarks I've added to MRELEASE-875, I can see things broken with 2.5.0 and I see things working fine it I change the m-r-p to use SCM 1.9.1. If I've missed a repro process, pleaseplease add it to the jira. Elsewise I think I should close this PR.

ignorecase = true
precomposeunicode = true
[remote "origin"]
url = /Users/domi/work/gitrepo/maven-scm/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/target/scm-test/repository2/.git

This comment has been minimized.

@NiematojakTomasz

NiematojakTomasz Sep 14, 2014

"/Users/domi/" ...

@NiematojakTomasz

NiematojakTomasz Sep 14, 2014

"/Users/domi/" ...

@asfgit asfgit closed this in 027cdbe Sep 14, 2014

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 14, 2014

Contributor

@imod I am very sorry, I closed this purely by accident while trying to clean up some others. I really intended to wait and see.

Contributor

bimargulies commented Sep 14, 2014

@imod I am very sorry, I closed this purely by accident while trying to clean up some others. I really intended to wait and see.

@imod

This comment has been minimized.

Show comment
Hide comment
@imod

imod Sep 16, 2014

Contributor

@bimargulies no worries - I have seen your comments on MRELEASE-875...
I'll try to find some time to reproduce the issue with your suggested setup.

Contributor

imod commented Sep 16, 2014

@bimargulies no worries - I have seen your comments on MRELEASE-875...
I'll try to find some time to reproduce the issue with your suggested setup.

@imod

This comment has been minimized.

Show comment
Hide comment
@imod

imod Sep 16, 2014

Contributor

@bimargulies the hole reason for this is PR is to get it to work with mvn -f ... so without having to cd into the directory. Without this possibility its hard to get this to work in a Ci server e.g. Jenkins with the m2release-plugin.

Contributor

imod commented Sep 16, 2014

@bimargulies the hole reason for this is PR is to get it to work with mvn -f ... so without having to cd into the directory. Without this possibility its hard to get this to work in a Ci server e.g. Jenkins with the m2release-plugin.

@bimargulies

This comment has been minimized.

Show comment
Hide comment
@bimargulies

bimargulies Sep 16, 2014

Contributor

That worked for me. I will try it again.
On Sep 16, 2014 3:08 AM, "Dominik Bartholdi" notifications@github.com
wrote:

@bimargulies https://github.com/bimargulies the hole reason for this is
PR is to get it to work with mvn -f ... so without having to cd into the
directory. Without this possibility its hard to get this to work in a Ci
server e.g. Jenkins with the m2release-plugin.


Reply to this email directly or view it on GitHub
#17 (comment).

Contributor

bimargulies commented Sep 16, 2014

That worked for me. I will try it again.
On Sep 16, 2014 3:08 AM, "Dominik Bartholdi" notifications@github.com
wrote:

@bimargulies https://github.com/bimargulies the hole reason for this is
PR is to get it to work with mvn -f ... so without having to cd into the
directory. Without this possibility its hard to get this to work in a Ci
server e.g. Jenkins with the m2release-plugin.


Reply to this email directly or view it on GitHub
#17 (comment).

@hubot hubot deleted the MRELEASE-875 branch Apr 28, 2017

@hubot hubot restored the MRELEASE-875 branch Apr 28, 2017

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