{"thread":{"id":"57771","subject":"[PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match","startedAt":"2022-04-21T12:54:02Z","lastAt":"2022-04-22T18:56:41Z","messageCount":21,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Eric Sunshine","Carlo Marcelo Arenas Belón","Carlo Arenas"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"454065","messageId":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","threadId":"57771","inReplyTo":null,"subject":"[PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T12:53:50Z","receivedAt":"2022-04-21T12:54:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Junio: Despite modifying CI stuff this series merges cleanly with\n\"seen\", and has no semantic conflicts with any outstanding CI changes.\n\nFor the past days we've again had CI failures due to \"brew install\"\ndetecting a SHA-256 mismatch when trying to install the perforce\npackage[1]. E.g. \"seen\" is now failing: https://github.com/git/git/runs/6104156856?check_suite_focus=true\n\nThis occurrence of this issue will no doubt be fixed within a few days\nas the homebrew-cask repository is updated, i.e. this recipe:\nhttps://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb\n\nBut for our CI usage being this anal about the check isn't worth it,\nhere's a passing CI run where we simply forced the installation:\nhttps://github.com/avar/git/runs/6092916675?check_suite_focus=true#step:3:87\nand subsequently passed the git-p4 tests:\nhttps://github.com/avar/git/runs/6092916675?check_suite_focus=true#step:4:1678\n\nÆvar Arnfjörð Bjarmason (2):\n CI: run \"brew install perforce\" without past workarounds\n CI: don't care about SHA256 mismatch on upstream \"perforce\" package\n\n ci/install-dependencies.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\n-- \n2.36.0.893.g80a51c675f6\n\n"},{"id":"454066","messageId":"patch-1.2-dcedf03c2d7-20220421T124225Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","subject":"[PATCH 1/2] CI: run \"brew install perforce\" without past workarounds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T12:53:51Z","receivedAt":"2022-04-21T12:54:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the alternating between --no-quarantine, --cask and fallback\n\"git pull\" updating of the \"perforce\" package.\n\nAs can be seen in [1], [2] and [3] these were workarounds for various\npast CI issues. Running \"brew install perforce\" works now in GitHub\nCI, so there's no need to alternate between package names, and the\n\"git pull\" method was a workaround for some staleness issue on the\nAzure pipelines removed in [4].\n\nWe do have a really common issue with this failing, but that's\nunrelated to any of those past fixes, and removing these old\nworkarounds makes dealing with that a lot easier.\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. 6081d3898fe (ci: retire the Azure Pipelines definition, 2020-04-11)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n ci/install-dependencies.sh | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb2..82fa87f97af 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,13 +37,7 @@ 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+\tbrew install perforce\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.893.g80a51c675f6\n\n"},{"id":"454067","messageId":"patch-2.2-28208bac859-20220421T124225Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","subject":"[PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T12:53:52Z","receivedAt":"2022-04-21T12:54:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As can be seen in the commit history[1] of the upstream perforce.rb in\nhomebrew-cask the upstream perforce package URL and its SHA-256 are\naren't a unique pair. The upstream will start publishing an updated\npackage at the same URL as the previous version, causing the CI to\nroutinely fail with errors like:\n\n\t==> Downloading https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\n\tError: SHA256 mismatch\n\tExpected: ffc757b9d4d0629b2594e2777edfb18097825e29c70d8f33a444c7482d622806\n\t Actual: 37bc306f0bdfd1d63cfcea113ada132d96f89d53cbb20c282735d51d06223054\n\nOnce someone gets around to updating the perforce.rb the failure of\ngit's CI will be cleared up, but in the meantime all osx-{gcc,clang}\njobs will encounter hard failures.\n\nLet's not be so anal about this and fallback to a \"sha256 :no_check\"\non failure. We are already downloading arbitrary binaries from\nperforce's servers, and the point of doing so is to run the\nt/*-git-p4-*.sh tests, not to validate the chain of custody between\nperforce.com and the homebrew-cask repository.\n\nIn the obscure (but unlikely to ever happen) that the failure is\nspecifically because perforce.com published a bad updated package, and\nit a failure that their testing wouldn't have caught, but whoever's\nupdating the homebrew SHA-256 recipe would have caught, we will have a\nfailure in our p4 tests that we wouldn't have otherwise had.\n\nBut I think that's so unlikely that we don't need to worry about it,\nwhereas seeing failures due to the homebrew recipe lagging upstream is\na real issue. E.g. \"seen\"'s latest push-out has such a failure: [3]\n\nNote: It's probably possible to embed this \"sed\" one-liner directly in\nthe HOMEBREW_EDITOR variable, i.e.:\n\n HOMEBREW_EDITOR='...' brew edit perforce\n\nBut my attempts to do so were unsuccessful, particularly since I don't\nhave access to a Mac OS X machine other than via by round-tripping\nthrough the CI. This version of getting the path via --print-path\nworks, and is arguably easier to reason about and debug than a cute\none-liner.\n\n1. https://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb\n2. https://docs.brew.sh/Cask-Cookbook#required-stanzas\n3. https://github.com/git/git/runs/6104156856?check_suite_focus=true\n---\n ci/install-dependencies.sh | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 82fa87f97af..540deab4488 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,7 +37,13 @@ macos-latest)\n \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n \tbrew install $BREW_INSTALL_PACKAGES\n \tbrew link --force gettext\n-\tbrew install perforce\n+\tbrew install perforce || {\n+\t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n+\n+\t\tpath=$(brew edit --print-path perforce) &&\n+\t\tsed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n+\t\tbrew install perforce\n+\t}\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.893.g80a51c675f6\n\n"},{"id":"454178","messageId":"xmqqo80uf9jn.fsf@gitster.g","threadId":"57771","inReplyTo":"patch-2.2-28208bac859-20220421T124225Z-avarab@gmail.com","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T20:47:40Z","receivedAt":"2022-04-21T20:48:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> In the obscure (but unlikely to ever happen) that the failure is\n> specifically because perforce.com published a bad updated package, and\n> it a failure that their testing wouldn't have caught, but whoever's\n> updating the homebrew SHA-256 recipe would have caught, we will have a\n> failure in our p4 tests that we wouldn't have otherwise had.\n\nOr DNS the CI site consults is tainted and we got a bad package from\na fake perforce.com?\n\n> @@ -37,7 +37,13 @@ macos-latest)\n> \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n> \tbrew install $BREW_INSTALL_PACKAGES\n> \tbrew link --force gettext\n> -\tbrew install perforce\n> +\tbrew install perforce || {\n> +\t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n\nI had to read this three times before realizing what you are\n\"assuming\". You suspect without having a way to verify that SHA-256\nmismatch was the reason why the attempt to install failed, and\nworking it around. Makes sense.\n\nWhat does it buy us to do this only as a fallback? If we munged the\n$path to disable sha256 checking before the initial \"brew install\",\nwe would install it happily if the package is the correct one, and\nif it is not a kosher one, we'd install it anyway.\n\nIs it so that we can tell if we had the checksum mismatch or not?\nIt is unfortunate that no_check is the only \"special\" value for the\nfield (I would have loved to use \"warn_only\" if it were available).\n\n> +\n> +\t\tpath=$(brew edit --print-path perforce) &&\n> +\t\tsed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n\n\"sed -i\" is not POSIX and without macOS box I do not know if it\nworks there. FreeBSD sed manual seems to indicate they have\n\n\tsed -i \n\nIn our current codebase, \"sed -i\" appears to be used only in vcxproj\npart of config.mak.uname\n\nI would usually have said that \"I'd rather see us not to use it\nhere, to prevent others from copying and pasting it, if it can be\nhelped\", but this is very much macOS specific part of an obscure\ncorner of the source tree, so as long as we are sure it works there,\nand if it is too cumbersome to avoid editing in-place, I'd let it\ngo.\n\nAh, no, I'd say we should NOT use \"sed -i\" here, not in the form in\nthis patch, after seeing\n\n https://unix.stackexchange.com/questions/401905/bsd-sed-vs-gnu-sed-and-i\n\nbut that is 4-year old information, so...\n\n> +\t\tbrew install perforce\n> +\t}\n> \n> \tif test -n \"$CC_PACKAGE\"\n> \tthen\n"},{"id":"454180","messageId":"220421.86pmla5eja.gmgdl@evledraar.gmail.com","threadId":"57771","inReplyTo":"xmqqo80uf9jn.fsf@gitster.g","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-21T21:08:41Z","receivedAt":"2022-04-21T21:10:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 21 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> In the obscure (but unlikely to ever happen) that the failure is\n>> specifically because perforce.com published a bad updated package, and\n>> it a failure that their testing wouldn't have caught, but whoever's\n>> updating the homebrew SHA-256 recipe would have caught, we will have a\n>> failure in our p4 tests that we wouldn't have otherwise had.\n>\n> Or DNS the CI site consults is tainted and we got a bad package from\n> a fake perforce.com?\n\nYeah, or any number of other things, all probably too obscure to worry about.\n\n>> @@ -37,7 +37,13 @@ macos-latest)\n>> \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n>> \tbrew install $BREW_INSTALL_PACKAGES\n>> \tbrew link --force gettext\n>> -\tbrew install perforce\n>> +\tbrew install perforce || {\n>> +\t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n>\n> I had to read this three times before realizing what you are\n> \"assuming\". You suspect without having a way to verify that SHA-256\n> mismatch was the reason why the attempt to install failed, and\n> working it around. Makes sense.\n>\n> What does it buy us to do this only as a fallback? If we munged the\n> $path to disable sha256 checking before the initial \"brew install\",\n> we would install it happily if the package is the correct one, and\n> if it is not a kosher one, we'd install it anyway.\n>\n> Is it so that we can tell if we had the checksum mismatch or not?\n> It is unfortunate that no_check is the only \"special\" value for the\n> field (I would have loved to use \"warn_only\" if it were available).\n\nYes, just to be able to tell that we tried, and overrode it. If anything\ngoes wrong we'll be able to see that we did that, in case it caused any\nfallout.\n\n>> +\n>> +\t\tpath=$(brew edit --print-path perforce) &&\n>> +\t\tsed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n>\n> \"sed -i\" is not POSIX and without macOS box I do not know if it\n> works there. FreeBSD sed manual seems to indicate they have\n>\n> \tsed -i \n>\n> In our current codebase, \"sed -i\" appears to be used only in vcxproj\n> part of config.mak.uname\n>\n> I would usually have said that \"I'd rather see us not to use it\n> here, to prevent others from copying and pasting it, if it can be\n> helped\", but this is very much macOS specific part of an obscure\n> corner of the source tree, so as long as we are sure it works there,\n> and if it is too cumbersome to avoid editing in-place, I'd let it\n> go.\n>\n> Ah, no, I'd say we should NOT use \"sed -i\" here, not in the form in\n> this patch, after seeing\n>\n> https://unix.stackexchange.com/questions/401905/bsd-sed-vs-gnu-sed-and-i\n>\n> but that is 4-year old information, so...\n\nIt works on the OSX we use now:\nhttps://github.com/avar/git/runs/6092916612?check_suite_focus=true\n\nI think it's fine to keep it, but we could also use \"perl -pi -e\" here,\nor a rename dance...\n\n>> +\t\tbrew install perforce\n>> +\t}\n>> \n>> \tif test -n \"$CC_PACKAGE\"\n>> \tthen\n\n"},{"id":"454183","messageId":"CAPig+cTWj2Xf4sfMJBkzLhG5oUBS+Pb5-D-K50xMt9rP0doBRA@mail.gmail.com","threadId":"57771","inReplyTo":"220421.86pmla5eja.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-21T21:29:42Z","receivedAt":"2022-04-21T21:30:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 21, 2022 at 5:10 PM Ævar Arnfjörð Bjarmason\n wrote:\n> On Thu, Apr 21 2022, Junio C Hamano wrote:\n> > Ævar Arnfjörð Bjarmason writes:\n> >> + sed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n> >\n> > \"sed -i\" is not POSIX and without macOS box I do not know if it\n> > works there. FreeBSD sed manual seems to indicate they have\n> >\n> > sed -i \n> >\n> > Ah, no, I'd say we should NOT use \"sed -i\" here, not in the form in\n> > this patch, after seeing\n> > https://unix.stackexchange.com/questions/401905/bsd-sed-vs-gnu-sed-and-i\n> > but that is 4-year old information, so...\n>\n> It works on the OSX we use now:\n> https://github.com/avar/git/runs/6092916612?check_suite_focus=true\n\nOn my end-of-life'd 10.13 macOS, `sed` (which is the FreeBSD version)\nrequires an argument after `-i`, thus it's taking `-e` as the\nextension name for the backup file, and then uses the raw\n's/\\(sha256.\\).*/\\1:no_check/' as the `sed` command. So, it's working,\nbut only by accident since `-e` is optional when invoking `sed`.\n\n> I think it's fine to keep it, but we could also use \"perl -pi -e\" here,\n> or a rename dance...\n\nAlthough it's working (by accident), it's rather misleading since it\nlooks like `-e` is introducing the `sed` command, even though it\nreally isn't (the `-e` is being consumed by the `-i` option). Any of\nthe following would likely be less confusing (in no particular order\nof preference):\n\n * sed -i .bak -e '...' \"$path\"\n * rename dance\n * perl -pi -e ...\n"},{"id":"454184","messageId":"20220421213024.xzkzuy3l7q5assxo@carlos-mbp.lan","threadId":"57771","inReplyTo":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","subject":"Re: [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-matchy","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-04-21T21:30:24Z","receivedAt":"2022-04-21T21:30:29Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, Apr 21, 2022 at 02:53:50PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> For the past days we've again had CI failures due to \"brew install\"\n> detecting a SHA-256 mismatch when trying to install the perforce\n\nSince the only reason why that is a concern is because it aborts the\nrest of the run and is a recurring problem, wouldn't it be better to\ntell the script to continue regardless and therefore skip all perforce\ntests?\n\nSure, there is a window where that integration could be broken which\nwill be only visible once the perforce cask gets fixed and perforce\ninstalls again, but wouldn't that be less intrusive and overall safer\nthan the currently proposed change?\n\nCarlo\n"},{"id":"454186","messageId":"xmqq7d7if76u.fsf@gitster.g","threadId":"57771","inReplyTo":"CAPig+cTWj2Xf4sfMJBkzLhG5oUBS+Pb5-D-K50xMt9rP0doBRA@mail.gmail.com","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T21:38:33Z","receivedAt":"2022-04-21T21:38:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> really isn't (the `-e` is being consumed by the `-i` option).\n\nThat makes sense. So after all 4-year old random finding on the\ninternet had some value in pregventing a new breakage to sneak into\nour codebase. Good ;-)\n\n> Any of\n> the following would likely be less confusing (in no particular order\n> of preference):\n>\n> * sed -i .bak -e '...' \"$path\"\n> * rename dance\n> * perl -pi -e ...\n\nThat order happens to match my preference, but if the first one\ncomes with a comment to dissuade readers to copy-and-paste the\nconstruct to other places in our code, that would be even better.\n\nPerhaps something along this line.\n\n ci/install-dependencies.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git c/ci/install-dependencies.sh w/ci/install-dependencies.sh\nindex 540deab448..2a5ee34246 100755\n--- c/ci/install-dependencies.sh\n+++ w/ci/install-dependencies.sh\n@@ -41,7 +41,12 @@ macos-latest)\n \t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n \n \t\tpath=$(brew edit --print-path perforce) &&\n-\t\tsed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n+\n+\t\t# we do not do this unconditionally because we want\n+\t\t# to know that we are falling back. Do not copy this\n+\t\t# use of 'sed -i .bak' elsewhere---it does not work with\n+\t\t# other implementations of \"sed\".\n+\t\tsed -i .bak -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n \t\tbrew install perforce\n \t}\n \n"},{"id":"454187","messageId":"xmqq35i6f754.fsf@gitster.g","threadId":"57771","inReplyTo":"20220421213024.xzkzuy3l7q5assxo@carlos-mbp.lan","subject":"Re: [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-matchy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T21:39:35Z","receivedAt":"2022-04-21T21:39:53Z","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> On Thu, Apr 21, 2022 at 02:53:50PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> For the past days we've again had CI failures due to \"brew install\"\n>> detecting a SHA-256 mismatch when trying to install the perforce\n>\n> Since the only reason why that is a concern is because it aborts the\n> rest of the run and is a recurring problem, wouldn't it be better to\n> tell the script to continue regardless and therefore skip all perforce\n> tests?\n>\n> Sure, there is a window where that integration could be broken which\n> will be only visible once the perforce cask gets fixed and perforce\n> installs again, but wouldn't that be less intrusive and overall safer\n> than the currently proposed change?\n\nGood suggestion. Care to come up with an alternative patch (or\ntwo)?\n\nThanks.\n"},{"id":"454189","messageId":"CAPig+cSMJJem_1t0rVwjbU9SsQfBpmbE2Yn6wZeNsY=h-1MAkg@mail.gmail.com","threadId":"57771","inReplyTo":"xmqq7d7if76u.fsf@gitster.g","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-04-21T21:48:20Z","receivedAt":"2022-04-21T21:48:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 21, 2022 at 5:38 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > Any of\n> > the following would likely be less confusing (in no particular order\n> > of preference):\n> >\n> > * sed -i .bak -e '...' \"$path\"\n> > * rename dance\n> > * perl -pi -e ...\n>\n> That order happens to match my preference, but if the first one\n> comes with a comment to dissuade readers to copy-and-paste the\n> construct to other places in our code, that would be even better.\n\nBikeshedding: I think I would prefer the rename-dance over a lengthy\ncomment meant to dissuade people from copying this non-portable usage,\nespecially since people often fail to read comments. The rename-dance\nidiom, on the other hand, can be cargo-culted without harm.\n"},{"id":"454190","messageId":"xmqqr15qdrqm.fsf@gitster.g","threadId":"57771","inReplyTo":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","subject":"Re: [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T21:57:37Z","receivedAt":"2022-04-21T21:57:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Junio: Despite modifying CI stuff this series merges cleanly with\n> \"seen\", and has no semantic conflicts with any outstanding CI changes.\n>\n> For the past days we've again had CI failures due to \"brew install\"\n> detecting a SHA-256 mismatch when trying to install the perforce\n> package[1]. E.g. \"seen\" is now failing: https://github.com/git/git/runs/6104156856?check_suite_focus=true\n\nYup, it was something I've been disturbed by and planning to ping\nfolks about if it continued.\n\n> This occurrence of this issue will no doubt be fixed within a few days\n> as the homebrew-cask repository is updated, i.e. this recipe:\n> https://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb\n>\n> But for our CI usage being this anal about the check isn't worth it,\n> here's a passing CI run where we simply forced the installation:\n> ...\n> Ævar Arnfjörð Bjarmason (2):\n> CI: run \"brew install perforce\" without past workarounds\n> CI: don't care about SHA256 mismatch on upstream \"perforce\" package\n\nI dunno. Does it open us to a new attack vector in some way?\n"},{"id":"454195","messageId":"xmqq5yn2dppk.fsf@gitster.g","threadId":"57771","inReplyTo":"CAPig+cSMJJem_1t0rVwjbU9SsQfBpmbE2Yn6wZeNsY=h-1MAkg@mail.gmail.com","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-21T22:41:27Z","receivedAt":"2022-04-21T22:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Thu, Apr 21, 2022 at 5:38 PM Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>> > Any of\n>> > the following would likely be less confusing (in no particular order\n>> > of preference):\n>> >\n>> > * sed -i .bak -e '...' \"$path\"\n>> > * rename dance\n>> > * perl -pi -e ...\n>>\n>> That order happens to match my preference, but if the first one\n>> comes with a comment to dissuade readers to copy-and-paste the\n>> construct to other places in our code, that would be even better.\n>\n> Bikeshedding: I think I would prefer the rename-dance over a lengthy\n> comment meant to dissuade people from copying this non-portable usage,\n> especially since people often fail to read comments. The rename-dance\n> idiom, on the other hand, can be cargo-culted without harm.\n\nYeah, that is fine, too.\n"},{"id":"454231","messageId":"patch-v2-3.3-3fdd54aa8df-20220422T085958Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com","subject":"[PATCH v2 3/3] CI: use https, not http to download binaries from perforce.com","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:07:39Z","receivedAt":"2022-04-22T09:14:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 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 cab6e04a358..00154a8ef45 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.879.g56a83971f3f\n\n"},{"id":"454232","messageId":"patch-v2-2.3-2805e89623c-20220422T085958Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com","subject":"[PATCH v2 2/3] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:07:38Z","receivedAt":"2022-04-22T09:14:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As can be seen in the commit history[1] of the upstream perforce.rb in\nhomebrew-cask the upstream perforce package URL and its SHA-256 are\naren't a unique pair. The upstream will start publishing an updated\npackage at the same URL as the previous version, causing the CI to\nroutinely fail with errors like:\n\n\t==> Downloading https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\n\tError: SHA256 mismatch\n\tExpected: ffc757b9d4d0629b2594e2777edfb18097825e29c70d8f33a444c7482d622806\n\t Actual: 37bc306f0bdfd1d63cfcea113ada132d96f89d53cbb20c282735d51d06223054\n\nOnce someone gets around to updating the perforce.rb the failure of\ngit's CI will be cleared up, but in the meantime all osx-{gcc,clang}\njobs will encounter hard failures.\n\nLet's not be so anal about this and fallback to a \"sha256 :no_check\"\non failure. For the \"ubuntu-latest\" jobs just a few lines earlier we\n\"wget\" and chmod binaries from http://filehost.perforce.com (no\nhttps!).\n\nWe're already trusting the DNS in the CI to do the right thing here,\nand for there not to be any MitM attacks between the CI and\nfilehost.perforce.com. Even if someone managed to get in the middle\nthe worst they could do is run arbitrary code in the CI environment.\n\nThe only thing we were getting out of the SHA-256 check was to serve\nas a canary that the homebrew-cask repository itself was drifting out\nof date. It seems sensible to just cut it out as a middle-man\nentirely (as the ubuntu-latest jobs do). We want to run the\nt/*-git-p4-*.sh tests, not to validate the chain of custody between\nperforce.com and the homebrew-cask repository.\n\nSee [2] for the \"no_check\" syntax, and [3] for an example of a failure\nin \"seen\" before this change.\n\nIt's been suggested as an alternative [4] to simply disable the p4\ntests if we can't install the package from homebrew. While I don't\nreally care, I think that would be strictly worse. Before this change\nwe've either run the p4 tests or failed, and we'll still fail now if\nwe can't run the p4 tests.\n\nWhereas skipping them entirely as [4] does is introducing the caveat\nthat our test coverage in these jobs today might be different than the\ncoverage tomorrow. If we do want to introduce such dynamic runs to CI\nI think they should use the relevant \"needs\" or \"if\" features, so that\nthe UX can make it obvious what was or wasn't dynamically skipped.\n\n1. https://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb\n2. https://docs.brew.sh/Cask-Cookbook#required-stanzas\n3. https://github.com/git/git/runs/6104156856?check_suite_focus=true\n4. https://lore.kernel.org/git/20220421225515.6316-1-carenas@gmail.com/\n---\n ci/install-dependencies.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 82fa87f97af..cab6e04a358 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,7 +37,14 @@ macos-latest)\n \ttest -z \"$BREW_INSTALL_PACKAGES\" ||\n \tbrew install $BREW_INSTALL_PACKAGES\n \tbrew link --force gettext\n-\tbrew install perforce\n+\tbrew install perforce || {\n+\t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n+\n+\t\tpath=$(brew edit --print-path perforce) &&\n+\t\tsed 's/\\(sha256.\\).*/\\1:no_check/' <\"$path\" >\"$path\".tmp &&\n+\t\tmv \"$path\".tmp \"$path\" &&\n+\t\tbrew install perforce\n+\t}\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.879.g56a83971f3f\n\n"},{"id":"454233","messageId":"cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-0.2-00000000000-20220421T124225Z-avarab@gmail.com","subject":"[PATCH v2 0/3] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:07:36Z","receivedAt":"2022-04-22T09:14:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I think this re-roll should address outstanding feedback with the v1.\n\nI also added an extra patch to change the existing http:// download of\nthe perforce binaries to use https://, which should further make the\ncase that we can do without any homebrew-specific sanity checking in\nthis area: It's what we're already doing for the ubuntu-latest jobs.\n\nI think a sensible follow-up to this would be to skip invoking\nhomebrew entirely, we can simply:\n\n wget https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz\n\nAnd then do the exact same thing we're doing to get the linux p4\nbinaries.\n\nA CI job showing this series working (including the working OSX fallback):\nhttps://github.com/avar/git/runs/6125736258?check_suite_focus=true#step:3:83\n\nIn addition to that the wget via https for the linux perforce package works:\nhttps://github.com/avar/git/runs/6125736067?check_suite_focus=true#step:3:273\n\nÆvar Arnfjörð Bjarmason (3):\n CI: run \"brew install perforce\" without past workarounds\n CI: don't care about SHA256 mismatch on upstream \"perforce\" package\n CI: use https, not http to download binaries from perforce.com\n\n ci/install-dependencies.sh | 17 +++++++++--------\n 1 file changed, 9 insertions(+), 8 deletions(-)\n\nRange-diff against v1:\n1: dcedf03c2d7 = 1: dcedf03c2d7 CI: run \"brew install perforce\" without past workarounds\n2: 28208bac859 ! 2: 2805e89623c CI: don't care about SHA256 mismatch on upstream \"perforce\" package\n @@ Commit message\n jobs will encounter hard failures.\n \n Let's not be so anal about this and fallback to a \"sha256 :no_check\"\n - on failure. We are already downloading arbitrary binaries from\n - perforce's servers, and the point of doing so is to run the\n - t/*-git-p4-*.sh tests, not to validate the chain of custody between\n - perforce.com and the homebrew-cask repository.\n + on failure. For the \"ubuntu-latest\" jobs just a few lines earlier we\n + \"wget\" and chmod binaries from http://filehost.perforce.com (no\n + https!).\n \n - In the obscure (but unlikely to ever happen) that the failure is\n - specifically because perforce.com published a bad updated package, and\n - it a failure that their testing wouldn't have caught, but whoever's\n - updating the homebrew SHA-256 recipe would have caught, we will have a\n - failure in our p4 tests that we wouldn't have otherwise had.\n + We're already trusting the DNS in the CI to do the right thing here,\n + and for there not to be any MitM attacks between the CI and\n + filehost.perforce.com. Even if someone managed to get in the middle\n + the worst they could do is run arbitrary code in the CI environment.\n \n - But I think that's so unlikely that we don't need to worry about it,\n - whereas seeing failures due to the homebrew recipe lagging upstream is\n - a real issue. E.g. \"seen\"'s latest push-out has such a failure: [3]\n + The only thing we were getting out of the SHA-256 check was to serve\n + as a canary that the homebrew-cask repository itself was drifting out\n + of date. It seems sensible to just cut it out as a middle-man\n + entirely (as the ubuntu-latest jobs do). We want to run the\n + t/*-git-p4-*.sh tests, not to validate the chain of custody between\n + perforce.com and the homebrew-cask repository.\n \n - Note: It's probably possible to embed this \"sed\" one-liner directly in\n - the HOMEBREW_EDITOR variable, i.e.:\n + See [2] for the \"no_check\" syntax, and [3] for an example of a failure\n + in \"seen\" before this change.\n \n - HOMEBREW_EDITOR='...' brew edit perforce\n + It's been suggested as an alternative [4] to simply disable the p4\n + tests if we can't install the package from homebrew. While I don't\n + really care, I think that would be strictly worse. Before this change\n + we've either run the p4 tests or failed, and we'll still fail now if\n + we can't run the p4 tests.\n \n - But my attempts to do so were unsuccessful, particularly since I don't\n - have access to a Mac OS X machine other than via by round-tripping\n - through the CI. This version of getting the path via --print-path\n - works, and is arguably easier to reason about and debug than a cute\n - one-liner.\n + Whereas skipping them entirely as [4] does is introducing the caveat\n + that our test coverage in these jobs today might be different than the\n + coverage tomorrow. If we do want to introduce such dynamic runs to CI\n + I think they should use the relevant \"needs\" or \"if\" features, so that\n + the UX can make it obvious what was or wasn't dynamically skipped.\n \n 1. https://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb\n 2. https://docs.brew.sh/Cask-Cookbook#required-stanzas\n 3. https://github.com/git/git/runs/6104156856?check_suite_focus=true\n + 4. https://lore.kernel.org/git/20220421225515.6316-1-carenas@gmail.com/\n \n ## ci/install-dependencies.sh ##\n @@ ci/install-dependencies.sh: macos-latest)\n @@ ci/install-dependencies.sh: macos-latest)\n +\t\techo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n +\n +\t\tpath=$(brew edit --print-path perforce) &&\n -+\t\tsed -i -e 's/\\(sha256.\\).*/\\1:no_check/' \"$path\" &&\n ++\t\tsed 's/\\(sha256.\\).*/\\1:no_check/' <\"$path\" >\"$path\".tmp &&\n ++\t\tmv \"$path\".tmp \"$path\" &&\n +\t\tbrew install perforce\n +\t}\n \n-: ----------- > 3: 3fdd54aa8df CI: use https, not http to download binaries from perforce.com\n-- \n2.36.0.879.g56a83971f3f\n\n"},{"id":"454234","messageId":"patch-v2-1.3-dcedf03c2d7-20220422T085958Z-avarab@gmail.com","threadId":"57771","inReplyTo":"cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com","subject":"[PATCH v2 1/3] CI: run \"brew install perforce\" without past workarounds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:07:37Z","receivedAt":"2022-04-22T09:14:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the alternating between --no-quarantine, --cask and fallback\n\"git pull\" updating of the \"perforce\" package.\n\nAs can be seen in [1], [2] and [3] these were workarounds for various\npast CI issues. Running \"brew install perforce\" works now in GitHub\nCI, so there's no need to alternate between package names, and the\n\"git pull\" method was a workaround for some staleness issue on the\nAzure pipelines removed in [4].\n\nWe do have a really common issue with this failing, but that's\nunrelated to any of those past fixes, and removing these old\nworkarounds makes dealing with that a lot easier.\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. 6081d3898fe (ci: retire the Azure Pipelines definition, 2020-04-11)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n ci/install-dependencies.sh | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dbcebad2fb2..82fa87f97af 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -37,13 +37,7 @@ 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+\tbrew install perforce\n \n \tif test -n \"$CC_PACKAGE\"\n \tthen\n-- \n2.36.0.879.g56a83971f3f\n\n"},{"id":"454236","messageId":"220422.86h76l5v7t.gmgdl@evledraar.gmail.com","threadId":"57771","inReplyTo":"20220421213024.xzkzuy3l7q5assxo@carlos-mbp.lan","subject":"Re: [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-matchy","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:21:43Z","receivedAt":"2022-04-22T09:22:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 21 2022, Carlo Marcelo Arenas Belón wrote:\n\n> On Thu, Apr 21, 2022 at 02:53:50PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> For the past days we've again had CI failures due to \"brew install\"\n>> detecting a SHA-256 mismatch when trying to install the perforce\n>\n> Since the only reason why that is a concern is because it aborts the\n> rest of the run and is a recurring problem, wouldn't it be better to\n> tell the script to continue regardless and therefore skip all perforce\n> tests?\n>\n> Sure, there is a window where that integration could be broken which\n> will be only visible once the perforce cask gets fixed and perforce\n> installs again, but wouldn't that be less intrusive and overall safer\n> than the currently proposed change?\n\nI tried to answer all of this in the updated v3 CL. Thanks!:\nhttps://lore.kernel.org/git/cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com/\n"},{"id":"454237","messageId":"220422.86czh95v74.gmgdl@evledraar.gmail.com","threadId":"57771","inReplyTo":"xmqq5yn2dppk.fsf@gitster.g","subject":"Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T09:22:15Z","receivedAt":"2022-04-22T09:22:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 21 2022, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n>\n>> On Thu, Apr 21, 2022 at 5:38 PM Junio C Hamano wrote:\n>>> Eric Sunshine writes:\n>>> > Any of\n>>> > the following would likely be less confusing (in no particular order\n>>> > of preference):\n>>> >\n>>> > * sed -i .bak -e '...' \"$path\"\n>>> > * rename dance\n>>> > * perl -pi -e ...\n>>>\n>>> That order happens to match my preference, but if the first one\n>>> comes with a comment to dissuade readers to copy-and-paste the\n>>> construct to other places in our code, that would be even better.\n>>\n>> Bikeshedding: I think I would prefer the rename-dance over a lengthy\n>> comment meant to dissuade people from copying this non-portable usage,\n>> especially since people often fail to read comments. The rename-dance\n>> idiom, on the other hand, can be cargo-culted without harm.\n>\n> Yeah, that is fine, too.\n\nI just used the rename dance in the updated v3:\nhttps://lore.kernel.org/git/cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com/\n"},{"id":"454243","messageId":"CAPUEspiuV4XjQH_r+uRMnstRfR4apB0Z83SoVbJTe7xrncyP-g@mail.gmail.com","threadId":"57771","inReplyTo":"patch-v2-1.3-dcedf03c2d7-20220422T085958Z-avarab@gmail.com","subject":"Re: [PATCH v2 1/3] CI: run \"brew install perforce\" without past workarounds","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T09:52:42Z","receivedAt":"2022-04-22T09:52:58Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Apr 22, 2022 at 2:07 AM Ævar Arnfjörð Bjarmason\n wrote:\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index dbcebad2fb2..82fa87f97af 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -37,13 +37,7 @@ 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> + brew install perforce\n\nWhile this might work under the current VM configuration used by\ngithub actions, is definitely not the usual configuration in macOS\ninstallations and therefore likely to break if run locally (as some\nother on the fly changes attempt to suggest)\n\nkeeping the \"--no-quarantine\" makes for a less likely to fail option\n(since SIP is enabled by default), and therefore I am also concerned\nthat by removing all these other (learned the hard way) workarounds we\nmight be making this more fragile for the future as well.\n\ninstead of this rewrite of the brew interface logic, removing brew as\nyou suggested would be probably better.\n\nCarlo\n"},{"id":"454253","messageId":"CAPUEsphEBdcxi_wbJxSQugo-bXS+eaEuhNuxciccux96M-B9cA@mail.gmail.com","threadId":"57771","inReplyTo":"patch-v2-2.3-2805e89623c-20220422T085958Z-avarab@gmail.com","subject":"Re: [PATCH v2 2/3] CI: don't care about SHA256 mismatch on upstream \"perforce\" package","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-04-22T11:15:51Z","receivedAt":"2022-04-22T11:16:06Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 82fa87f97af..cab6e04a358 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -37,7 +37,14 @@ macos-latest)\n> test -z \"$BREW_INSTALL_PACKAGES\" ||\n> brew install $BREW_INSTALL_PACKAGES\n> brew link --force gettext\n> - brew install perforce\n> + brew install perforce || {\n> + echo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&\n> +\n> + path=$(brew edit --print-path perforce) &&\n> + sed 's/\\(sha256.\\).*/\\1:no_check/' <\"$path\" >\"$path\".tmp &&\n> + mv \"$path\".tmp \"$path\" &&\n> + brew install perforce\n\nbrew install $path\n\nseems to be a safer way to ensure that brew will use the locally modified cask\n\nCarlo\n"},{"id":"454270","messageId":"220422.86o80tq7ex.gmgdl@evledraar.gmail.com","threadId":"57771","inReplyTo":"CAPUEspiuV4XjQH_r+uRMnstRfR4apB0Z83SoVbJTe7xrncyP-g@mail.gmail.com","subject":"Re: [PATCH v2 1/3] CI: run \"brew install perforce\" without past workarounds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T18:46:14Z","receivedAt":"2022-04-22T18:56:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 22 2022, Carlo Arenas wrote:\n\n> On Fri, Apr 22, 2022 at 2:07 AM Ævar Arnfjörð Bjarmason\n> wrote:\n>> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n>> index dbcebad2fb2..82fa87f97af 100755\n>> --- a/ci/install-dependencies.sh\n>> +++ b/ci/install-dependencies.sh\n>> @@ -37,13 +37,7 @@ 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>> + brew install perforce\n>\n> While this might work under the current VM configuration used by\n> github actions, is definitely not the usual configuration in macOS\n> installations and therefore likely to break if run locally (as some\n> other on the fly changes attempt to suggest)\n>\n> keeping the \"--no-quarantine\" makes for a less likely to fail option\n> (since SIP is enabled by default), and therefore I am also concerned\n> that by removing all these other (learned the hard way) workarounds we\n> might be making this more fragile for the future as well.\n\nIt works with the current CI, and keeping those fallbacks would have\nmeant turning this into some for-loop where we track which command\nvariant failed exactly, then retrying that with the SHA-256 munging.\n\n> instead of this rewrite of the brew interface logic, removing brew as\n> you suggested would be probably better.\n\nI won't have time to pursue that in the near future, sorry. Especially\nas I've got no OSX box, so any changes to this series mean long painful\nbouncing around against the GitHub CI.\n\nAnyway. I'd be happy for you to pick this up, whether that means\nre-rolling my series with your suggested changes or taking yours over\nmine I really don't care, I just want to not see those OSX CI errors\nanymore.\n\nAs noted I have a mild preference out of general principle of having the\nCI at least somewhat deterministic, i.e. to do this series where if we\ncan't get p4 at all we'll fail.\n\nBut I'd be fine either way, i.e. your series is also fine by\nme. Whatever stops these errors from happening whenever perforce.com\nupdates the packages & the brew recipes haven't been bumped yet...\n\nThanks!\n"}]}