{"thread":{"id":"46805","subject":"[PATCH v1] travis-ci: fix \"skip_branch_tip_with_tag()\" string comparison","startedAt":"2017-09-21T20:48:38Z","lastAt":"2017-09-22T01:57:42Z","messageCount":4,"participants":["larsxschneider@gmail.com","Jonathan Nieder","Lars Schneider","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"328591","messageId":"20170921204830.66831-1-larsxschneider@gmail.com","threadId":"46805","inReplyTo":null,"subject":"[PATCH v1] travis-ci: fix \"skip_branch_tip_with_tag()\" string comparison","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-09-21T20:48:30Z","receivedAt":"2017-09-21T20:48:38Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\n09f5e97 (\"travis-ci: skip a branch build if equal tag is present\",\n2017-09-17) introduced the \"skip_branch_tip_with_tag\" function with\na broken string comparison. Fix it!\n\nReported-by: SZEDER Gábor \nSigned-off-by: Lars Schneider \n---\n\nHi,\n\nprevious discussion:\nhttps://public-inbox.org/git/3B175D35-5B1C-43CD-A7E9-85693335B10A@gmail.com/\n\nSorry that this trivial fix took so long.\n\nCheers,\nLars\n\n\nNotes:\n Base Commit: a81423d7cf (a81423d7cfdc57238783f05394dddd1064c99165)\n Diff on Web: https://github.com/larsxschneider/git/commit/6b532a42f0\n Checkout: git fetch https://github.com/larsxschneider/git travisci/fix-skip-branch-v1 && git checkout 6b532a42f0\n\n ci/lib-travisci.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh\nindex 9c4ae9bdd0..c3b46f4a7d 100755\n--- a/ci/lib-travisci.sh\n+++ b/ci/lib-travisci.sh\n@@ -14,7 +14,7 @@ skip_branch_tip_with_tag () {\n \t# of a tag.\n\n \tif TAG=$(git describe --exact-match \"$TRAVIS_BRANCH\" 2>/dev/null) &&\n-\t\t$TAG != $TRAVIS_BRANCH\n+\t\t[ \"$TAG\" != \"$TRAVIS_BRANCH\" ]\n \tthen\n \t\techo \"Tip of $TRAVIS_BRANCH is exactly at $TAG\"\n \t\texit 0\n--\n2.14.1\n\n"},{"id":"328593","messageId":"20170921212848.GJ27425@aiede.mtv.corp.google.com","threadId":"46805","inReplyTo":"20170921204830.66831-1-larsxschneider@gmail.com","subject":"Re: [PATCH v1] travis-ci: fix \"skip_branch_tip_with_tag()\" string comparison","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-09-21T21:28:48Z","receivedAt":"2017-09-21T21:28:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"larsxschneider@gmail.com wrote:\n\n> 09f5e97 (\"travis-ci: skip a branch build if equal tag is present\",\n> 2017-09-17) introduced the \"skip_branch_tip_with_tag\" function with\n> a broken string comparison. Fix it!\n>\n> Reported-by: SZEDER Gábor \n> Signed-off-by: Lars Schneider \n> ---\n\nThanks for the fix.\n\n09f5e97 appears to be for the ls/travis-scriptify branch, which is\nalready part of \"next\" (if it weren't, I'd suggest just squashing your\npatch into that commit).\n\n> --- a/ci/lib-travisci.sh\n> +++ b/ci/lib-travisci.sh\n> @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () {\n> \t# of a tag.\n> \n> \tif TAG=$(git describe --exact-match \"$TRAVIS_BRANCH\" 2>/dev/null) &&\n> -\t\t$TAG != $TRAVIS_BRANCH\n> +\t\t[ \"$TAG\" != \"$TRAVIS_BRANCH\" ]\n\nGit style is to use 'test' instead of '[' for this. See\nhttps://public-inbox.org/git/2f3cdc85-f051-c0ae-b9db-fd13cac78aed@gmail.com/\nfor more on that subject.\n\nCould you squash in the following?\n\nThanks,\nJonathan\n\ndiff --git i/ci/lib-travisci.sh w/ci/lib-travisci.sh\nindex c3b46f4a7d..b3ed0a0dda 100755\n--- i/ci/lib-travisci.sh\n+++ w/ci/lib-travisci.sh\n@@ -14,7 +14,7 @@ skip_branch_tip_with_tag () {\n \t# of a tag.\n \n \tif TAG=$(git describe --exact-match \"$TRAVIS_BRANCH\" 2>/dev/null) &&\n-\t\t[ \"$TAG\" != \"$TRAVIS_BRANCH\" ]\n+\t\ttest \"$TAG\" != \"$TRAVIS_BRANCH\"\n \tthen\n \t\techo \"Tip of $TRAVIS_BRANCH is exactly at $TAG\"\n \t\texit 0\n"},{"id":"328595","messageId":"4FC081C4-355E-4E62-8CD8-89C299485182@gmail.com","threadId":"46805","inReplyTo":"20170921212848.GJ27425@aiede.mtv.corp.google.com","subject":"Re: [PATCH v1] travis-ci: fix \"skip_branch_tip_with_tag()\" string comparison","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-09-21T22:26:44Z","receivedAt":"2017-09-21T22:26:52Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 21 Sep 2017, at 23:28, Jonathan Nieder wrote:\n> \n> larsxschneider@gmail.com wrote:\n> \n>> 09f5e97 (\"travis-ci: skip a branch build if equal tag is present\",\n>> 2017-09-17) introduced the \"skip_branch_tip_with_tag\" function with\n>> a broken string comparison. Fix it!\n>> \n>> Reported-by: SZEDER Gábor \n>> Signed-off-by: Lars Schneider \n>> ---\n> \n> Thanks for the fix.\n> \n> 09f5e97 appears to be for the ls/travis-scriptify branch, which is\n> already part of \"next\" (if it weren't, I'd suggest just squashing your\n> patch into that commit).\n> \n>> --- a/ci/lib-travisci.sh\n>> +++ b/ci/lib-travisci.sh\n>> @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () {\n>> \t# of a tag.\n>> \n>> \tif TAG=$(git describe --exact-match \"$TRAVIS_BRANCH\" 2>/dev/null) &&\n>> -\t\t$TAG != $TRAVIS_BRANCH\n>> +\t\t[ \"$TAG\" != \"$TRAVIS_BRANCH\" ]\n> \n> Git style is to use 'test' instead of '[' for this. See\n> https://public-inbox.org/git/2f3cdc85-f051-c0ae-b9db-fd13cac78aed@gmail.com/\n> for more on that subject.\n\nOh, you're right!\n\n\n> Could you squash in the following?\n\n@Junio: Can you squash it when you apply the patch?\n\nThank you,\nLars\n\n\n> \n> Thanks,\n> Jonathan\n> \n> diff --git i/ci/lib-travisci.sh w/ci/lib-travisci.sh\n> index c3b46f4a7d..b3ed0a0dda 100755\n> --- i/ci/lib-travisci.sh\n> +++ w/ci/lib-travisci.sh\n> @@ -14,7 +14,7 @@ skip_branch_tip_with_tag () {\n> \t# of a tag.\n> \n> \tif TAG=$(git describe --exact-match \"$TRAVIS_BRANCH\" 2>/dev/null) &&\n> -\t\t[ \"$TAG\" != \"$TRAVIS_BRANCH\" ]\n> +\t\ttest \"$TAG\" != \"$TRAVIS_BRANCH\"\n> \tthen\n> \t\techo \"Tip of $TRAVIS_BRANCH is exactly at $TAG\"\n> \t\texit 0\n\n"},{"id":"328609","messageId":"xmqqvakblckw.fsf@gitster.mtv.corp.google.com","threadId":"46805","inReplyTo":"4FC081C4-355E-4E62-8CD8-89C299485182@gmail.com","subject":"Re: [PATCH v1] travis-ci: fix \"skip_branch_tip_with_tag()\" string comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-22T01:57:35Z","receivedAt":"2017-09-22T01:57:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n> Oh, you're right!\n>\n>\n>> Could you squash in the following?\n>\n> @Junio: Can you squash it when you apply the patch?\n\nI do not mind and I already did.\n\nThe patches in the series this patch is fixing up were solely about\nsplitting these scripts out of the YAML file as-is, and I think it\nwas correct to carry these style differences over without adjusting\nthem.\n\nBut resulting scripts in ci/ are riddled with styleguide deviations,\nwhich may want to be cleaned up later. There also are some\nbash-isms marked with \"#!env bash\" in them, but I think they are OK\nbecause we know we are running them only at a very specific place\nand the need to make them portable is very small.\n\nThanks.\n"}]}