# [PATCH v1] travis-ci: fix "skip_branch_tip_with_tag()" string comparison 4 messages from 2017-09-21 to 2017-09-22. Participants: larsxschneider@gmail.com, Jonathan Nieder, Lars Schneider, Junio C Hamano. Thread: https://gitlist.dev/t/46805 ## larsxschneider@gmail.com, 2017-09-21 20:48 Subject: [PATCH v1] travis-ci: fix "skip_branch_tip_with_tag()" string comparison Message-ID: <20170921204830.66831-1-larsxschneider@gmail.com> URL: https://gitlist.dev/e/20170921204830.66831-1-larsxschneider%40gmail.com ``` From: Lars Schneider 09f5e97 ("travis-ci: skip a branch build if equal tag is present", 2017-09-17) introduced the "skip_branch_tip_with_tag" function with a broken string comparison. Fix it! Reported-by: SZEDER Gábor Signed-off-by: Lars Schneider --- Hi, previous discussion: https://public-inbox.org/git/3B175D35-5B1C-43CD-A7E9-85693335B10A@gmail.com/ Sorry that this trivial fix took so long. Cheers, Lars Notes: Base Commit: a81423d7cf (a81423d7cfdc57238783f05394dddd1064c99165) Diff on Web: https://github.com/larsxschneider/git/commit/6b532a42f0 Checkout: git fetch https://github.com/larsxschneider/git travisci/fix-skip-branch-v1 && git checkout 6b532a42f0 ci/lib-travisci.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index 9c4ae9bdd0..c3b46f4a7d 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () { # of a tag. if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) && - $TAG != $TRAVIS_BRANCH + [ "$TAG" != "$TRAVIS_BRANCH" ] then echo "Tip of $TRAVIS_BRANCH is exactly at $TAG" exit 0 -- 2.14.1 ``` ## Jonathan Nieder, 2017-09-21 21:28 Subject: Re: [PATCH v1] travis-ci: fix "skip_branch_tip_with_tag()" string comparison Message-ID: <20170921212848.GJ27425@aiede.mtv.corp.google.com> URL: https://gitlist.dev/e/20170921212848.GJ27425%40aiede.mtv.corp.google.com In-Reply-To: <20170921204830.66831-1-larsxschneider@gmail.com> ``` larsxschneider@gmail.com wrote: > 09f5e97 ("travis-ci: skip a branch build if equal tag is present", > 2017-09-17) introduced the "skip_branch_tip_with_tag" function with > a broken string comparison. Fix it! > > Reported-by: SZEDER Gábor > Signed-off-by: Lars Schneider > --- Thanks for the fix. 09f5e97 appears to be for the ls/travis-scriptify branch, which is already part of "next" (if it weren't, I'd suggest just squashing your patch into that commit). > --- a/ci/lib-travisci.sh > +++ b/ci/lib-travisci.sh > @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () { > # of a tag. > > if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) && > - $TAG != $TRAVIS_BRANCH > + [ "$TAG" != "$TRAVIS_BRANCH" ] Git style is to use 'test' instead of '[' for this. See https://public-inbox.org/git/2f3cdc85-f051-c0ae-b9db-fd13cac78aed@gmail.com/ for more on that subject. Could you squash in the following? Thanks, Jonathan diff --git i/ci/lib-travisci.sh w/ci/lib-travisci.sh index c3b46f4a7d..b3ed0a0dda 100755 --- i/ci/lib-travisci.sh +++ w/ci/lib-travisci.sh @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () { # of a tag. if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) && - [ "$TAG" != "$TRAVIS_BRANCH" ] + test "$TAG" != "$TRAVIS_BRANCH" then echo "Tip of $TRAVIS_BRANCH is exactly at $TAG" exit 0 ``` ## Lars Schneider, 2017-09-21 22:26 Subject: Re: [PATCH v1] travis-ci: fix "skip_branch_tip_with_tag()" string comparison Message-ID: <4FC081C4-355E-4E62-8CD8-89C299485182@gmail.com> URL: https://gitlist.dev/e/4FC081C4-355E-4E62-8CD8-89C299485182%40gmail.com In-Reply-To: <20170921212848.GJ27425@aiede.mtv.corp.google.com> ``` > On 21 Sep 2017, at 23:28, Jonathan Nieder wrote: > > larsxschneider@gmail.com wrote: > >> 09f5e97 ("travis-ci: skip a branch build if equal tag is present", >> 2017-09-17) introduced the "skip_branch_tip_with_tag" function with >> a broken string comparison. Fix it! >> >> Reported-by: SZEDER Gábor >> Signed-off-by: Lars Schneider >> --- > > Thanks for the fix. > > 09f5e97 appears to be for the ls/travis-scriptify branch, which is > already part of "next" (if it weren't, I'd suggest just squashing your > patch into that commit). > >> --- a/ci/lib-travisci.sh >> +++ b/ci/lib-travisci.sh >> @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () { >> # of a tag. >> >> if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) && >> - $TAG != $TRAVIS_BRANCH >> + [ "$TAG" != "$TRAVIS_BRANCH" ] > > Git style is to use 'test' instead of '[' for this. See > https://public-inbox.org/git/2f3cdc85-f051-c0ae-b9db-fd13cac78aed@gmail.com/ > for more on that subject. Oh, you're right! > Could you squash in the following? @Junio: Can you squash it when you apply the patch? Thank you, Lars > > Thanks, > Jonathan > > diff --git i/ci/lib-travisci.sh w/ci/lib-travisci.sh > index c3b46f4a7d..b3ed0a0dda 100755 > --- i/ci/lib-travisci.sh > +++ w/ci/lib-travisci.sh > @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () { > # of a tag. > > if TAG=$(git describe --exact-match "$TRAVIS_BRANCH" 2>/dev/null) && > - [ "$TAG" != "$TRAVIS_BRANCH" ] > + test "$TAG" != "$TRAVIS_BRANCH" > then > echo "Tip of $TRAVIS_BRANCH is exactly at $TAG" > exit 0 ``` ## Junio C Hamano, 2017-09-22 01:57 Subject: Re: [PATCH v1] travis-ci: fix "skip_branch_tip_with_tag()" string comparison Message-ID: URL: https://gitlist.dev/e/xmqqvakblckw.fsf%40gitster.mtv.corp.google.com In-Reply-To: <4FC081C4-355E-4E62-8CD8-89C299485182@gmail.com> ``` Lars Schneider writes: > Oh, you're right! > > >> Could you squash in the following? > > @Junio: Can you squash it when you apply the patch? I do not mind and I already did. The patches in the series this patch is fixing up were solely about splitting these scripts out of the YAML file as-is, and I think it was correct to carry these style differences over without adjusting them. But resulting scripts in ci/ are riddled with styleguide deviations, which may want to be cleaned up later. There also are some bash-isms marked with "#!env bash" in them, but I think they are OK because we know we are running them only at a very specific place and the need to make them portable is very small. Thanks. ```