{"thread":{"id":"57778","subject":"[PATCH] ci: make perforce installation optional in macOS","startedAt":"2022-04-21T22:55:32Z","lastAt":"2022-05-12T22:40:15Z","messageCount":26,"participants":["Carlo Marcelo Arenas Belón","Eric Sunshine","Junio C Hamano","Carlo Arenas","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454197","messageId":"20220421225515.6316-1-carenas@gmail.com","threadId":"57778","inReplyTo":null,"subject":"[PATCH] ci: make perforce installation optional in macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-21T22:55:15Z","receivedAt":"2022-04-21T22:55:32Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Using brew to install perforce has several documented[1,2,3,4] edge\ncases that make it fail periodically, so if problems were found while\ninstalling it, just continue without it.\n\nThis means that until the problem is solved all perforce tests will be\nskipped in macOS, but they are still most likely covered by the other\nunaffected runs and will be covered again once the issue solves itself.\n\n1 0eb3671ed96 (ci(osx): use new location of the `perforce` cask, 2019-10-23)\n2 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\n3 3831132ace6 (ci/install-depends: attempt to fix \"brew cask\" stuff, 2021-01-14)\n4 https://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/\n---\nThis is based on master and can merge upwards except with seen where it\nconflicts with the recently added ab/ci-osx-loosen-package-requirement\nwhich it is meant to replace.\n\nA successful sample run when merged with master available in :\n\n https://github.com/carenas/git/actions/runs/2204519374\n\n ci/install-dependencies.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\nwi\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb2..53da1afd9f3 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -43,7 +43,7 @@ macos-latest)\n \t\tgit -C \"$cask_repo\" pull --no-stat --ff-only &&\n \t\tbrew install --cask --no-quarantine perforce\n \t} ||\n-\tbrew install homebrew/cask/perforce\n+\tbrew install homebrew/cask/perforce || true\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454198","messageId":"CAPig+cQh7piGQwQwS+d8RRSDODQwmB2V73w__s4yX=09YLRpWg@mail.gmail.com","threadId":"57778","inReplyTo":"20220421225515.6316-1-carenas@gmail.com","subject":"Re: [PATCH] ci: make perforce installation optional in macOS","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-21T22:58:30Z","receivedAt":"2022-04-21T22:58:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 21, 2022 at 6:55 PM Carlo Marcelo Arenas Belón\n wrote:\n> Using brew to install perforce has several documented[1,2,3,4] edge\n> cases that make it fail periodically, so if problems were found while\n> installing it, just continue without it.\n>\n> This means that until the problem is solved all perforce tests will be\n> skipped in macOS, but they are still most likely covered by the other\n> unaffected runs and will be covered again once the issue solves itself.\n>\n> 1 0eb3671ed96 (ci(osx): use new location of the `perforce` cask, 2019-10-23)\n> 2 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\n> 3 3831132ace6 (ci/install-depends: attempt to fix \"brew cask\" stuff, 2021-01-14)\n> 4 https://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/\n> ---\n\nMissing sign-off.\n"},{"id":"454199","messageId":"CAPig+cT+KWZNYYCWEoc5Wk4Kp_aQw1m48Tks62CAe2EaYJnc+A@mail.gmail.com","threadId":"57778","inReplyTo":"20220421225515.6316-1-carenas@gmail.com","subject":"Re: [PATCH] ci: make perforce installation optional in macOS","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-21T23:05:43Z","receivedAt":"2022-04-21T23:05:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 21, 2022 at 6:55 PM Carlo Marcelo Arenas Belón\n wrote:\n> Using brew to install perforce has several documented[1,2,3,4] edge\n> cases that make it fail periodically, so if problems were found while\n> installing it, just continue without it.\n>\n> This means that until the problem is solved all perforce tests will be\n> skipped in macOS, but they are still most likely covered by the other\n> unaffected runs and will be covered again once the issue solves itself.\n>\n> 1 0eb3671ed96 (ci(osx): use new location of the `perforce` cask, 2019-10-23)\n> 2 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\n> 3 3831132ace6 (ci/install-depends: attempt to fix \"brew cask\" stuff, 2021-01-14)\n> 4 https://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/\n> ---\n> This is based on master and can merge upwards except with seen where it\n> conflicts with the recently added ab/ci-osx-loosen-package-requirement\n> which it is meant to replace.\n>\n> A successful sample run when merged with master available in :\n>\n> https://github.com/carenas/git/actions/runs/2204519374\n>\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> @@ -43,7 +43,7 @@ macos-latest)\n> - brew install homebrew/cask/perforce\n> + brew install homebrew/cask/perforce || true\n\nYou do get a vaguely alarming error message[1] with the solution\nimplemented by this patch:\n\n ci/install-dependencies.sh: line 81: type: p4d: not found\n\nHowever, that's an unrelated and existing minor mistake (outside of\nthe scope of this fix, probably). It should be using `command -v`\nrather than `type` in the couple conditionals at the bottom of the\nscript.\n\n[1]: https://github.com/carenas/git/runs/6119943232?check_suite_focus=true#step:3:116\n"},{"id":"454202","messageId":"20220422013911.7646-1-carenas@gmail.com","threadId":"57778","inReplyTo":"20220421225515.6316-1-carenas@gmail.com","subject":"[PATCH v2 0/2] ci: avoid perforce/brew issues affecting macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T01:39:09Z","receivedAt":"2022-04-22T01:39:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"This series is a simpler alternative to ab/ci-osx-loosen-package-requirement\nwhich while it doesn't address the underlying issue, avoids the CI failures\nin macOS as well.\n\nFor an example of a successful run with it merged with next see:\n\n https://github.com/carenas/git/actions/runs/2205005763\n\nCarlo Marcelo Arenas Belón (2):\n ci: make failure to find perforce more user friendly\n ci: make perforce installation optional in macOS\n\n ci/install-dependencies.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454203","messageId":"20220422013911.7646-2-carenas@gmail.com","threadId":"57778","inReplyTo":"20220422013911.7646-1-carenas@gmail.com","subject":"[PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T01:39:10Z","receivedAt":"2022-04-22T01:39:29Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In preparation for a future change that will make perforce installation\noptional in macOS, make sure that the check for it is done without\ntriggering scary looking errors and add a user friendly message instead.\n\nOnly one invocation of type is changed as that is what is only needed\nfor the expected future use case, and because `type` is recommended in\nthe CodingGuidelines, so changing that recommendation or a more complex\nchange has been specifically punted.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n ci/install-dependencies.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb2..6de20108775 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -78,12 +78,14 @@ linux-gcc-default)\n \t;;\n esac\n \n-if type p4d >/dev/null && type p4 >/dev/null\n+if command -v p4d >/dev/null && type p4 >/dev/null\n then\n \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n \tp4d -V | grep Rev.\n \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n \tp4 -V | grep Rev.\n+else\n+\techo \"WARNING: perforce wasn't installed, see above for clues why\"\n fi\n if type git-lfs >/dev/null\n then\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454204","messageId":"20220422013911.7646-3-carenas@gmail.com","threadId":"57778","inReplyTo":"20220422013911.7646-1-carenas@gmail.com","subject":"[PATCH v2 2/2] ci: make perforce installation optional in macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T01:39:11Z","receivedAt":"2022-04-22T01:39:32Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Using brew to install perforce has several documented[1,2,3,4] edge\ncases that make it fail periodically, so if problems were found while\ninstalling it, just continue without it.\n\nThis means that until the problem is solved all perforce tests will be\nskipped in macOS, but they are still most likely covered by the other\nunaffected runs and will be covered again once the issue solves itself.\n\n1 0eb3671ed96 (ci(osx): use new location of the `perforce` cask, 2019-10-23)\n2 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\n3 3831132ace6 (ci/install-depends: attempt to fix \"brew cask\" stuff, 2021-01-14)\n4 https://lore.kernel.org/git/cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com/\n\nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n ci/install-dependencies.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 6de20108775..aca01a414ab 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -43,7 +43,7 @@ macos-latest)\n \t\tgit -C \"$cask_repo\" pull --no-stat --ff-only &&\n \t\tbrew install --cask --no-quarantine perforce\n \t} ||\n-\tbrew install homebrew/cask/perforce\n+\tbrew install homebrew/cask/perforce || true\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454222","messageId":"xmqqo80td5wj.fsf@gitster.g","threadId":"57778","inReplyTo":"20220422013911.7646-2-carenas@gmail.com","subject":"Re: [PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T05:49:16Z","receivedAt":"2022-04-22T05:49:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón writes:\n\n> In preparation for a future change that will make perforce installation\n> optional in macOS, make sure that the check for it is done without\n> triggering scary looking errors and add a user friendly message instead.\n>\n> Only one invocation of type is changed as that is what is only needed\n> for the expected future use case, and because `type` is recommended in\n> the CodingGuidelines, so changing that recommendation or a more complex\n> change has been specifically punted.\n\nThis may be in the \"POSIX may say this but the real world may not\nwork that way\" territory. As far as I can tell, \"command -v\" [*1*]\nand \"type\" [*2*] both ought to give diagnostic messages to their\nstandard error stream, and they both should signal an error with\nnon-zero exit status. It may be that the shell implementation you\nhave tried had \"command -v\" that is less noisy than \"type\" when\ngiven a command that is not installed, but I wonder if the next\nshell implementation you find has \"command -v\" that is as noisy and\nscary as \"type\", in which case this patch amounts to a no-op.\n\nI wonder if \"type p4d >/dev/null 2>/dev/null\" (or \"command -v\" with\nthe same) is a more futureproof fix.\n\n\n[References]\n\n*1* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html\n*2* https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html\n\n> Helped-by: Eric Sunshine \n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> ci/install-dependencies.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index dbcebad2fb2..6de20108775 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -78,12 +78,14 @@ linux-gcc-default)\n> \t;;\n> esac\n> \n> -if type p4d >/dev/null && type p4 >/dev/null\n> +if command -v p4d >/dev/null && type p4 >/dev/null\n> then\n> \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> \tp4d -V | grep Rev.\n> \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> \tp4 -V | grep Rev.\n> +else\n> +\techo \"WARNING: perforce wasn't installed, see above for clues why\"\n> fi\n> if type git-lfs >/dev/null\n> then\n"},{"id":"454310","messageId":"xmqq5yn069lq.fsf@gitster.g","threadId":"57778","inReplyTo":"xmqqo80td5wj.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T22:23:29Z","receivedAt":"2022-04-22T22:56:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This may be in the \"POSIX may say this but the real world may not\n> work that way\" territory. As far as I can tell, \"command -v\" [*1*]\n> and \"type\" [*2*] both ought to give diagnostic messages to their\n> standard error stream, and they both should signal an error with\n> non-zero exit status. It may be that the shell implementation you\n> have tried had \"command -v\" that is less noisy than \"type\" when\n> given a command that is not installed, but I wonder if the next\n> shell implementation you find has \"command -v\" that is as noisy and\n> scary as \"type\", in which case this patch amounts to a no-op.\n>\n> I wonder if \"type p4d >/dev/null 2>/dev/null\" (or \"command -v\" with\n> the same) is a more futureproof fix.\n\nSo, how about replacing it with something like this?\n\n----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----\nFrom: Carlo Marcelo Arenas Belón \n\nIn preparation for a future change that will make perforce installation\noptional in macOS, make sure that the check for it is done without\ntriggering scary looking errors and add a user friendly message instead.\n\nAll other existing uses of 'type ' in our shell scripts that\ncheck the availability of a command send both standard output\nand error stream to /dev/null to squelch \" not found\" diagnostic\noutput, but this script left the standard error stream shown.\n\nRedirect it just like everybody else to squelch this error message that\nwe fully expect to see.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Junio C Hamano \n---\n ci/install-dependencies.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb..e598dc28df 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -78,14 +78,16 @@ linux-gcc-default)\n \t;;\n esac\n \n-if type p4d >/dev/null && type p4 >/dev/null\n+if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n \tp4d -V | grep Rev.\n \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n \tp4 -V | grep Rev.\n+else\n+\techo \"WARNING: perforce wasn't installed, see above for clues why\"\n fi\n-if type git-lfs >/dev/null\n+if type git-lfs >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n \tgit-lfs version\n-- \n2.36.0-184-gca3de164ba\n\n"},{"id":"454314","messageId":"CAPUEspjGX4L-4T-93a2729SkB92TP500JYmEbKyrRFbv7moTXw@mail.gmail.com","threadId":"57778","inReplyTo":"xmqq5yn069lq.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T23:13:15Z","receivedAt":"2022-04-22T23:30:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Apr 22, 2022 at 3:23 PM Junio C Hamano wrote:\n>\n> So, how about replacing it with something like this?\n\nAgree 100%.\n\nIt would theoretically make the issue raised by Ævar of not knowing\nwhen perforce was skipped slightly worse\nbut getting that fixed would seem like something that could be done in\na follow up.\n\nI have to also admit, with all the on the fly changes to these same\nfiles, it might be wiser to wait until later anyway.\n\nCarlo\n"},{"id":"454317","messageId":"xmqqo80s4qn0.fsf@gitster.g","threadId":"57778","inReplyTo":"CAPUEspjGX4L-4T-93a2729SkB92TP500JYmEbKyrRFbv7moTXw@mail.gmail.com","subject":"Re: [PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T23:58:27Z","receivedAt":"2022-04-22T23:58:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Arenas writes:\n\n> On Fri, Apr 22, 2022 at 3:23 PM Junio C Hamano wrote:\n>>\n>> So, how about replacing it with something like this?\n>\n> Agree 100%.\n>\n> It would theoretically make the issue raised by Ævar of not knowing\n> when perforce was skipped slightly worse\n> but getting that fixed would seem like something that could be done in\n> a follow up.\n>\n> I have to also admit, with all the on the fly changes to these same\n> files, it might be wiser to wait until later anyway.\n\nActually, I was thinking about taking these two (possibly with\nÆvar's \"download with https://, we are in 21st century\" change to\nmake them 3-patch series) and fast-tracking.\n\nThe homebrew specific \"packagers can take a bit of time to catch up\nand we may see hash mismatch\" and other tweaks Ævar has can come on\ntop once we see the basic \"any parts of tests can fail and the rest\ncan still be tested, why does it have to be world-stopping if we\nfail to install p4?\" working.\n\nThanks.\n"},{"id":"454318","messageId":"CAPUEspjJVQZb1q8Xk7E1nKT3PGmMcc4SJ6M=ejHCjOUVTVV51Q@mail.gmail.com","threadId":"57778","inReplyTo":"xmqqo80s4qn0.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] ci: make failure to find perforce more user friendly","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T00:37:17Z","receivedAt":"2022-04-23T00:37:34Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Apr 22, 2022 at 4:58 PM Junio C Hamano wrote:\n>\n> Carlo Arenas writes:\n>\n> > I have to also admit, with all the on the fly changes to these same\n> > files, it might be wiser to wait until later anyway.\n>\n> Actually, I was thinking about taking these two (possibly with\n> Ævar's \"download with https://, we are in 21st century\" change to\n> make them 3-patch series) and fast-tracking.\n\nSounds like a good plan to me, and unlikely to introduce much pain to\nall other on the fly changes, but I think it is worth mentioning that\nsome kind soul did fix the mismatch in homebrew already, so there is\nnot really an urgent need for fast-tracking right now either.\n\nCarlo\n"},{"id":"454334","messageId":"20220423142559.32507-1-carenas@gmail.com","threadId":"57778","inReplyTo":"20220422013911.7646-1-carenas@gmail.com","subject":"[PATCH v3 0/4] ci: avoid perforce/brew issues affecting macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T14:25:55Z","receivedAt":"2022-04-23T14:26:39Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"This series is an alternative to ab/ci-osx-loosen-package-requirement\nand includes the parts proposed on it in v2 that didn't conflict.\n\nThe first patch is just a slightly modified version from the one\nproposed by Junio and that adds also a message if git-lfs wasn't found\njust like we do now for perforce while making those messages go to stderr.\n\nThe second patch replaces the original proposal with a more complicated\none that removes the use of brew as suggested by Ævar, it is also a\nlittle bigger than it really needs to be just to make sure it won't\nconflict with on the fly changes, and for the same reason uses some\nhardcoded values that would be worth fixing after.\n\nThe third patch is only needed to avoid regressions with Azure Pipelines\nwhich might be deprecated as proposed in other on the fly changes, so it\nmight not be needed in seen, but is still needed if this progresses\nfurther first. It was made independent so it could be easy to revert if\nAzure support is going away just like TravisCI did.\n\nThe last patch was taken from Ævar's v2 and is the only one that will\nrequire changes in other core on the fly but the conflict resolution is\ntrivial, and more importantly doesn't conflict when merged to seen in\nthe current spot this branch uses.\n\nFor an example of a \"successful\" run with it merged on top of seen\n(with the previous iteration reverted) see:\n\n https://github.com/carenas/git/runs/6140236148\n\nNote that perforce no longer fails because someone fixed the issue\nwith their Cask but still most of the new code is excercised correctly.\n\nThis introduces a problem identified by Ævar of making the CI runs less\nconsistent, as they might be skipping some of the perforce integrations\nif installing that fails again, but the likelyhood of that failing has\nbeen reduced by removing the inherent brew flakyness, and so improving\nthe UI so that it will be more visible when that happens is probably\nsomething that can be addressed later, once all other CI related changes\nare settled, or we could reintroduce the hard dependency on installing\nperforce for macOS (just like it is done in Linux)\n\nCarlo Marcelo Arenas Belón (3):\n ci: make failure to find perforce more user friendly\n ci: avoid brew for installing perforce\n ci: reintroduce prevention from perforce being quarantined in macOS\n\nÆvar Arnfjörð Bjarmason (1):\n CI: use https, not http to download binaries from perforce.com\n\n ci/install-dependencies.sh | 26 ++++++++++++++++----------\n 1 file changed, 16 insertions(+), 10 deletions(-)\n\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454335","messageId":"20220423142559.32507-2-carenas@gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-1-carenas@gmail.com","subject":"[PATCH v3 1/4] ci: make failure to find perforce more user friendly","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T14:25:56Z","receivedAt":"2022-04-23T14:26:41Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"In preparation for a future change that will make perforce installation\noptional in macOS, make sure that the check for it is done without\ntriggering scary looking errors and add a user friendly message instead.\n\nAll other existing uses of 'type ' in our shell scripts that\ncheck the availability of a command send both standard output\nand error stream to /dev/null to squelch \" not found\" diagnostic\noutput, but this script left the standard error stream shown.\n\nRedirect it just like everybody else to squelch this error message that\nwe fully expect to see.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n ci/install-dependencies.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb2..41e9290fbdd 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -78,15 +78,19 @@ linux-gcc-default)\n \t;;\n esac\n \n-if type p4d >/dev/null && type p4 >/dev/null\n+if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n \tp4d -V | grep Rev.\n \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n \tp4 -V | grep Rev.\n+else\n+\techo \"WARNING: perforce wasn't installed, see above for clues why\" >2\n fi\n-if type git-lfs >/dev/null\n+if type git-lfs >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n \tgit-lfs version\n+else\n+\techo \"WARNING: git-lfs wasn't installed, see above for clues why\" >2\n fi\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454336","messageId":"20220423142559.32507-4-carenas@gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-1-carenas@gmail.com","subject":"[PATCH v3 3/4] ci: reintroduce prevention from perforce being quarantined in macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T14:25:58Z","receivedAt":"2022-04-23T14:26:43Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\nintroduces this prevention for brew, but brew has been removed in a\nprevious commit, so reintroduce an equivalent option to avoid a possible\nregression.\n\nThis doesn't affect github actions (as configure now) and is therefore\ndone silently to avoid any possible scary irrelevant messages.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n ci/install-dependencies.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 9da03350d09..46a23a33dbd 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -41,7 +41,8 @@ macos-latest)\n \t(\n \t\tcd $HOME/bin\n \t\twget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n-\t\ttar -xf helix-core-server.tgz\n+\t\ttar -xf helix-core-server.tgz &&\n+\t\tsudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true\n \t)\n \tPATH=\"$PATH:${HOME}/bin\"\n \texport PATH\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454337","messageId":"20220423142559.32507-5-carenas@gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-1-carenas@gmail.com","subject":"[PATCH v3 4/4] CI: use https, not http to download binaries from perforce.com","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T14:25:59Z","receivedAt":"2022-04-23T14:26:44Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"From: Ævar Arnfjörð Bjarmason \n\nSince 522354d70f4 (Add Travis CI support, 2015-11-27) the CI has used\nhttp://filehost.perforce.com/perforce/ to download binaries from\nfilehost.perforce.com, they were then moved to this script in\n657343a602e (travis-ci: move Travis CI code into dedicated scripts,\n2017-09-10).\n\nLet's use https instead for good measure. I don't think we need to\nworry about the DNS or network between the GitHub CI and perforce.com\nbeing MitM'd, but using https gives us extra validation of the payload\nat least, and is one less thing to worry about when checking where\nelse we rely on non-TLS'd http connections.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n ci/install-dependencies.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 46a23a33dbd..a2d3578f4e3 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -5,7 +5,7 @@\n \n . ${0%/*}/lib.sh\n \n-P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION\n+P4WHENCE=https://filehost.perforce.com/perforce/r$LINUX_P4_VERSION\n LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION\n UBUNTU_COMMON_PKGS=\"make libssl-dev libcurl4-openssl-dev libexpat-dev\n tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454338","messageId":"20220423142559.32507-3-carenas@gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-1-carenas@gmail.com","subject":"[PATCH v3 2/4] ci: avoid brew for installing perforce","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-23T14:25:57Z","receivedAt":"2022-04-23T14:27:42Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Perfoce's cask in brew is meant[1] to be used only by humans, so replace\nits use from the CI with a scripted binary download which is less likely\nto fail, as it is done in Linux.\n\nKept the logic together so it will be less likely to break when moved\naround as on the fly code changes in this area are settled, at which\npoint it will also feasable to ammend it to avoid some of the hardcoded\nvalues by using similar variables to the ones Linux does.\n\nIn that same line, a POSIX sh syntax is used instead of the similar one\nused in Linux in preparation for an unrelated future change that might\nchange the shell currently configured for it.\n\nThis change reintroduces the risk that the installed binaries might not\nwork because of being quarantined that was fixed with 5ed9fc3fc86 (ci:\nprevent `perforce` from being quarantined, 2020-02-27) but fixing that\nnow was also punted for simplicity and since the affected cloud provider\nis scheduled to be retired with an on the fly change, but should be\naddressed if that other change is not integrated further.\n\nThe discussion on the need to keep 2 radically different versions of\nthe binaries to be tested with Linux vs macOS or how to upgrade to\nnewer versions now that brew won't do that automatically for us has\nbeen punted for now as well. On that line the now obsolete comment\nabout it in lib.sh was originally being updated by this change but\ncreated conflicts as it is moved around by other on the fly changes,\nso will be addressed independently as well.\n\n[1] https://github.com/Homebrew/homebrew-cask/pull/122347#discussion_r856026584\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n ci/install-dependencies.sh | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 41e9290fbdd..9da03350d09 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,13 +37,14 @@ macos-latest)\n \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n \tbrew install $BREW_INSTALL_PACKAGES\n \tbrew link --force gettext\n-\tbrew install --cask --no-quarantine perforce || {\n-\t\t# Update the definitions and try again\n-\t\tcask_repo=\"$(brew --repository)\"/Library/Taps/homebrew/homebrew-cask &&\n-\t\tgit -C \"$cask_repo\" pull --no-stat --ff-only &&\n-\t\tbrew install --cask --no-quarantine perforce\n-\t} ||\n-\tbrew install homebrew/cask/perforce\n+\tmkdir -p $HOME/bin\n+\t(\n+\t\tcd $HOME/bin\n+\t\twget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n+\t\ttar -xf helix-core-server.tgz\n+\t)\n+\tPATH=\"$PATH:${HOME}/bin\"\n+\texport PATH\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.266.g59f845bde02\n\n"},{"id":"454355","messageId":"CAPig+cSex1y=dFAfVBo+PuuwyF3GF4OJzK65ykSxVec3qQRhPQ@mail.gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-3-carenas@gmail.com","subject":"Re: [PATCH v3 2/4] ci: avoid brew for installing perforce","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-24T06:43:16Z","receivedAt":"2022-04-24T06:43:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 23, 2022 at 10:26 AM Carlo Marcelo Arenas Belón\n wrote:\n> Perfoce's cask in brew is meant[1] to be used only by humans, so replace\n> its use from the CI with a scripted binary download which is less likely\n> to fail, as it is done in Linux.\n\ns/Perfoce/Perforce/\n\n> Kept the logic together so it will be less likely to break when moved\n> around as on the fly code changes in this area are settled, at which\n> point it will also feasable to ammend it to avoid some of the hardcoded\n> values by using similar variables to the ones Linux does.\n\ns/Kept/Keep/\ns/also/also be/\ns/feasable/feasible/\ns/ammend/amend/\n\n> In that same line, a POSIX sh syntax is used instead of the similar one\n> used in Linux in preparation for an unrelated future change that might\n> change the shell currently configured for it.\n>\n> This change reintroduces the risk that the installed binaries might not\n> work because of being quarantined that was fixed with 5ed9fc3fc86 (ci:\n> prevent `perforce` from being quarantined, 2020-02-27) but fixing that\n> now was also punted for simplicity and since the affected cloud provider\n> is scheduled to be retired with an on the fly change, but should be\n> addressed if that other change is not integrated further.\n>\n> The discussion on the need to keep 2 radically different versions of\n> the binaries to be tested with Linux vs macOS or how to upgrade to\n> newer versions now that brew won't do that automatically for us has\n> been punted for now as well. On that line the now obsolete comment\n> about it in lib.sh was originally being updated by this change but\n> created conflicts as it is moved around by other on the fly changes,\n> so will be addressed independently as well.\n>\n> [1] https://github.com/Homebrew/homebrew-cask/pull/122347#discussion_r856026584\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón \n"},{"id":"454357","messageId":"CAPig+cTiNr2E56g-nUMYxK5Dj1YoQJPZUL7fW-iyA2R5DyursA@mail.gmail.com","threadId":"57778","inReplyTo":"20220423142559.32507-4-carenas@gmail.com","subject":"Re: [PATCH v3 3/4] ci: reintroduce prevention from perforce being quarantined in macOS","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-24T06:47:37Z","receivedAt":"2022-04-24T06:47:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 23, 2022 at 10:26 AM Carlo Marcelo Arenas Belón\n wrote:\n> 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\n> introduces this prevention for brew, but brew has been removed in a\n> previous commit, so reintroduce an equivalent option to avoid a possible\n> regression.\n>\n> This doesn't affect github actions (as configure now) and is therefore\n> done silently to avoid any possible scary irrelevant messages.\n\ns/configure/configured/\n\n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> @@ -41,7 +41,8 @@ macos-latest)\n> (\n> cd $HOME/bin\n> wget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n> - tar -xf helix-core-server.tgz\n> + tar -xf helix-core-server.tgz &&\n> + sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true\n> )\n> PATH=\"$PATH:${HOME}/bin\"\n> export PATH\n\nI don't see the point of the `|| true` since the code doesn't care\nabout the eventual outcome of the &&-chain. (The &&-chain itself makes\nsense, though, to avoid executing commands pointlessly if an earlier\nstep failed.) May not be worth a reroll, though.\n"},{"id":"454417","messageId":"xmqq35i0tzow.fsf@gitster.g","threadId":"57778","inReplyTo":"20220423142559.32507-2-carenas@gmail.com","subject":"Re: [PATCH v3 1/4] ci: make failure to find perforce more user friendly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-26T01:12:47Z","receivedAt":"2022-04-26T01:12:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón writes:\n\n> -if type p4d >/dev/null && type p4 >/dev/null\n> +if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n> then\n> \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n> \tp4d -V | grep Rev.\n> \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n> \tp4 -V | grep Rev.\n> +else\n> +\techo \"WARNING: perforce wasn't installed, see above for clues why\" >2\n> fi\n> -if type git-lfs >/dev/null\n> +if type git-lfs >/dev/null 2>&1\n> then\n> \techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n> \tgit-lfs version\n> +else\n> +\techo \"WARNING: git-lfs wasn't installed, see above for clues why\" >2\n> fi\n\nNO! Why do we want to create a file whose name is \"2\" here?\n\nGood that I caught them before I merged them to 'next'.\n\nWill locally amend and see if it fixes CI failure in 'seen'\n\nThanks.\n\n"},{"id":"454445","messageId":"nycvar.QRO.7.76.6.2204261747530.355@tvgsbejvaqbjf.bet","threadId":"57778","inReplyTo":"20220423142559.32507-3-carenas@gmail.com","subject":"Re: [PATCH v3 2/4] ci: avoid brew for installing perforce","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-04-26T15:55:25Z","receivedAt":"2022-04-26T15:55:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Sat, 23 Apr 2022, Carlo Marcelo Arenas Belón wrote:\n\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 41e9290fbdd..9da03350d09 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -37,13 +37,14 @@ macos-latest)\n> \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n> \tbrew install $BREW_INSTALL_PACKAGES\n> \tbrew link --force gettext\n> -\tbrew install --cask --no-quarantine perforce || {\n> -\t\t# Update the definitions and try again\n> -\t\tcask_repo=\"$(brew --repository)\"/Library/Taps/homebrew/homebrew-cask &&\n> -\t\tgit -C \"$cask_repo\" pull --no-stat --ff-only &&\n> -\t\tbrew install --cask --no-quarantine perforce\n> -\t} ||\n> -\tbrew install homebrew/cask/perforce\n> +\tmkdir -p $HOME/bin\n> +\t(\n> +\t\tcd $HOME/bin\n> +\t\twget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n\nI vaguely recall that Gábor Szeder attempted something similar, but I\n_think_ that ultimately there were too many moving parts in that URL that\nwe did not want to hardcode.\n\nFor example, when opening a PR against `maint` or an even older topic\nbranch, we are stuck with the exact code in that revision that obtains\nthe perforce package. But since that server is outside our control, it can\nvery well evolve to a state that is not amenable to hard-coding such a\nURL.\n\nWhile I don't think that we can solve this fully, I would prefer to keep\nthe existing `brew install` calls but fall back to downloading from a\nhard-coded URL. That still would have the shortcoming of hard-coded\n`r21.2` and `1015`, but it should fail less often than the proposed patch.\n\nFor the record, the recent problems stem from the fact that the package\nwas cached on GitHub's build agents (I guess to avoid many identical\ndownloads), and the cached package did not match what was recorded in the\nupdated package definition. Which means that those `brew` errors are only\ntransient, until a new VM image is built that caches the then-current\nperforce package.\n\nCiao,\nDscho\n\n> +\t\ttar -xf helix-core-server.tgz\n> +\t)\n> +\tPATH=\"$PATH:${HOME}/bin\"\n> +\texport PATH\n>\n> \tif test -n \"$CC_PACKAGE\"\n> \tthen\n> --\n> 2.36.0.266.g59f845bde02\n>\n>\n"},{"id":"454456","messageId":"CAPUEspgBNWPtiriYTVWdnJtdy6jZBBov3zTnwZZGA8gXwMiooA@mail.gmail.com","threadId":"57778","inReplyTo":"nycvar.QRO.7.76.6.2204261747530.355@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v3 2/4] ci: avoid brew for installing perforce","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-26T17:07:18Z","receivedAt":"2022-04-26T17:11:41Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Apr 26, 2022 at 8:55 AM Johannes Schindelin\n wrote:\n>\n> On Sat, 23 Apr 2022, Carlo Marcelo Arenas Belón wrote:\n>\n> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> > index 41e9290fbdd..9da03350d09 100755\n> > --- a/ci/install-dependencies.sh\n> > +++ b/ci/install-dependencies.sh\n> > @@ -37,13 +37,14 @@ macos-latest)\n> > test -z \"$BREW_INSTALL_PACKAGES\" ||\n> > brew install $BREW_INSTALL_PACKAGES\n> > brew link --force gettext\n> > - brew install --cask --no-quarantine perforce || {\n> > - # Update the definitions and try again\n> > - cask_repo=\"$(brew --repository)\"/Library/Taps/homebrew/homebrew-cask &&\n> > - git -C \"$cask_repo\" pull --no-stat --ff-only &&\n> > - brew install --cask --no-quarantine perforce\n> > - } ||\n> > - brew install homebrew/cask/perforce\n> > + mkdir -p $HOME/bin\n> > + (\n> > + cd $HOME/bin\n> > + wget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n>\n> I vaguely recall that Gábor Szeder attempted something similar, but I\n> _think_ that ultimately there were too many moving parts in that URL that\n> we did not want to hardcode.\n\nI can see that, and indeed that is also why those series still keep\nperforce installation as optional so it wouldn't block CI when it does.\n\nBTW most of the reasons why those values are hardcoded here\nis just to avoid conflicts with on the fly changes, and making it smarter\nand avoiding the hardcoding would be done as a prerequisite to making\nit mandatory again (if that is what is preferred)\n\n> While I don't think that we can solve this fully, I would prefer to keep\n> the existing `brew install` calls but fall back to downloading from a\n> hard-coded URL.\n\nConsidering that the expectation from brew maintainers (as documented[1])\nis that those mismatches would happen and should be solved manually\nand that perforce releases just do inplace binary pushes I still stand by my\nsuggestion to avoid brew, but it might be useful as a fallback of the wget\napproach if that helps with your concerns above.\n\n> For the record, the recent problems stem from the fact that the package\n> was cached on GitHub's build agents (I guess to avoid many identical\n> downloads), and the cached package did not match what was recorded in the\n> updated package definition. Which means that those `brew` errors are only\n> transient, until a new VM image is built that caches the then-current\n> perforce package.\n\nFWIW as I updated the brew cask[2], I realized that livecheck pointed to an even\nnewer release, so if we are adding brew back it might be worth adding\nyet another\nworkaround to delete the cached file and try without a SHA as Ævar suggested.\n\nCarlo\n\n[1] https://docs.brew.sh/Common-Issues#cask---checksum-does-not-match\n[2] https://github.com/Homebrew/homebrew-cask/pull/122347\n"},{"id":"455228","messageId":"20220512223940.238367-2-gitster@pobox.com","threadId":"57778","inReplyTo":"20220512223940.238367-1-gitster@pobox.com","subject":"[PATCH v4 1/4] ci: make failure to find perforce more user friendly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T22:39:37Z","receivedAt":"2022-05-12T22:39:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Carlo Marcelo Arenas Belón \n\nIn preparation for a future change that will make perforce installation\noptional in macOS, make sure that the check for it is done without\ntriggering scary looking errors and add a user friendly message instead.\n\nAll other existing uses of 'type ' in our shell scripts that\ncheck the availability of a command send both standard output\nand error stream to /dev/null to squelch \" not found\" diagnostic\noutput, but this script left the standard error stream shown.\n\nRedirect it just like everybody else to squelch this error message that\nwe fully expect to see.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Junio C Hamano \n---\n ci/install-dependencies.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb..fa620f8fcd 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -78,15 +78,19 @@ linux-gcc-default)\n \t;;\n esac\n \n-if type p4d >/dev/null && type p4 >/dev/null\n+if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Perforce Server Version$(tput sgr0)\"\n \tp4d -V | grep Rev.\n \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n \tp4 -V | grep Rev.\n+else\n+\techo >&2 \"WARNING: perforce wasn't installed, see above for clues why\"\n fi\n-if type git-lfs >/dev/null\n+if type git-lfs >/dev/null 2>&1\n then\n \techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n \tgit-lfs version\n+else\n+\techo >&2 \"WARNING: git-lfs wasn't installed, see above for clues why\"\n fi\n-- \n2.36.1-338-g1c7f76a54c\n\n"},{"id":"455229","messageId":"20220512223940.238367-1-gitster@pobox.com","threadId":"57778","inReplyTo":"20220423142559.32507-1-carenas@gmail.com","subject":"[PATCH v4 0/4] ci: avoid perforce/brew issues affecting macOS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T22:39:36Z","receivedAt":"2022-05-12T22:39:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We see macOS CI job failures from time to time when the version of\np4 brew expects is slightly older than what is offered at Perforce's\ndownload site. Instead of failing to run the tests altogether, just\nskip \"git-p4\" tests, like we do for tests that depends on any other\noptional packages, when this happens.\n\nThis is essentially unchanged from the v3 posted by Carlo on April\n23rd, with a few fix-ups that have been cooking on top squashed in.\n\nThe changes from the previous round (i.e. the \"fix-ups\" that have\nbeen floating on top, which got squashed in) are:\n\n * The first step has a fix to a couple of wrong redirections that\n created a junk file whose name is \"2\", instead of sending the\n messages to the standard error.\n\n * The last step makes two places that we download Perforce packages\n to use https://cdist2.perforce.com/ (earlier one of the places\n used that, the other https://filehost.perforce.com/).\n\nI am sending this out as a final \"complain now, or this will go to\n'next' soonish\" warning. The \"What's cooking\" report is getting\ncrowded with too many topics marked as \"Expecting a reroll\", and I'm\ntrying to do easier ones myself to see how much reduction we can\nmake.\n\nCarlo Marcelo Arenas Belón (3):\n ci: make failure to find perforce more user friendly\n ci: avoid brew for installing perforce\n ci: reintroduce prevention from perforce being quarantined in macOS\n\nÆvar Arnfjörð Bjarmason (1):\n ci: use https, not http to download binaries from perforce.com\n\n ci/install-dependencies.sh | 26 ++++++++++++++++----------\n 1 file changed, 16 insertions(+), 10 deletions(-)\n\n\n1: 5730a749bc ! 1: 024459486c ci: make failure to find perforce more user friendly\n @@ ci/install-dependencies.sh: linux-gcc-default)\n \techo \"$(tput setaf 6)Perforce Client Version$(tput sgr0)\"\n \tp4 -V | grep Rev.\n +else\n -+\techo \"WARNING: perforce wasn't installed, see above for clues why\" >2\n ++\techo >&2 \"WARNING: perforce wasn't installed, see above for clues why\"\n fi\n -if type git-lfs >/dev/null\n +if type git-lfs >/dev/null 2>&1\n @@ ci/install-dependencies.sh: linux-gcc-default)\n \techo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\"\n \tgit-lfs version\n +else\n -+\techo \"WARNING: git-lfs wasn't installed, see above for clues why\" >2\n ++\techo >&2 \"WARNING: git-lfs wasn't installed, see above for clues why\"\n fi\n2: 8cd96645ae = 2: 8717dbe8d9 ci: avoid brew for installing perforce\n3: 1f06d0ba06 = 3: 6a4f085d63 ci: reintroduce prevention from perforce being quarantined in macOS\n4: 6bf267b995 ! 4: 5be72d9150 ci: use https, not http to download binaries from perforce.com\n @@ Commit message\n at least, and is one less thing to worry about when checking where\n else we rely on non-TLS'd http connections.\n \n + Also, use the same download site at perforce.com for Linux and macOS\n + tarballs for consistency.\n +\n Signed-off-by: Ævar Arnfjörð Bjarmason \n + Signed-off-by: Carlo Marcelo Arenas Belón \n Signed-off-by: Junio C Hamano \n \n ## ci/install-dependencies.sh ##\n @@ ci/install-dependencies.sh\n . ${0%/*}/lib.sh\n \n -P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION\n -+P4WHENCE=https://filehost.perforce.com/perforce/r$LINUX_P4_VERSION\n ++P4WHENCE=https://cdist2.perforce.com/perforce/r$LINUX_P4_VERSION\n LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION\n UBUNTU_COMMON_PKGS=\"make libssl-dev libcurl4-openssl-dev libexpat-dev\n tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl\n\n\n\n-- \n2.36.1-338-g1c7f76a54c\n\n"},{"id":"455230","messageId":"20220512223940.238367-3-gitster@pobox.com","threadId":"57778","inReplyTo":"20220512223940.238367-1-gitster@pobox.com","subject":"[PATCH v4 2/4] ci: avoid brew for installing perforce","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T22:39:38Z","receivedAt":"2022-05-12T22:39:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Carlo Marcelo Arenas Belón \n\nPerfoce's cask in brew is meant[1] to be used only by humans, so replace\nits use from the CI with a scripted binary download which is less likely\nto fail, as it is done in Linux.\n\nKept the logic together so it will be less likely to break when moved\naround as on the fly code changes in this area are settled, at which\npoint it will also feasable to ammend it to avoid some of the hardcoded\nvalues by using similar variables to the ones Linux does.\n\nIn that same line, a POSIX sh syntax is used instead of the similar one\nused in Linux in preparation for an unrelated future change that might\nchange the shell currently configured for it.\n\nThis change reintroduces the risk that the installed binaries might not\nwork because of being quarantined that was fixed with 5ed9fc3fc86 (ci:\nprevent `perforce` from being quarantined, 2020-02-27) but fixing that\nnow was also punted for simplicity and since the affected cloud provider\nis scheduled to be retired with an on the fly change, but should be\naddressed if that other change is not integrated further.\n\nThe discussion on the need to keep 2 radically different versions of\nthe binaries to be tested with Linux vs macOS or how to upgrade to\nnewer versions now that brew won't do that automatically for us has\nbeen punted for now as well. On that line the now obsolete comment\nabout it in lib.sh was originally being updated by this change but\ncreated conflicts as it is moved around by other on the fly changes,\nso will be addressed independently as well.\n\n[1] https://github.com/Homebrew/homebrew-cask/pull/122347#discussion_r856026584\n\nSigned-off-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Junio C Hamano \n---\n ci/install-dependencies.sh | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex fa620f8fcd..8e796fa669 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,13 +37,14 @@ macos-latest)\n \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n \tbrew install $BREW_INSTALL_PACKAGES\n \tbrew link --force gettext\n-\tbrew install --cask --no-quarantine perforce || {\n-\t\t# Update the definitions and try again\n-\t\tcask_repo=\"$(brew --repository)\"/Library/Taps/homebrew/homebrew-cask &&\n-\t\tgit -C \"$cask_repo\" pull --no-stat --ff-only &&\n-\t\tbrew install --cask --no-quarantine perforce\n-\t} ||\n-\tbrew install homebrew/cask/perforce\n+\tmkdir -p $HOME/bin\n+\t(\n+\t\tcd $HOME/bin\n+\t\twget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n+\t\ttar -xf helix-core-server.tgz\n+\t)\n+\tPATH=\"$PATH:${HOME}/bin\"\n+\texport PATH\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.1-338-g1c7f76a54c\n\n"},{"id":"455231","messageId":"20220512223940.238367-4-gitster@pobox.com","threadId":"57778","inReplyTo":"20220512223940.238367-1-gitster@pobox.com","subject":"[PATCH v4 3/4] ci: reintroduce prevention from perforce being quarantined in macOS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T22:39:39Z","receivedAt":"2022-05-12T22:40:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Carlo Marcelo Arenas Belón \n\n5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)\nintroduces this prevention for brew, but brew has been removed in a\nprevious commit, so reintroduce an equivalent option to avoid a possible\nregression.\n\nThis doesn't affect github actions (as configure now) and is therefore\ndone silently to avoid any possible scary irrelevant messages.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Junio C Hamano \n---\n ci/install-dependencies.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 8e796fa669..c150bce2d9 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -41,7 +41,8 @@ macos-latest)\n \t(\n \t\tcd $HOME/bin\n \t\twget -q \"https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\" &&\n-\t\ttar -xf helix-core-server.tgz\n+\t\ttar -xf helix-core-server.tgz &&\n+\t\tsudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true\n \t)\n \tPATH=\"$PATH:${HOME}/bin\"\n \texport PATH\n-- \n2.36.1-338-g1c7f76a54c\n\n"},{"id":"455232","messageId":"20220512223940.238367-5-gitster@pobox.com","threadId":"57778","inReplyTo":"20220512223940.238367-1-gitster@pobox.com","subject":"[PATCH v4 4/4] ci: use https, not http to download binaries from perforce.com","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T22:39:40Z","receivedAt":"2022-05-12T22:40:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason \n\nSince 522354d70f4 (Add Travis CI support, 2015-11-27) the CI has used\nhttp://filehost.perforce.com/perforce/ to download binaries from\nfilehost.perforce.com, they were then moved to this script in\n657343a602e (travis-ci: move Travis CI code into dedicated scripts,\n2017-09-10).\n\nLet's use https instead for good measure. I don't think we need to\nworry about the DNS or network between the GitHub CI and perforce.com\nbeing MitM'd, but using https gives us extra validation of the payload\nat least, and is one less thing to worry about when checking where\nelse we rely on non-TLS'd http connections.\n\nAlso, use the same download site at perforce.com for Linux and macOS\ntarballs for consistency.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Junio C Hamano \n---\n ci/install-dependencies.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex c150bce2d9..107757a1fe 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -5,7 +5,7 @@\n \n . ${0%/*}/lib.sh\n \n-P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION\n+P4WHENCE=https://cdist2.perforce.com/perforce/r$LINUX_P4_VERSION\n LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION\n UBUNTU_COMMON_PKGS=\"make libssl-dev libcurl4-openssl-dev libexpat-dev\n tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl\n-- \n2.36.1-338-g1c7f76a54c\n\n"}]}