[
https://issues.apache.org/jira/browse/SCM-1027?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jakob Vad Nielsen updated SCM-1027:
-----------------------------------
Summary: Wrong implementation of git diff --cached (when endRevision set)
(was: Wrong implementation of git diff, with --cached argument when endRevision
set)
> Wrong implementation of git diff --cached (when endRevision set)
> ----------------------------------------------------------------
>
> Key: SCM-1027
> URL: https://issues.apache.org/jira/browse/SCM-1027
> Project: Maven SCM
> Issue Type: Bug
> Components: maven-scm-provider-gitexe, maven-scm-provider-jgit
> Affects Versions: 1.13.0, 2.0.0, 2.0.1, 2.1.0
> Reporter: Jakob Vad Nielsen
> Priority: Major
>
> Hi,
> The {{createCommandLine}} function In {{{}GitDiffCommand.java{}}}, has a
> buggy implementation. The bug appears to have been there for a very long
> time. It is triggered when giving both {{startRevision}} and {{endRevision}}
> to this method (via {{{}executeDiffCommand{}}})
> [link to the method
> |https://github.com/apache/maven-scm/blob/master/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/diff/GitDiffCommand.java#L75]
> {code:java}
> public static Commandline createCommandLine(File workingDirectory, ScmVersion
> startVersion, ScmVersion endVersion, boolean cached) {
>
> Commandline cl =
> GitCommandLineUtils.getBaseGitCommandLine(workingDirectory, "diff");
> if (cached) {
> cl.createArg().setValue("--cached");
> }
> if (startVersion != null && StringUtils.isNotEmpty(startVersion.getName()))
> {
> cl.createArg().setValue(startVersion.getName());
> }
> if (endVersion != null && StringUtils.isNotEmpty(endVersion.getName())) {
> cl.createArg().setValue(endVersion.getName());
> }
> return cl;
> } {code}
> The problem is that both startRevision and endRevision are added as arguments
> to git diff when the --cached flag is set. Git diff only supports one
> argument in that case. It has been like that since the flag was introduced in
> Git, long time ago. Setting an endRevision throws an exception, as the git
> runtime fails with two arguments.
> Reproduce by running this command in any repo:
> {code:java}
> git diff --cached HEAD~2 HEAD~1 {code}
> From git doc:
> {code:java}
> git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]
> This form is to view the changes you staged for the next commit
> relative to the named <commit>. Typically you would want comparison with the
> latest commit, so if you do not give <commit>, it defaults to HEAD. If HEAD
> does not exist (e.g. unborn branches) and <commit> is not
> given, it shows all staged changes. --staged is a synonym of
> --cached. {code}
> The easy fix would be to make sure the endVersion argument is not added when
> the boolean cached argument is set to true.
> But I do not think this is actually the problem. When I want to do a diff
> between two revisions, I do not want to get a diff that also includes staged
> changes (which is what --cached actually does). Actually, I do not want it at
> all. I suspect that this diff command was introduced to solve special case
> for the Maven Release Plugin, where you want to diff against a specific
> commit AND make sure there are no staged commits. And then solved both
> scenarios in the same executeDiffCommand.
> In order for me to be able to use this code in my plugin (where I want to
> diff between for example HEAD~3 and HEAD~2), I think there are two possible
> solutions:
> # Introduce a boolean parameter to the exeucteDiffCommand. Like
> `includeCached` or `includeStagedChanges`. This way I could at least chose to
> not include staged.
> # Have two diff commands. DiffCommand and DiffCachedCommand.
> I guess both variants in some way would introduce a breaking change, unless
> new commands are added in addition to the existing one.
> Just wanted to let you know. I can help out with a PR if this is something
> you want to fix. If so, we need to agree on what the right solution would be.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
