Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRAVIS_COMMIT_RANGE includes commits unrelated to PR #4596

Closed
jawshooah opened this issue Jul 30, 2015 · 18 comments
Closed

TRAVIS_COMMIT_RANGE includes commits unrelated to PR #4596

jawshooah opened this issue Jul 30, 2015 · 18 comments

Comments

@jawshooah
Copy link

The commit range stored in the TRAVIS_COMMIT_RANGE environment variable uses the triple-dot syntax r1...r2, which equates to "the set of commits that are reachable from either one of r1 or r2 but not from both".

This does not fit the description given in the documentation, which claims that TRAVIS_COMMIT_RANGE is "the range of commits that were included in the push or pull request".

In order to be accurate to that description, the commit range should use the double-dot syntax, r1..r2, which equates to "commits that are reachable from r2 excluding those that are reachable from r1". Otherwise, commits made to the target branch after the PR branch was created will be included in the commit range.

Refs Homebrew/homebrew-cask#12903

wking added a commit to wking/opencontainer-runtime-spec that referenced this issue Sep 12, 2015
Work around travis-ci/travis-ci#4596 until that is fixed upstream [1].
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 [2]).

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]: travis-ci/travis-ci#4596
[2]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link

wking commented Sep 12, 2015

I'm working around this with ${TRAVIS_COMMIT_RANGE/.../..} in my .travis.yml.

wking added a commit to wking/opencontainer-runtime-spec that referenced this issue Oct 6, 2015
Work around travis-ci/travis-ci#4596 until that is fixed upstream [1].
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 [2]).

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]: travis-ci/travis-ci#4596
[2]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
@jawshooah
Copy link
Author

Any comment on this, maintainers?

@spuder
Copy link

spuder commented Dec 17, 2015

+1

@LinusU
Copy link

LinusU commented May 25, 2016

@BanzaiMan any updates on this? ❤️ It would be nice to at least get the problem confirmed by someone working at Travis, maybe add the workaround to the documentation

fnichol added a commit to habitat-sh/habitat that referenced this issue Jun 4, 2016
See travis-ci/travis-ci#4596 for more details.

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
thesentinels pushed a commit to habitat-sh/habitat that referenced this issue Jun 4, 2016
See travis-ci/travis-ci#4596 for more details.

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>

Pull request: #653
Approved by: fnichol
jtimberman pushed a commit to habitat-sh/habitat that referenced this issue Jun 12, 2016
See travis-ci/travis-ci#4596 for more details.

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>

Pull request: #653
Approved by: fnichol
tommywo added a commit to tommywo/selenium that referenced this issue Sep 21, 2016
tommywo added a commit to stepstone-tech/selenium that referenced this issue Sep 27, 2016
joshcooper added a commit to joshcooper/puppet that referenced this issue Dec 2, 2016
TRAVIS_COMMIT_RANGE uses triple dots, which includes commits that are in
the base branch that are not in the PR. For example, if a commit is
pushed to master with an incorrect summary, then every PR created before
that commit will include the unrelated commit, causing the PR to fail
the summary check.

As suggested in travis-ci/travis-ci#4596 (comment),
replace triple dots with double dots.
joshcooper added a commit to joshcooper/puppet that referenced this issue Dec 7, 2016
TRAVIS_COMMIT_RANGE uses triple dots, which includes commits that are in
the base branch that are not in the PR. For example, if a commit is
pushed to master with an incorrect summary, then every PR created before
that commit will include the unrelated commit, causing the PR to fail
the summary check.

As suggested in travis-ci/travis-ci#4596 (comment),
replace triple dots with double dots.
wking added a commit to wking/opencontainer-runtime-spec that referenced this issue Jan 18, 2017
Work around travis-ci/travis-ci#4596 until that is fixed upstream [1].
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 [2]).

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]: travis-ci/travis-ci#4596
[2]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this issue Jan 19, 2017
Work around travis-ci/travis-ci#4596 until that is fixed upstream [1].
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 [2]).

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]: travis-ci/travis-ci#4596
[2]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/git-validation that referenced this issue Mar 21, 2017
Master builds only have a 'git clone ...' [1] so FETCH_HEAD isn't
defined and git-validation crashes [2].  This commit partially reverts
8a12a8f (main: default travis commit range is unreliable, 2017-03-21,
vbatts#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]: opencontainers/runc#1378 (comment)
[4]: travis-ci/travis-ci#4596
[5]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/git-validation that referenced this issue Mar 21, 2017
Master builds only have a 'git clone ...' [1] so FETCH_HEAD isn't
defined and git-validation crashes [2].  This commit partially reverts
8a12a8f (main: default travis commit range is unreliable, 2017-03-21,
vbatts#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]: opencontainers/runc#1378 (comment)
[4]: travis-ci/travis-ci#4596
[5]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
@s9gf4ult
Copy link

Here the quote from git diff --help

       git diff [--options] <commit>...<commit> [--] [<path>...]
           This form is to view the changes on the branch containing and up to the second <commit>, starting at a common ancestor of both <commit>. "git diff A...B" is equivalent to "git
           diff $(git-merge-base A B) B". You can omit any one of <commit>, which has the same effect as using HEAD instead.

and here from git logrevisions

       The .. (two-dot) Range Notation
           The ^r1 r2 set operation appears so often that there is a shorthand for it. When you have two commits r1 and r2 (named according to the syntax explained in SPECIFYING
           REVISIONS above), you can ask for commits that are reachable from r2 excluding those that are reachable from r1 by ^r1 r2 and it can be written as r1..r2.

       The ... (three dot) Symmetric Difference Notation
           A similar notation r1...r2 is called symmetric difference of r1 and r2 and is defined as r1 r2 --not $(git merge-base --all r1 r2). It is the set of commits that are reachable
           from either one of r1 (left side) or r2 (right side) but not from both.

So git diff and git log undertstands three dots differently. In fact three dots for git diff is like two dots for git log. The common syntax which works same is ^rev1 rev2

git log ^rev1 rev2
git diff ^rev1 rev2

will work with same commits set.

timabbott added a commit to zulip/zulip that referenced this issue Apr 25, 2017
This was causing a bunch of spurious failures.

Workaround for travis-ci/travis-ci#4596 ;
TRAVIS_COMMIT_RANGE is actually just totally wrong for this purpose.
brockwhittaker pushed a commit to brockwhittaker/zulip that referenced this issue Apr 26, 2017
This was causing a bunch of spurious failures.

Workaround for travis-ci/travis-ci#4596 ;
TRAVIS_COMMIT_RANGE is actually just totally wrong for this purpose.
igchor added a commit to igchor/libpmemobj-cpp that referenced this issue Mar 13, 2019
igchor added a commit to igchor/libpmemobj-cpp that referenced this issue Mar 13, 2019
igchor added a commit to igchor/pmdk that referenced this issue Mar 14, 2019
igchor added a commit to igchor/pmdk that referenced this issue Mar 14, 2019
in pull-or-rebuild-image.sh to contain only
commits from a PR.

Ref: travis-ci/travis-ci/issues/4596
igchor added a commit to igchor/pmdk that referenced this issue Mar 14, 2019
in pull-or-rebuild-image.sh to contain only
commits from a PR.

Ref: travis-ci/travis-ci/issues/4596
igchor added a commit to igchor/pmdk that referenced this issue Mar 14, 2019
in pull-or-rebuild-image.sh to contain only
commits from a PR.

Ref: travis-ci/travis-ci/issues/4596
krOoze added a commit to krOoze/Vulkan-ValidationLayers that referenced this issue Aug 15, 2019
mark-lunarg pushed a commit to KhronosGroup/Vulkan-ValidationLayers that referenced this issue Aug 16, 2019
marc-hb added a commit to marc-hb/sof-test that referenced this issue Apr 16, 2020
$TRAVIS_COMMIT_RANGE contains "..." instead of "..", see
travis-ci/travis-ci#4596

Fixes: faadb20 ("CI: travis: new shellcheck-gitrange.bash")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof-test that referenced this issue Apr 16, 2020
$TRAVIS_COMMIT_RANGE contains "..." instead of "..", see
travis-ci/travis-ci#4596

Fixes: faadb20 ("CI: travis: new shellcheck-gitrange.bash")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof-test that referenced this issue Apr 16, 2020
$TRAVIS_COMMIT_RANGE contains "..." instead of "..", see
travis-ci/travis-ci#4596

Fixes: faadb20 ("CI: travis: new shellcheck-gitrange.bash")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof-test that referenced this issue Apr 16, 2020
$TRAVIS_COMMIT_RANGE contains "..." instead of "..", see
travis-ci/travis-ci#4596

Fixes: faadb20 ("CI: travis: new shellcheck-gitrange.bash")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb added a commit to marc-hb/sof-test that referenced this issue Apr 16, 2020
$TRAVIS_COMMIT_RANGE contains "..." instead of "..", see
travis-ci/travis-ci#4596

Fixes: faadb20 ("CI: travis: new shellcheck-gitrange.bash")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-h38
Copy link

tl;dr: convert ${TRAVIS_COMMIT_RANGE} to double dots only for git log, not for git diff

The commit range stored in the TRAVIS_COMMIT_RANGE environment variable uses the triple-dot syntax r1...r2, which equates to "the set of commits that are reachable from either one of r1 or r2 but not from both".

That's true for git log but as explained by @s9gf4ult , the triple dots mean something different (and very useful) for git diff. In fact the triple dotted ${TRAVIS_COMMIT_RANGE} is the perfect argument for git diff --name-only.

@marc-h38
Copy link

So git diff and git log undertstands three dots differently. In fact three dots for git diff is like two dots for git log.

and vice-versa!

          *---*---PR pull request
         /
--*---base---*----upstream

     Double dots        |  Triple dots
       upstream..PR     |        upstream...PR
------------------------+--------------------------
diff   upstream  PR "V" |  base             PR "/" 
log    ^base     PR "/" | ^base  upstream   PR "V"

@pprindeville
Copy link

Sounds like there's an issue with git. Has a bug been filed against it?

@marc-h38
Copy link

Git user interface issues like this one are unfixable, imagine how many things this would break.

OK, one way to "fix" interface issues like this is to add new commands and leave the old ones as they are. For instance the new git switch helps you avoid the confusing and dangerous git checkout : https://www.infoq.com/news/2019/08/git-2-23-switch-restore/
So maybe there could be new git helps revisions syntaxes that don't use dots? I don't know.

For similar backward compatibility reasons it's way too late to change TRAVIS_COMMIT_RANGE. This issue should stay closed.

On the other hand Travis could/should have additional variables, for instance Gitlab has CI_COMMIT_REF_NAME, CI_COMMIT_SHA, CI_COMMIT_BRANCH and a good few others. Feel free to file a new Travis issue if none yet.

joshcooper added a commit to puppetlabs/puppet-agent that referenced this issue Mar 16, 2022
The TRAVIS_COMMIT_RANGE environment variable uses triple dots to describe the
range[1]. When that is used in `git log base...HEAD`, then it will include all
commits that are not in both A and B. That means we will include commits in the
base branch that are unrelated to the PR.

For now, do what puppet does to convert triple dots to double dots. In the
future, use GITHUB_BASE_REF to determine the range of commits.

[1] travis-ci/travis-ci#4596
joshcooper added a commit to puppetlabs/puppet-agent that referenced this issue Mar 16, 2022
The TRAVIS_COMMIT_RANGE environment variable uses triple dots to describe the
range[1]. When that is used in `git log base...HEAD`, then it will include all
commits that are not in both A and B. That means we will include commits in the
base branch that are unrelated to the PR.

For now, do what puppet does to convert triple dots to double dots. In the
future, use GITHUB_BASE_REF to determine the range of commits.

[1] travis-ci/travis-ci#4596
joshcooper added a commit to joshcooper/puppet-agent that referenced this issue Mar 28, 2022
The TRAVIS_COMMIT_RANGE environment variable uses triple dots to describe the
range[1]. When that is used in `git log base...HEAD`, then it will include all
commits that are not in both A and B. That means we will include commits in the
base branch that are unrelated to the PR.

For now, do what puppet does to convert triple dots to double dots. In the
future, use GITHUB_BASE_REF to determine the range of commits.

[1] travis-ci/travis-ci#4596

(cherry picked from commit f693074)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests