{"thread":{"id":"57976","subject":"[PATCH 1/2] ci(github): use grouping also in the `win-build` job","startedAt":"2022-06-09T11:33:43Z","lastAt":"2022-06-13T23:01:31Z","messageCount":11,"participants":["Johannes Schindelin via GitGitGadget","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"456934","messageId":"5212c5ec474b9441046203da363f2205795f213e.1654774347.git.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":"pull.1253.git.1654774347.gitgitgadget@gmail.com","subject":"[PATCH 1/2] ci(github): use grouping also in the `win-build` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-09T11:32:26Z","receivedAt":"2022-06-09T11:33:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe already do the same when building Git in all the other jobs.\n\nThis will allow us to piggy-back on top of grouping to mark up compiler\nerrors in the next commit.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/make-test-artifacts.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/make-test-artifacts.sh b/ci/make-test-artifacts.sh\nindex 646967481f6..74141af0cc7 100755\n--- a/ci/make-test-artifacts.sh\n+++ b/ci/make-test-artifacts.sh\n@@ -7,6 +7,6 @@ mkdir -p \"$1\" # in case ci/lib.sh decides to quit early\n \n . ${0%/*}/lib.sh\n \n-make artifacts-tar ARTIFACTS_DIRECTORY=\"$1\"\n+group Build make artifacts-tar ARTIFACTS_DIRECTORY=\"$1\"\n \n check_unignored_build_artifacts\n-- \ngitgitgadget\n\n"},{"id":"456935","messageId":"pull.1253.git.1654774347.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":null,"subject":"[PATCH 0/2] ci(GitHub): mark up compile errors, too","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-09T11:32:25Z","receivedAt":"2022-06-09T11:35:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like we mark up test failures, it makes sense to mark up compile\nerrors, too.\n\nIn a sense, it makes even more sense with compile errors than with test\nfailures because we can link directly to the corresponding source code in\nthe former case (if said code has been touched by the Pull Request, that\nis). The only downside is that this link currently is kind of misleading if\nthe Pull Request did not even touch the offending source code (such as was\nthe case when a GCC upgrade in Git for Windows' SDK all of a sudden pointed\nout problems in the source code that had existed for a long time already).\nWe will see how the GitHub Actions engineers will develop this feature\nfurther.\n\nThis patch series is based on js/ci-github-workflow-markup. Which also\nserves as an example how this looks like if the offending source code was\nnot touched by the Pull Request:\nhttps://github.com/gitgitgadget/git/actions/runs/2461737185 because it still\ntriggers the above-referenced GCC build failure.\n\nJohannes Schindelin (2):\n ci(github): use grouping also in the `win-build` job\n ci(github): also mark up compile errors\n\n ci/lib.sh | 10 ++++++++--\n ci/make-test-artifacts.sh | 2 +-\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\n\nbase-commit: 3069f2a6f4c38e7e599067d2e4a8e31b4f53e2d3\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1253%2Fdscho%2Fci-mark-up-compile-failures-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1253/dscho/ci-mark-up-compile-failures-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1253\n-- \ngitgitgadget\n"},{"id":"456936","messageId":"19d6e34f038121b927cdfacc3c4ae5abd1791415.1654774347.git.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":"pull.1253.git.1654774347.gitgitgadget@gmail.com","subject":"[PATCH 2/2] ci(github): also mark up compile errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-09T11:32:27Z","receivedAt":"2022-06-09T11:35:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen GCC produces those helpful errors, we will want to present them in\nthe GitHub workflow runs in the most helpful manner. To that end, we\nwant to use workflow commands to render errors and warnings:\nhttps://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions\n\nIn the previous commit, we ensured that grouping is used for the build\nin all jobs, and this allows us to piggy-back onto the `group` function\nto transmogrify the output.\n\nNote: If `set -o pipefail` was available, we could do this in a little\nmore elegant way. But since some of the steps are run using `dash`, we\nhave to do a little `{ ...; echo $? >exit.status; } | ...` dance.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 2f6d9d26e40..b747e34842c 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -29,8 +29,14 @@ else\n \t\tset +x\n \t\tbegin_group \"$1\"\n \t\tshift\n-\t\t\"$@\"\n-\t\tres=$?\n+\t\t# work around `dash` not supporting `set -o pipefail`\n+\t\t{\n+\t\t\t\"$@\" 2>&1\n+\t\t\techo $? >exit.status\n+\t\t} |\n+\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2 line=\\3::\\1/'\n+\t\tres=$(cat exit.status)\n+\t\trm exit.status\n \t\tend_group\n \t\treturn $res\n \t}\n-- \ngitgitgadget\n"},{"id":"456940","messageId":"220609.864k0ut010.gmgdl@evledraar.gmail.com","threadId":"57976","inReplyTo":"19d6e34f038121b927cdfacc3c4ae5abd1791415.1654774347.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] ci(github): also mark up compile errors","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-09T13:47:35Z","receivedAt":"2022-06-09T13:54:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 09 2022, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> When GCC produces those helpful errors, we will want to present them in\n> the GitHub workflow runs in the most helpful manner. To that end, we\n> want to use workflow commands to render errors and warnings:\n> https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions\n\nThe docs you're linking to state:\n\n\t::warning file={name},line={line},endLine={endLine},title={title}::{message}\n\nBut here...\n\n> +\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2 line=\\3::\\1/'\n\nYou seem to omit the comma, and the CI output itself seems to note the\nfilename as \"compat/win32/syslog.c line=53#L1\".\n\nI haven't tested, but is this the issue you noted in the CL as \"the only\ndownside\"? I.e. the link to the source code is nonsensical in that CI\noutput, it links to the diff of the PR itself.\n\nBut the GH docs say \"associate the message with a particular file in\nyour repository.\", so it would seem that there should be a way to link\nto the file at that revision, not only if it was altered in the given\ncommit.\n\nOn the \"sed\" one-liner, at least GCC supports emitting JSON error\noutput:\nhttps://stackoverflow.com/questions/36657869/how-do-i-dump-gcc-warnings-into-a-structured-format\n\nYou don't fill in \"column\" now, but if you used that presumably that\nwould be easy, and more useful.\n\nIt seems clang also supports it, but not any easily machine-readable\nformat:\nhttps://clang.llvm.org/docs/UsersManual.html#cmdoption-fdiagnostics-format\n"},{"id":"456970","messageId":"xmqqbkv1gzmy.fsf@gitster.g","threadId":"57976","inReplyTo":"pull.1253.git.1654774347.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] ci(GitHub): mark up compile errors, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-09T23:56:05Z","receivedAt":"2022-06-09T23:56:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Just like we mark up test failures, it makes sense to mark up compile\n> errors, too.\n>\n> In a sense, it makes even more sense with compile errors than with test\n> failures because we can link directly to the corresponding source code in\n\nAbsolutely ;-).\n"},{"id":"457030","messageId":"xmqqzgikfpm8.fsf@gitster.g","threadId":"57976","inReplyTo":"19d6e34f038121b927cdfacc3c4ae5abd1791415.1654774347.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] ci(github): also mark up compile errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-10T16:30:07Z","receivedAt":"2022-06-10T16:30:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> +\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2 line=\\3::\\1/'\n\nhttps://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message\n\nseems to use comma as field separator, not SP, for files and lines.\nWe'll see if they are equivalent without getting documented soon, as\nI will be adding this to my tree for 'seen' today. We should throw\na build failure in to see its effect ;-)\n\n"},{"id":"457097","messageId":"34daf06bb710434a2392b3cb91d35047565d5602.1655125988.git.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":"pull.1253.v2.git.1655125988.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] ci(github): also mark up compile errors","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-13T13:13:08Z","receivedAt":"2022-06-13T15:39:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen GCC produces those helpful errors, we will want to present them in\nthe GitHub workflow runs in the most helpful manner. To that end, we\nwant to use workflow commands to render errors and warnings:\nhttps://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions\n\nIn the previous commit, we ensured that grouping is used for the build\nin all jobs, and this allows us to piggy-back onto the `group` function\nto transmogrify the output.\n\nNote: If `set -o pipefail` was available, we could do this in a little\nmore elegant way. But since some of the steps are run using `dash`, we\nhave to do a little `{ ...; echo $? >exit.status; } | ...` dance.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/lib.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 2f6d9d26e40..aa7e979a0bf 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -29,8 +29,14 @@ else\n \t\tset +x\n \t\tbegin_group \"$1\"\n \t\tshift\n-\t\t\"$@\"\n-\t\tres=$?\n+\t\t# work around `dash` not supporting `set -o pipefail`\n+\t\t(\n+\t\t\t\"$@\" 2>&1\n+\t\t\techo $? >exit.status\n+\t\t) |\n+\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2,line=\\3::\\1/'\n+\t\tres=$(cat exit.status)\n+\t\trm exit.status\n \t\tend_group\n \t\treturn $res\n \t}\n-- \ngitgitgadget\n"},{"id":"457098","messageId":"pull.1253.v2.git.1655125988.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":"pull.1253.git.1654774347.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] ci(GitHub): mark up compile errors, too","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-13T13:13:06Z","receivedAt":"2022-06-13T15:39:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like we mark up test failures, it makes sense to mark up compile\nerrors, too.\n\nIn a sense, it makes even more sense with compile errors than with test\nfailures because we can link directly to the corresponding source code in\nthe former case (if said code has been touched by the Pull Request, that\nis). The only downside is that this link currently is kind of misleading if\nthe Pull Request did not even touch the offending source code (such as was\nthe case when a GCC upgrade in Git for Windows' SDK all of a sudden pointed\nout problems in the source code that had existed for a long time already).\nWe will see how the GitHub Actions engineers will develop this feature\nfurther.\n\nThis patch series is based on js/ci-github-workflow-markup. Which also\nserves as an example how this looks like if the offending source code was\nnot touched by the Pull Request:\nhttps://github.com/dscho/git/actions/runs/2477526645 because it still\ntriggers the above-referenced GCC build failure.\n\nChanges since v1:\n\n * Using a comma in the workflow command now, as described in the official\n documentation ;-) (Thank you, Ævar)\n * The curly bracket construct was replaced by a proper subshell, to avoid\n jumbled output and a race where the exit.status file could be read before\n it was written.\n\nJohannes Schindelin (2):\n ci(github): use grouping also in the `win-build` job\n ci(github): also mark up compile errors\n\n ci/lib.sh | 10 ++++++++--\n ci/make-test-artifacts.sh | 2 +-\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\n\nbase-commit: 3069f2a6f4c38e7e599067d2e4a8e31b4f53e2d3\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1253%2Fdscho%2Fci-mark-up-compile-failures-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1253/dscho/ci-mark-up-compile-failures-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1253\n\nRange-diff vs v1:\n\n 1: 5212c5ec474 = 1: 5212c5ec474 ci(github): use grouping also in the `win-build` job\n 2: 19d6e34f038 ! 2: 34daf06bb71 ci(github): also mark up compile errors\n @@ ci/lib.sh: else\n -\t\t\"$@\"\n -\t\tres=$?\n +\t\t# work around `dash` not supporting `set -o pipefail`\n -+\t\t{\n ++\t\t(\n +\t\t\t\"$@\" 2>&1\n +\t\t\techo $? >exit.status\n -+\t\t} |\n -+\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2 line=\\3::\\1/'\n ++\t\t) |\n ++\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2,line=\\3::\\1/'\n +\t\tres=$(cat exit.status)\n +\t\trm exit.status\n \t\tend_group\n\n-- \ngitgitgadget\n"},{"id":"457099","messageId":"5212c5ec474b9441046203da363f2205795f213e.1655125988.git.gitgitgadget@gmail.com","threadId":"57976","inReplyTo":"pull.1253.v2.git.1655125988.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] ci(github): use grouping also in the `win-build` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-06-13T13:13:07Z","receivedAt":"2022-06-13T15:39:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWe already do the same when building Git in all the other jobs.\n\nThis will allow us to piggy-back on top of grouping to mark up compiler\nerrors in the next commit.\n\nSigned-off-by: Johannes Schindelin \n---\n ci/make-test-artifacts.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ci/make-test-artifacts.sh b/ci/make-test-artifacts.sh\nindex 646967481f6..74141af0cc7 100755\n--- a/ci/make-test-artifacts.sh\n+++ b/ci/make-test-artifacts.sh\n@@ -7,6 +7,6 @@ mkdir -p \"$1\" # in case ci/lib.sh decides to quit early\n \n . ${0%/*}/lib.sh\n \n-make artifacts-tar ARTIFACTS_DIRECTORY=\"$1\"\n+group Build make artifacts-tar ARTIFACTS_DIRECTORY=\"$1\"\n \n check_unignored_build_artifacts\n-- \ngitgitgadget\n\n"},{"id":"457108","messageId":"xmqq35g88p9y.fsf@gitster.g","threadId":"57976","inReplyTo":"pull.1253.v2.git.1655125988.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] ci(GitHub): mark up compile errors, too","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-13T17:08:25Z","receivedAt":"2022-06-13T19:10:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> * The curly bracket construct was replaced by a proper subshell, to avoid\n> jumbled output and a race where the exit.status file could be read before\n> it was written.\n\nI do prefer () when making a subshell in a case like this (e.g.\nupstream of a pipe), so I am happy with this version, but the above\nis curious.\n\nI am not sure how \"jumbled output\" is possible, let alone \"reading\nexit.status before it is written\". The output goes to sed to be\nprocessed either way, nobody else other than \"$@\" produces such an\noutput from there, and sed would not exit until it finishes reading\nfrom the upstream so res=$(cat exit.status) won't kick in before the\nupstream exits.\n\nAnyway, thanks, will queue.\n\n> Johannes Schindelin (2):\n> ci(github): use grouping also in the `win-build` job\n> ci(github): also mark up compile errors\n>\n> ci/lib.sh | 10 ++++++++--\n> ci/make-test-artifacts.sh | 2 +-\n> 2 files changed, 9 insertions(+), 3 deletions(-)\n>\n>\n> base-commit: 3069f2a6f4c38e7e599067d2e4a8e31b4f53e2d3\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1253%2Fdscho%2Fci-mark-up-compile-failures-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1253/dscho/ci-mark-up-compile-failures-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1253\n>\n> Range-diff vs v1:\n>\n> 1: 5212c5ec474 = 1: 5212c5ec474 ci(github): use grouping also in the `win-build` job\n> 2: 19d6e34f038 ! 2: 34daf06bb71 ci(github): also mark up compile errors\n> @@ ci/lib.sh: else\n> -\t\t\"$@\"\n> -\t\tres=$?\n> +\t\t# work around `dash` not supporting `set -o pipefail`\n> -+\t\t{\n> ++\t\t(\n> +\t\t\t\"$@\" 2>&1\n> +\t\t\techo $? >exit.status\n> -+\t\t} |\n> -+\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2 line=\\3::\\1/'\n> ++\t\t) |\n> ++\t\tsed 's/^\\(\\([^ ]*\\):\\([0-9]*\\):\\([0-9]*:\\) \\)\\(error\\|warning\\): /::\\5 file=\\2,line=\\3::\\1/'\n> +\t\tres=$(cat exit.status)\n> +\t\trm exit.status\n> \t\tend_group\n"},{"id":"457149","messageId":"220614.86tu8oyxu9.gmgdl@evledraar.gmail.com","threadId":"57976","inReplyTo":"pull.1253.v2.git.1655125988.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] ci(GitHub): mark up compile errors, too","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-13T22:41:28Z","receivedAt":"2022-06-13T23:01:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 13 2022, Johannes Schindelin via GitGitGadget wrote:\n\n> Just like we mark up test failures, it makes sense to mark up compile\n> errors, too.\n>\n> In a sense, it makes even more sense with compile errors than with test\n> failures because we can link directly to the corresponding source code in\n> the former case (if said code has been touched by the Pull Request, that\n> is). The only downside is that this link currently is kind of misleading if\n> the Pull Request did not even touch the offending source code (such as was\n> the case when a GCC upgrade in Git for Windows' SDK all of a sudden pointed\n> out problems in the source code that had existed for a long time already).\n> We will see how the GitHub Actions engineers will develop this feature\n> further.\n>\n> This patch series is based on js/ci-github-workflow-markup. Which also\n> serves as an example how this looks like if the offending source code was\n> not touched by the Pull Request:\n> https://github.com/dscho/git/actions/runs/2477526645 because it still\n> triggers the above-referenced GCC build failure.\n>\n> Changes since v1:\n>\n> * Using a comma in the workflow command now, as described in the official\n> documentation ;-) (Thank you, Ævar)\n\nYou're welcome!\n\n> * The curly bracket construct was replaced by a proper subshell, to avoid\n> jumbled output and a race where the exit.status file could be read before\n> it was written.\n>\n> Johannes Schindelin (2):\n> ci(github): use grouping also in the `win-build` job\n> ci(github): also mark up compile errors\n\nIt's still genuinely unclear to me what exactly the expected\nbefore/after result is, and I wish the 2/2 commit would discuss it.\n\nSo, in v1 we had this: https://github.com/gitgitgadget/git/actions/runs/2461737185\n\nWhere the *summary* for the CI said e.g. \"syslog.c line=53#L1\", so that\nwas the \"needs a comma\" bug, now it says syslog.c#L53 instead:\nhttps://github.com/dscho/git/actions/runs/2477526645 (your link\nabove). So that's good.\n\nBut re my earlier comment where I asked/wondered if fixing that would\nlink to the source file at line 53 it still seems to just link to the\ndiff.\n\nIs that a bug? The desired result? If the commit was modifying syslog.c\nwould the link work?\n\nClearly an end result where we link to the source file/lines at the rev\nwe're testing is much more useful.\n\nI found this discussion:\nhttps://github.community/t/are-github-actions-notice-warning-error-annotations-broken/225674\n\nWhich has a link to an example run at:\nhttps://github.com/IronTooch-ColdStorage/Github-AnnotationTest/actions/runs/1782265048\n\nSo isn't this for creating \"annotations\" for just the regions that would\nbe involved in your diff? I.e. it shows a notice for the line(s)\ninvolved in the diff itself, but presumably nothing else?\n\nIf that's the case I think it would be much more useful to just\ne.g. wrap $(CC) in some \"tee\"-like command to spew its output somewhere,\nand then have a \"step\" where we extract the warnings/errors emitted, and\nemit URLs you could click on, unless there's some way to make the GitHub\nUX emit the same information.\n\nI.e. it'll be quite hit & miss whether the annotation will show up in\nthe diff, the compiler will often warn about a line some distance away\nfrom the change made, e.g. if a variable is made unused.\n\nUnless the intent is only to aggregate them on the summary page, but\nthen why do we need to link to the \"line\" at all, which will at best\nwork unreliably, and at worst be actively misleading.\n\nIn any case, needing to do less reading of the tea leaves would be nice,\ni.e. if the commit message explain what the desired change is exactly,\nand how it should be handling these cases.\n\nThanks.\n"}]}