-
Notifications
You must be signed in to change notification settings - Fork 174
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
[SCM-706] finer-grained handling of file rename status for gitexe provider... #31
base: master
Are you sure you want to change the base?
Conversation
Can you please also take a look at #26, which touches the same area. It would make sense to apply these two patches in one go. |
3f2e345
to
65ff544
Compare
Rebased the PR on top of the current master |
When can this PR be approved and merged? We have migrated from SVN to Git/Stash but we can't use release-with-pom anymore and the team is not happy. |
HI sergei, I patched the latest scm with the patch on top, built it as version 1.9.5-beta-1 and use it with version 2.5 maven release plugin as such:
However I am getting error: [INFO] Checking in modified POMs... I am wondering about which version of maven-release-plugin did you try the patch with? Did you run 'release-with-pom'? Thank you |
@indrgun
Because you can see that Maven actually changes to that directory, but then passes to the git command a path to the pom, which is relative to the top-level project directory. |
With maven-release-plugin version 2.5 and dependency on
|
I do not specify pomFileName for maven release prepare plugin here. |
What is the command line you are using, and what is the directory you are running it from, and where is the root of your git repo (my guess is |
The command line supplied to Jenkins maven release field in maven project: "-Dresume=false -Prelease-profile release:prepare-with-pom release:perform -s ${M2_HOME}/conf/settings-rel.xml"
I presume should apply when maven perform goal runs. I guess it is used by release-prepare-with-pom. |
Actually I thnk you may be onto something here. It looks like the patch may have a problem with relative paths, when it is not run from the root of the git repository. I'll need to rebuild my test set-up and try it. |
What if you remove the default 'perform' execution completely and instead append the parameter to Again, it may not solve the problem if you are running from a subdirectory. |
I am getting same error:
with command line: |
You are right the patch #31 does not work probably if the pom.xml is not in the root directory of git repo. |
why the git add appends the subdir path again to the pom.xml? This does not happen with maven release prepare goal.
|
Maven passes |
Without the patch, using version 1.9.1 of maven-scm-api and maven-scm-provider-gitexe, I am running into git add error that is different:
The release-pom.xml was actually deleted by git but the subsequent git add wanted to add it. As you said the patch is supposed to fix this. |
When can we expect a release for this commit? |
@YogiKhan see the message "This branch has conflicts that must be resolved" |
@sergei-ivanov can we resolve that. This plugin is quite powerful and we are waiting for so long to get that bug fix. Can you please resolve and merge. |
…der, rename source is not added to the set of files for commit operation anymore
@olamy I've done a rebase onto the latest master and re-pushed the changes to the PR branch. However, there still exists an unresolved regression (as pointed out by @indrgun), when the release is being run for a submodule of a bigger project. In that case, the working directory is a number of levels down from the git repository root, and somewhere in the git scm provider code there's a subtle bug related to path relativisation. I spent a few hours trying to track it down, and still could not put my finger on it. I would really appreciate it if someone like @rfscholte took a look at it, because he had previously invested a lot of effort in that area, and may still have a better idea of what is going on there. |
Thanks @sergei-ivanov @olamy can we merge that and create a release ? is it gonna take time? and once merge done, hope it will resolve the below error. [ERROR] Provider message: |
@sergei-ivanov, till it get merge, where i can find the 1.9.5-beta1 version? So that i can test it ? |
@YogiKhan you will need to build this PR branch of |
I need help with that from the Apache team. |
I think we missed something.
That was probably true before introducing the
So we need to go through the codebase to verify if the proper paths are used. |
https://github.com/apache/maven-scm/blob/master/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/test/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusConsumerTest.java#L191 confirms this if you see that subDirectory is actually the working directory. |
@sergei-ivanov can you take a look at what @rfscholte suggested. After running it manually suggested by you, things works for root but not working for submodules. |
@YogiKhan Unfortunately, I haven't got the resources to actively work on this issue. I've already spent an awful lot of time debugging it, and still have no clear idea where exactly it goes wrong. At some point I had an suspicion that we were relativising the wrong way round, but when I played around again yesterday, it kind of made sense what was going on there. What scares me is that this code appears to be quite sensitive to changes, and any change in working directory logic may cause unwanted side effects elsewhere. Feel free to fork my fork and experiment. I guess the way to go might be by rigging proper integration tests to cover all cases we are dealing with. |
|
This bug is critical because it is impossible to work with release:prepare-with-pom. Example of use: jenkinsfile
pom.xml
Jenkins out compilation:
If i change to 'release:prepare', this works.
😢 😭 |
Can someone please rebase and drop unaffected changes even if they are correct? We also need a proper IT which reproduces the issue. I'd be glad to merge this then. |
+1, unable to run with 2.5.3 even with release:prepare.
Building on Jenkins, I get
|
@feliperoos my request from last August still holds true. |
Anyone wants to pick this up? |
..., rename source is not added to the set of files for commit operation anymore
This is an actualisation of the original Darryl L. Miles's patch from SCM-706.
All tests for gitexe provider are passing locally.
I've also confirmed locally that it fixes the
release-with-pom
behaviour inmaven-release-plugin
under git.