Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build and Static Code Analysis not showing the offending file and line for submodules (OSOE-874) #365

Closed
Piedone opened this issue Jul 2, 2024 · 8 comments · Fixed by #366
Assignees
Labels
bug Something isn't working

Comments

@Piedone
Copy link
Member

Piedone commented Jul 2, 2024

If the build finds a code analysis error, then you'll see something like this in the workflow output:

image

Note how no indication is given where the analyzer violation is coming from. The file and line should be shown, as it is when you simply run dotnet build.

This happens if the offending code is not part of the repo, i.e. it's in a submodule.

Further info (from a Teams convo) by @sarahelsaig:

GitHub automatically converts the MSBuild style output from dotnet build into workflow messages. This used to work fine, though the link on the message box was broken. Then they changed something so now if the file doesn't exist in the current repo (e.g. it's in a submodule) the summary message won't show up at all.

I think the only possible way to fix this is to pipe the output ofdotnet build through a script that does the conversion into ::error file={name},line={line},endLine={endLine},title={title}::{message} format ourselves (you can decide whether to include the file part by checking the path withgit ls-files --error-unmatch filename). This seems like a lot of effort though, when you could just run static code analysis from the command line locally. Also you can download the binlog file from the Summary page in the Artifacts section. That contains the error and all of its context.

Jira issue

@Piedone Piedone added the bug Something isn't working label Jul 2, 2024
@github-actions github-actions bot changed the title Bulid and Static Code Analysis not showing the offending file and line for submodules Bulid and Static Code Analysis not showing the offending file and line for submodules (OSOE-874) Jul 2, 2024
@Piedone
Copy link
Member Author

Piedone commented Jul 2, 2024

I think such formatting is already supposed to happen:

dotnet build $SolutionOrProject @buildSwitches 2>&1 | ForEach-Object {
Maybe it's just that the format indeed changed.

Running dotnet build Lombiq.Tenants.Core.sln --no-incremental -warnaserror /p:TreatWarningsAsErrors=true /p:RunAnalyzersDuringBuild=true -nologo -consoleLoggerParameters:NoSummary -verbosity:quiet yields this for a different commit in the same codebase:

E:\Projects\Lombiq\Lombiq.Tenants\Lombiq-Tenants-Core1\test\Lombiq.Tenants.Core.Tests.UI\Tests\LombiqDotComTests\VisualVerificationTests\VisualVerificationTests.cs(6,1): error IDE0005: Using directive is unnecessary. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0005) [E:\Projects\Lombiq\Lombiq.Tenants\Lombiq-Tenants-Core1\test\Lombiq.Tenants.Core.Tests.UI\Lombiq.Tenants.Core.Tests.UI.csproj]

I think we just need to change the regex. It still works with the above output, but apparently, something is different under the (Linux) runners of GHA. Because e.g. Regexr shows this working fine:

image

@Psichorex can you perhaps link other examples? @sarahelsaig is this something you'd like to work on?

@Psichorex
Copy link
Contributor

I think all of the failed workflows are gone now everything has passed. But we can easily repro this by making a simple intentional warning and commit it to this issue's branch for example. Everytime I got a warning in a submodule this no-info exit happened.

@Piedone
Copy link
Member Author

Piedone commented Jul 2, 2024

So you can confirm that this only affects code in submodules?

@Piedone Piedone changed the title Bulid and Static Code Analysis not showing the offending file and line for submodules (OSOE-874) Build and Static Code Analysis not showing the offending file and line for submodules (OSOE-874) Jul 2, 2024
@Psichorex
Copy link
Contributor

I can't say it's 100% just submodules. I can test it tomorrow. What I can tell is that I haven't seen an informative exit 1 for a long time now.

@Piedone
Copy link
Member Author

Piedone commented Jul 2, 2024

OK, thanks. I'm looking into this now.

@Piedone
Copy link
Member Author

Piedone commented Jul 2, 2024

It's not necessarily just submodules. This one is for a file directly in the repo: https://github.com/Lombiq/Open-Source-Orchard-Core-Extensions/actions/runs/9767321918/job/26962333964#step:7:168

@Psichorex
Copy link
Contributor

Then I remembered correctly because I had OSOE warnings but I couldnt recall info.

@Piedone
Copy link
Member Author

Piedone commented Jul 2, 2024

Some investigation notes:

  • The file, line, column are missing for both submodule changes and the ones directly in the repo.
  • Using a relative file path for a non-submodule file, as the PR diff view also displays them (e.g. "test/Lombiq.OSOCE.Tests.UI/UITestBase.cs") instead of the full physical path (e.g. "/home/runner/work/Open-Source-Orchard-Core-Extensions/Open-Source-Orchard-Core-Extensions/test/Lombiq.OSOCE.Tests.UI/UITestBase.cs") doesn't help: the workflow output still doesn't include the file path/line/col, nor does the annotation show up on the diff view.
  • Using a relative file path and setting the title, what is supposed to be required doesn't help either.
  • Using debug logging doesn't show any information about the annotations.
  • The generated ::error:: is actually correct, as seen in this output.
  • Simply not doing anything with the dotnet build output yields errors showing up in the workflow summary, like for Orchard Core. We don't actually need any of the current ceremony, except for expected errors, but that can be handled on its own. Doing this indeed shows proper errors, though neither PR diff annotations, nor errors listed in the workflow summary. This also works with submodules, see here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants