maven-scm
maven-scm copied to clipboard
apache
[SCM-706] finer-grained handling of file rename status for gitexe provider...
..., 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 in maven-release-plugin under git.
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.
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:
<id>release-profile</id>
<build>
<plugins>
...
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<version>2.5</version>
<dependencies>
<dependency>
<groupId>org.apache.maven.scm</groupId>
<artifactId>maven-scm-api</artifactId>
<version>1.9.5-beta-1</version>
</dependency>
<dependency>
<groupId>org.apache.maven.scm</groupId>
<artifactId>maven-scm-provider-gitexe</artifactId>
<version>1.9.5-beta-1</version>
</dependency>
</dependencies>
<configuration>
<scmCommentPrefix>${release.commit.comment} [maven-release-plugin]</scmCommentPrefix>
<preparationGoals>-s ${release.settings.file} clean install -DskipTests=true</preparationGoals>
<goals>-s ${release.settings.file} deploy -DskipTests=true</goals>
<username>${release.commit.user}</username>
<localCheckout>true</localCheckout>
<!-- <autoVersionSubmodules>true</autoVersionSubmodules> -->
</configuration>
<executions>
<execution>
<id>default</id>
<goals>
<goal>perform</goal>
</goals>
<configuration>
<pomFileName>apic_int/comp/ifm_apic_rest/pom.xml</pomFileName>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
However I am getting error:
[INFO] Checking in modified POMs... [INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git rev-parse --show-toplevel [INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest [INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git status --porcelain . [INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest [WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/pom.xml.releaseBackup [WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/release.properties [INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git add -- apic_int/comp/ifm_apic_rest/pom.xml apic_int/comp/ifm_apic_rest/release-pom.xml [INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 01:52 min [INFO] Finished at: 2016-02-02T13:09:29-08:00 [INFO] Final Memory: 58M/378M [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare-with-pom (default-cli) on project ifm_apic_rest: Unable to commit files [ERROR] Provider message: [ERROR] The git-add command failed. [ERROR] Command output: [ERROR] fatal: pathspec 'apic_int/comp/ifm_apic_rest/pom.xml' did not match any files
I am wondering about which version of maven-release-plugin did you try the patch with? Did you run 'release-with-pom'?
Thank you -Indra
@indrgun I tested it on a vanilla project, which did not have any major config alterations for the release plugin. I wonder if the following bit of configuration kicks in and messes up the relative paths:
<configuration>
<pomFileName>apic_int/comp/ifm_apic_rest/pom.xml</pomFileName>
</configuration>
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.
Does mvn release:prepare (not prepare-with-pom) work for you with the same version of m-r-p and the same settings, but without the SCM provider patch? I have a feeling that the issue you are facing may be of a different nature than this patch is trying to address.
Alternatively, you may try prefixing the pomFileName path with ${project.basedir}, which will turn it into an absolute path.
With maven-release-plugin version 2.5 and dependency on maven-scm-provider-gitexe version 1.9.1 including that configuration (our pom.xml is not on root of git repo), mvn release prepare works. On Feb 2, 2016 4:04 PM, "Sergei Ivanov" [email protected] wrote:
@indrgun https://github.com/indrgun I tested it on a vanilla project, which did not have any major config alterations for the release plugin. I wonder if the following bit of configuration kicks in and messes up the relative paths:
apic_int/comp/ifm_apic_rest/pom.xml 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. Does mvn release:prepare (not prepare-with-pom) work for you with the same version of m-r-p and the same settings, but without the SCM provider patch? I have a feeling that the issue you are facing may be of a different nature than this patch is trying to address. Alternatively, you may try prefixing the pomFileName path with ${project.basedir}, which will turn it into an absolute path.
— Reply to this email directly or view it on GitHub https://github.com/apache/maven-scm/pull/31#issuecomment-178898208.
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 /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/)? Do you need to activate release-profile explicitly? I still think it may be a combination of factors that results in the above error, but I'll also take another look at my patch now.
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"
<execution>
<id>default</id>
<goals>
<goal>perform</goal>
</goals>
<configuration>
<pomFileName>comp/ifm_apic_rest/pom.xml</pomFileName>
</configuration>
</execution>
</executions>
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 <goals> as -DpomFileName=apic_int/comp/ifm_apic_rest/pom.xml ?
Again, it may not solve the problem if you are running from a subdirectory.
There are actually 2 separate patches: patch #31 and patch #26. Which one has the problem?
I applied your patch #31 and I tried it and got same error.
I then applied patch #26 and same error still persists.
I haven't tried #26 and I do not think it makes any difference here. I think the problem is with #31 in the way it tries to calculate the set of files fo be added to git.
I am getting same error:
[INFO] Checking in modified POMs...
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git rev-parse --show-toplevel
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git status --porcelain .
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/pom.xml.releaseBackup
[WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/release.properties
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git add -- apic_int/comp/ifm_apic_rest/pom.xml apic_int/comp/ifm_apic_rest/release-pom.xml
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:59 min
[INFO] Finished at: 2016-02-02T17:10:06-08:00
[INFO] Final Memory: 60M/351M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare-with-pom (default-cli) on project ifm_apic_rest: Unable to commit files
[ERROR] Provider message:
[ERROR] The git-add command failed.
[ERROR] Command output:
[ERROR] fatal: pathspec 'apic_int/comp/ifm_apic_rest/pom.xml' did not match any files
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
with command line: -Dresume=false -Prelease-profile release:prepare-with-pom release:perform -s ${M2_HOME}/conf/settings-rel.xml -DpomFileName=apic_int/comp/ifm_apic_rest/pom.xml
You are right the patch #31 does not work probably if the pom.xml is not in the root directory of git repo. I removed pomFileName in pom.xml and also in the command line and I am still getting same error.
why the git add appends the subdir path again to the pom.xml? This does not happen with maven release prepare goal.
[WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/pom.xml.releaseBackup
[WARNING] Ignoring unrecognized line: ?? apic_int/comp/ifm_apic_rest/release.properties
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git add -- apic_int/comp/ifm_apic_rest/pom.xml apic_int/comp/ifm_apic_rest/release-pom.xml
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:38 min
[INFO] Finished at: 2016-02-02T17:16:34-08:00
[INFO] Final Memory: 58M/376M
[INFO] ------------------------------------------------------------------------
Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare-with-pom (default-cli) on project ifm_apic_rest: Unable to commit files
[ERROR] Provider message:
[ERROR] The git-add command failed.
[ERROR] Command output:
[ERROR] fatal: pathspec 'apic_int/comp/ifm_apic_rest/pom.xml' did not match any files
Maven passes release-pom.xml and pom.xml to the GitCheckInCommand. Previously, it would attempt to git add both before doing the git commit. Naturally, that would fail for release-pom.xml, which was deleted, not added. The purpose of patch #31 was to run the status command and to figure out, which of the specified files actually need to be added. Unfortunately, git status returns paths relative to git root, and there appears to be a problem converting them back to the paths relative to the working directory. I have spent the last couple of hours trying to figure out what is going wrong there, and I think it works correctly when the git root is equal to or below the working directory, but does not work when the working directory is below the git root.
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:
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git ls-files
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] Transforming 'ifm_apic_rest'...
[INFO] Removing release POM for 'ifm_apic_rest'...
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git rm release-pom.xml
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] Checking in modified POMs...
[INFO] Executing: /bin/sh -c cd /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest && git add -- pom.xml release-pom.xml
[INFO] Working directory: /scratch/jenkinsci-ws/workspace/apic-int-ifm_apic_rest/apic_int/comp/ifm_apic_rest
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:45 min
[INFO] Finished at: 2016-02-03T11:05:11-08:00
[INFO] Final Memory: 60M/386M
[INFO] ------------------------------------------------------------------------
Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5:prepare-with-pom (default-cli) on project ifm_apic_rest: Unable to commit files
[ERROR] Provider message:
[ERROR] The git-add command failed.
[ERROR] Command output:
[ERROR] fatal: pathspec 'release-pom.xml' did not match any files
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.
@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.
@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: [ERROR] The git-add command failed. [ERROR] Command output: [ERROR] fatal: pathspec 'release-pom.xml' did not match any files [ERROR] -> [Help 1]
@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 maven-scm, and then build https://github.com/apache/maven-release project against it. Then you'll need to update the version of maven-release-plugin in your own project to the version you've just built and test it.
I need help with that from the Apache team.
In the original version of GitCheckInCommand, just before committing, the list of files to be committed was passed to git add. This ensured that all the new files were staged into the commit.
Unfortunately, it did not work when there were deleted files in that list. So, the patched version attempts to check git status on each of the files in the list, in order to filter out the deleted files from the list. See the changes in GitCheckInCommand.
Here is a problem though. Some of the git commands assume file paths relative to the working directory, while the other work with paths relative to the git repository root. Accordingly, the file paths need to be converted back and forth, and it seems that at some point we have a mismatch.
This caused grief previously (https://issues.apache.org/jira/browse/SCM-709) and it looks like we are observing yet another manifestation of the same problem.
I think we missed something. GitStatusConsumer says
/**
* Entries are relative to working directory, not to the repositoryroot
*/
private List<ScmFile> changedFiles = new ArrayList<ScmFile>();
That was probably true before introducing the --porcelain argument, because the related git-status docs says
Porcelain Format
The porcelain format is similar to the short format, but is guaranteed not to change in a backwards-incompatible way between Git versions or based on user configuration. This makes it ideal for parsing by scripts. The description of the short format above also describes the porcelain format, with a few exceptions:
1.The user’s color.status configuration is not respected; color will always be off.
2.The user’s status.relativePaths configuration is not respected; paths shown will always be relative to the repository root.
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. Also release-pom.xml should not get checked-in, thats why i think we had the git rm to remove those. Once we add the pom and release-pom.xml to git tag, we need to remove the release-pom.xml from tag.
@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.