{"thread":{"id":"56290","subject":"[PATCH] ci: add job for gcc-4.8 to GitHub Actions","startedAt":"2021-08-16T04:58:10Z","lastAt":"2021-08-17T19:36:11Z","messageCount":7,"participants":["Carlo Marcelo Arenas Belón","Derrick Stolee","Jeff King","Carlo Arenas","SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"432782","messageId":"20210816045750.36499-1-carenas@gmail.com","threadId":"56290","inReplyTo":null,"subject":"[PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-16T04:57:50Z","receivedAt":"2021-08-16T04:58:10Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"unlike the other jobs; using an older ubuntu base image that provides\nthat compiler as an option.\n\nnote the obsoleted travis job used an image of the OS that is EOL and\ntherefore not available, but the compiler used will be the same, and\nmore importantly will fail in the same (C89 compatibility) issues.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\nbased on top of my tip for cb/reftable-fixes, but applies cleanly all\nthe way to maint.\n\na succesful run can be seen in:\n\n https://github.com/carenas/git/runs/3336674183\n\nit adds 2m to the current setup, but gcc 4.8 is hard to find in modern\ndeveloper workstations (or even non EOL enterprise systems)\n\n .github/workflows/main.yml | 3 +++\n ci/install-dependencies.sh | 6 +++++-\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 73856bafc9..0f211173fc 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -297,6 +297,9 @@ jobs:\n - jobname: linux-gcc-default\n cc: gcc\n pool: ubuntu-latest\n+ - jobname: linux-gcc-4.8\n+ cc: gcc-4.8\n+ pool: ubuntu-18.04\n env:\n CC: ${{matrix.vector.cc}}\n jobname: ${{matrix.vector.jobname}}\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 67852d0d37..950bc39129 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -72,10 +72,14 @@ Documentation)\n \ttest -n \"$ALREADY_HAVE_ASCIIDOCTOR\" ||\n \tsudo gem install --version 1.5.8 asciidoctor\n \t;;\n-linux-gcc-default|linux-gcc-4.8)\n+linux-gcc-default)\n \tsudo apt-get -q update\n \tsudo apt-get -q -y install $UBUNTU_COMMON_PKGS\n \t;;\n+linux-gcc-4.8)\n+\tsudo apt-get -q update\n+\tsudo apt-get -q -y install $UBUNTU_COMMON_PKGS gcc-4.8\n+\t;;\n esac\n \n if type p4d >/dev/null && type p4 >/dev/null\n-- \n2.33.0.rc2.476.g1b09a32a73\n\n"},{"id":"432807","messageId":"444e7410-c9c9-1b90-da5f-d6862a35c1fc@gmail.com","threadId":"56290","inReplyTo":"20210816045750.36499-1-carenas@gmail.com","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-08-16T16:06:46Z","receivedAt":"2021-08-16T16:06:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/16/2021 12:57 AM, Carlo Marcelo Arenas Belón wrote:\n> unlike the other jobs; using an older ubuntu base image that provides\n> that compiler as an option.\n> \n> note the obsoleted travis job used an image of the OS that is EOL and\n> therefore not available, but the compiler used will be the same, and\n> more importantly will fail in the same (C89 compatibility) issues.\n> \n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> based on top of my tip for cb/reftable-fixes, but applies cleanly all\n> the way to maint.\n> \n> a succesful run can be seen in:\n> \n> https://github.com/carenas/git/runs/3336674183\n> \n> it adds 2m to the current setup, but gcc 4.8 is hard to find in modern\n> developer workstations (or even non EOL enterprise systems)\n\nForgive me, I probably missed a discussion about this\nsomewhere else on the list, but...\n\nCould you describe why we want GCC 4.8 in our CI? Is that a\ncompiler version that we officially support? What kind of\nsyntax triggers a problem on 4.8 versus latest?\n\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 73856bafc9..0f211173fc 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -297,6 +297,9 @@ jobs:\n> - jobname: linux-gcc-default\n> cc: gcc\n> pool: ubuntu-latest\n> + - jobname: linux-gcc-4.8\n> + cc: gcc-4.8\n> + pool: ubuntu-18.04\n\nMakes sense.\n\n> env:\n> CC: ${{matrix.vector.cc}}\n> jobname: ${{matrix.vector.jobname}}\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 67852d0d37..950bc39129 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -72,10 +72,14 @@ Documentation)\n> \ttest -n \"$ALREADY_HAVE_ASCIIDOCTOR\" ||\n> \tsudo gem install --version 1.5.8 asciidoctor\n> \t;;\n> -linux-gcc-default|linux-gcc-4.8)\n> +linux-gcc-default)\n> \tsudo apt-get -q update\n> \tsudo apt-get -q -y install $UBUNTU_COMMON_PKGS\n> \t;;\n> +linux-gcc-4.8)\n> +\tsudo apt-get -q update\n> +\tsudo apt-get -q -y install $UBUNTU_COMMON_PKGS gcc-4.8\n> +\t;;\n\nInteresting that we already had a case here. Is there interesting\nhistory about this prior-existing case that might be illuminating\nto the current need?\n\nThanks,\n-Stolee\n"},{"id":"432808","messageId":"YRqP7J7Gu0VmB6q2@coredump.intra.peff.net","threadId":"56290","inReplyTo":"444e7410-c9c9-1b90-da5f-d6862a35c1fc@gmail.com","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-16T16:18:52Z","receivedAt":"2021-08-16T16:19:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 16, 2021 at 12:06:46PM -0400, Derrick Stolee wrote:\n\n> Forgive me, I probably missed a discussion about this\n> somewhere else on the list, but...\n> \n> Could you describe why we want GCC 4.8 in our CI? Is that a\n> compiler version that we officially support? What kind of\n> syntax triggers a problem on 4.8 versus latest?\n\nTry fb9d7431cf (travis-ci: build with GCC 4.8 as well, 2019-07-18).\n(found with \"git log -Sgcc-4.8 ci\"). The gist of it is to find variable\ndeclarations in for-loops.\n\nIMHO it may be more trouble than it's worth. If we can't find a compiler\nthat complains on this construct, then maybe it is not a construct worth\nworrying too much about.\n\n-Peff\n"},{"id":"432816","messageId":"CAPUEsphsyxdjf7WfqQQhBbVYt_URAdj4Cg-OT+dJenO7vpfOJQ@mail.gmail.com","threadId":"56290","inReplyTo":"444e7410-c9c9-1b90-da5f-d6862a35c1fc@gmail.com","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-08-16T16:58:04Z","receivedAt":"2021-08-16T16:58:21Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Aug 16, 2021 at 9:06 AM Derrick Stolee wrote:\n> On 8/16/2021 12:57 AM, Carlo Marcelo Arenas Belón wrote:\n> > it adds 2m to the current setup, but gcc 4.8 is hard to find in modern\n> > developer workstations (or even non EOL enterprise systems)\n>\n> Forgive me, I probably missed a discussion about this\n> somewhere else on the list, but...\n>\n> Could you describe why we want GCC 4.8 in our CI? Is that a\n> compiler version that we officially support?\n\ncouldn't find the specific thread I seem to remember, but AFAIK it was\nbecause it is the compiler from RHEL7\n\nCarlo\n"},{"id":"432941","messageId":"20210817111512.GA2257957@szeder.dev","threadId":"56290","inReplyTo":"YRqP7J7Gu0VmB6q2@coredump.intra.peff.net","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-17T11:15:12Z","receivedAt":"2021-08-17T11:15:21Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Aug 16, 2021 at 12:18:52PM -0400, Jeff King wrote:\n> On Mon, Aug 16, 2021 at 12:06:46PM -0400, Derrick Stolee wrote:\n> \n> > Forgive me, I probably missed a discussion about this\n> > somewhere else on the list, but...\n> > \n> > Could you describe why we want GCC 4.8 in our CI? Is that a\n> > compiler version that we officially support? What kind of\n> > syntax triggers a problem on 4.8 versus latest?\n> \n> Try fb9d7431cf (travis-ci: build with GCC 4.8 as well, 2019-07-18).\n> (found with \"git log -Sgcc-4.8 ci\"). The gist of it is to find variable\n> declarations in for-loops.\n> \n> IMHO it may be more trouble than it's worth. If we can't find a compiler\n> that complains on this construct, then maybe it is not a construct worth\n> worrying too much about.\n\nI for one like for loop initial declarations, because they allow us to\nlimit the scope of the loop variable to the loop, and would love to\nsee it used in more places (well, wherever possible, actually, but\nthat'd be a lot of churn). So I would prefer to just drop this job\nsooner rather than later, update CodingGuidelines, and, if deemed\nnecessary, launch a weather balloon.\n\n"},{"id":"432988","messageId":"YRvSoGodEJB26twE@coredump.intra.peff.net","threadId":"56290","inReplyTo":"20210817111512.GA2257957@szeder.dev","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-17T15:15:44Z","receivedAt":"2021-08-17T15:15:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 17, 2021 at 01:15:12PM +0200, SZEDER Gábor wrote:\n\n> > Try fb9d7431cf (travis-ci: build with GCC 4.8 as well, 2019-07-18).\n> > (found with \"git log -Sgcc-4.8 ci\"). The gist of it is to find variable\n> > declarations in for-loops.\n> > \n> > IMHO it may be more trouble than it's worth. If we can't find a compiler\n> > that complains on this construct, then maybe it is not a construct worth\n> > worrying too much about.\n> \n> I for one like for loop initial declarations, because they allow us to\n> limit the scope of the loop variable to the loop, and would love to\n> see it used in more places (well, wherever possible, actually, but\n> that'd be a lot of churn). So I would prefer to just drop this job\n> sooner rather than later, update CodingGuidelines, and, if deemed\n> necessary, launch a weather balloon.\n\nYeah, I think it would be nice to use that, too, if it works everywhere.\nThe last discussion of this was from 2017, where peple likewise seemed\nin favor:\n\n https://lore.kernel.org/git/xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com/\n\nThere was even a weather balloon patch:\n\n https://lore.kernel.org/git/20170719181956.15845-1-sbeller@google.com/\n\nbut it got hung up on somebody using gcc 4.8. ;)\n\nIt looks like the default flavor bumped to gnu90 in gcc 5. That came out\nin 2015, but the last 4.x release was in August 2016. Which is getting\nold-ish, but still well within the realm of what people may be using an\non older distro (e.g., RHEL7, which is still supported).\n\nFor gcc, at least, this is trivially fixable with a `-std` flag (though\nit may be tricky to make it work out-of-the-box through the Makefile). I\ndon't think we'll know if there problems with other compilers until we\ndo the weather balloon.\n\n-Peff\n"},{"id":"433021","messageId":"xmqqbl5v50ev.fsf@gitster.g","threadId":"56290","inReplyTo":"20210817111512.GA2257957@szeder.dev","subject":"Re: [PATCH] ci: add job for gcc-4.8 to GitHub Actions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-17T19:36:08Z","receivedAt":"2021-08-17T19:36:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> IMHO it may be more trouble than it's worth. If we can't find a compiler\n>> that complains on this construct, then maybe it is not a construct worth\n>> worrying too much about.\n\nAbsolutely. Of course, there are vendor compilers that are not GNU\nor clang that may throw us surprises ;-)\n\n> I for one like for loop initial declarations, because they allow us to\n> limit the scope of the loop variable to the loop, and would love to\n> see it used in more places (well, wherever possible, actually, but\n> that'd be a lot of churn). So I would prefer to just drop this job\n> sooner rather than later, update CodingGuidelines, and, if deemed\n> necessary, launch a weather balloon.\n\nI'd agree in general, but it must be done in a bit different order,\ni.e. weather balloon first.\n"}]}