jira-importer opened a new issue, #1252: URL: https://github.com/apache/maven-scm/issues/1252
**[Jakob Vad Nielsen](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=lazeetown)** opened **[SCM-1027](https://issues.apache.org/jira/browse/SCM-1027?redirect=false)** and commented 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) ```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; } ``` 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: ```java git diff --cached HEAD~2 HEAD~1 ``` From git doc: ```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. ``` 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: 1. Introduce a boolean parameter to the exeucteDiffCommand. Like `includeCached` or `includeStagedChanges`. This way I could at least chose to not include staged. 2. 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. --- **Affects:** 1.13.0, 2.0.0, 2.0.1, 2.1.0 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
