From a5aad5a99b77b61e76145b9a25b8b703bf623eed Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 21 Mar 2017 11:06:09 -0700 Subject: [PATCH 1/2] git/commits: Test merge commits Partially reverting ca825225 (git: do not include merges in the commit range, 2017-03-21, #16). I've kept the unrelated --no-pager addition from that commit. I think merges in the commit range should be checked against whatever criteria the caller is asking for. If the caller does not want to check a machine-generated commit, they shouldn't include it in the commit range. And we already have special-cases for merges. For example, we skip DCO checks for merge commits. I'd be happier without that special case, because human-generated merge commits can still add novel code (and therefore should have Signed-off-by), etc. But I don't think we want a blanket pass for merge commits. Signed-off-by: W. Trevor King --- git/commits.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/commits.go b/git/commits.go index c84e64c..a9d5685 100644 --- a/git/commits.go +++ b/git/commits.go @@ -12,7 +12,7 @@ import ( // If commitrange is a git still range 12345...54321, then it will be isolated set of commits. // If commitrange is a single commit, all ancestor commits up through the hash provided. func Commits(commitrange string) ([]CommitEntry, error) { - cmdArgs := []string{"git", "--no-pager", "log", `--no-merges`, `--pretty=format:%H`, commitrange} + cmdArgs := []string{"git", "--no-pager", "log", `--pretty=format:%H`, commitrange} if debug() { logrus.Infof("[git] cmd: %q", strings.Join(cmdArgs, " ")) } From cdd7165f804affb39d5ab3a7876e17966858b714 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 21 Mar 2017 11:56:45 -0700 Subject: [PATCH 2/2] main: Return to using TRAVIS_COMMIT_RANGE (with ... -> .. fix) Master builds only have a 'git clone ...' [1] so FETCH_HEAD isn't defined and git-validation crashes [2]. This commit partially reverts 8a12a8fc (main: default travis commit range is unreliable, 2017-03-21, #13) to avoid that crash. If TRAVIS_COMMIT_RANGE is unset [3], falling back to TRAVIS_COMMIT may be fine. The ... -> .. replacement works around travis-ci/travis-ci#4596 until that is fixed upstream [4]. This avoids pulling in commits from the base tip that aren't reachable from the head tip (e.g. if master has advanced since the PR branched off, and the PR is against master). We only want to check commits that are in the head branch but not in the base branch (more details on the range syntax in [5]). Once the Travis bug does get fixed, the shell replacement will be a no-op. So we don't have to worry about checks breaking once the bug gets fixed, and can periodically poll the bug and remove the workaround at out leisure after the fix. [1]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L243 [2]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L347 [3]: https://github.com/opencontainers/runc/pull/1378#issuecomment-287903471 [4]: https://github.com/travis-ci/travis-ci/issues/4596 [5]: http://git-scm.com/docs/gitrevisions#_specifying_ranges Signed-off-by: W. Trevor King --- .travis.yml | 1 + main.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index b053bff..7c1ecfc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,6 +11,7 @@ before_install: - mkdir -p $GOPATH/src/github.com/vbatts && ln -sf $(pwd) $GOPATH/src/github.com/vbatts/git-validation && go get ./... before_script: + - echo $TRAVIS_COMMIT_RANGE - echo $TRAVIS_COMMIT - echo $TRAVIS_BRANCH - echo $TRAVIS_TAG diff --git a/main.go b/main.go index 65d4bb0..36d0f77 100644 --- a/main.go +++ b/main.go @@ -50,8 +50,8 @@ func main() { var commitRange = *flCommitRange if commitRange == "" { if strings.ToLower(os.Getenv("TRAVIS")) == "true" && !*flNoTravis { - if os.Getenv("TRAVIS_BRANCH") != "" { - commitRange = fmt.Sprintf("%s..FETCH_HEAD", os.Getenv("TRAVIS_BRANCH")) + if os.Getenv("TRAVIS_COMMIT_RANGE") != "" { + commitRange = strings.Replace("...", "..", os.Getenv("TRAVIS_COMMIT_RANGE"), 1) } else if os.Getenv("TRAVIS_COMMIT") != "" { commitRange = os.Getenv("TRAVIS_COMMIT") }