jenkinsci/git-client-plugin — pull request #144 by Tetradeus (2014-09-22 16:20:13+00:00)
Title: JENKINS-23606 Fixed showrevision to list only modified paths with merge ...

### Git Polling Jobs with Included Regions being triggered unnecessarily with merge commits

#### Example
I have two branch : A and B.

```
A B

A1
| |
... |
| /
A-n
```
I have a job responsible to watch and build the branch A.
A build is triggered by the commit A1 because A1 changed a watched path (included region).

Then, a new commit arrives on branch A : A2 (did not change any watched path)
Then the branch B is merged on the branch A and result in a new commit :A3 (did not change any watched path)

```
A B

A3
| \
A2 B1
| |
A1 |
| |
... |
| /
A-n
```

A job is triggered due to an SCM change.

Here is the resulting polling log :
```
Polling SCM changes on xxx
Using strategy: Default
[poll] Last Built Revision: Revision A1 (origin/A)
> git rev-parse --is-inside-work-tree
Fetching changes from the remote Git repositories
> git config remote.origin.url git@xxx/xxx.git
Fetching upstream changes from git@xxx/xxx.git
> git --version
> git fetch --tags --progress git@xxx/xxx.git +refs/heads/A:refs/remotes/origin/A
Polling for changes in
> git rev-parse origin/A^{commit}
> git log --full-history --no-abbrev --format=raw -M -m --raw A1..A3
Ignored commit A3: Found only excluded paths:
Done. Took 7,3 s
Changes found
```

The interesting command is this one :
```
git log --full-history --no-abbrev --format=raw -M -m --raw A1..A3
```

What this command do is to log the diff between A1 and A3 from both side of the merge (the -m option)

```
commit A3 (from A1)
tree xxx
parent A1
parent B1
author webadmin account 1409229426 +0200
committer webadmin account 1409229426 +0200

Merge remote-tracking branch 'origin/B' into A

:100644 100644 effd34ca258b275bbe3cfc33ddb90cee3ad2a6c7 cb5c83f47c316a15cfea476ceb560152537dc334 M xxx.java

commit A3 (from B1)
tree xxx
parent A1
parent B1
author webadmin account 1409229426 +0200
committer webadmin account 1409229426 +0200

Merge remote-tracking branch 'origin/B' into A

All modified files between A2 and A-99 !!!
```

Indeed, this list all modified files on branch A compared to the branch B at the moment of the merge.
So in the modified files list, we will have all the modified files between A2 and A-99 (common ancestor)

What we would like to have is only the really modified files on the branch A during the merge.

It looks like this bug was known: https://github.com/jenkinsci/git-client-plugin/commit/340da905
```
[JENKINS-16993] Use "git log" to determine changed files.

The output of "git show" included commits that were not introduced since
the last build, so could falsely trigger build in the presence of
"includedRegions".

A benefit of not including extraneous commits is the size of the output
is reduced, and therefore less heap required.

This can still trigger extra builds as "-m" means the changes between all
legs of a merge commit are displayed. However, without this, conflict
resolution changed as part of the merge commit would be skipped.
```

I added some unit test to show that behavior and looked at the git merge help.
It looks like the -m option is needed to print the merge modified files but it exists the "--first-parent" option that will only list the commits and the modified files of the destination branch.
We don't really care about commits of the merged branch, only about modified files at merging time.

Here are the impacts on the git plugin
https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L1456

```java

private boolean isRevExcluded(GitClient git, Revision r, TaskListener listener, BuildData buildData) throws IOException, InterruptedException
try {
List revShow;
if (buildData != null && buildData.lastBuild != null) {
revShow = git.showRevision(buildData.lastBuild.revision.getSha1(), r.getSha1());
} else {
revShow = git.showRevision(r.getSha1());
}
```

For the moment, exclusions just come from a PathRestriction or a UserExclusion.
The PathRestriction is OK with the fix because all modified paths will be logged with the merge commit.
The UserRestriction is used to ignore commits from certain users, but because we are skipping commits, this option is also OK with the fix.