NativeAOT the XUnitLogChecker #103795

agocke · 2024-06-21T05:54:42Z

This breaks the dependency on having dotnet installed.

dotnet-policy-service · 2024-06-22T07:20:24Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

carlossanlop

Thanks for submitting this change, it's looking good so far. Left some questions.

Don't forget to run the nativeaot azp run if the regular run looks good.

src/tests/Common/XUnitLogChecker/XUnitLogChecker.csproj

carlossanlop · 2024-06-24T22:45:22Z

src/libraries/tests.proj

@@ -762,7 +762,8 @@
  <ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(IsXUnitLogCheckerSupported)' == 'true'">
    <ProjectReference
      Include="$(RepoRoot)src\tests\Common\XUnitLogChecker\XUnitLogChecker.csproj"
-      AdditionalProperties="%(AdditionalProperties);Configuration=Release;OutDir=$(XUnitLogCheckerLibrariesOutDir)" />
+      Targets="Publish"
+      AdditionalProperties="%(AdditionalProperties);_IsPublishing=true;Configuration=Release;PublishDir=$(XUnitLogCheckerLibrariesOutDir)" />


For my own education, why is _IsPublishing being passed? Why isn't int sufficient to specify Targets="Publish" here?

This is tricky. In short, _IsPublishing is required for publishing, but not set when you're using msbuild /t:publish, only when you're doing dotnet publish.

src/tests/Common/Coreclr.TestWrapper/Coreclr.TestWrapper.csproj

dotnet-policy-service · 2024-07-25T02:17:32Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

agocke · 2024-08-24T01:47:03Z

I think this is ready for review.

agocke · 2024-08-26T18:27:35Z

@carlossanlop Would appreciate another review when you get a chance.

Also @dotnet/runtime-infrastructure

agocke · 2024-08-29T17:59:45Z

ping @carlossanlop

carlossanlop

I left a couple minor suggestions, but otherwise it LGTM. Before merging, could you execute a few more /azp runs? There were many failures that never showed up in the default CI legs. Is there any you could think of that could potentially show an edge-case behavior?

eng/testing/RunnerTemplate.cmd

eng/testing/RunnerTemplate.sh

carlossanlop · 2024-08-29T18:35:46Z

src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs

+            var crashReport = JsonNode.Parse(contents)!;
+            var threads = (JsonArray)crashReport["payload"]!["threads"]!;


Hopefully we will not see null objects where the ! was added.

Correct. And if we do, we would throw an exception anyway, so I figure any further checking is a waste.

carlossanlop · 2024-08-29T18:37:03Z

src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs

            int timeout = environmentVar != null ? int.Parse(environmentVar) : DEFAULT_TIMEOUT_MS;
            bool collectCrashDumps = Environment.GetEnvironmentVariable(COLLECT_DUMPS_ENVIRONMENT_VAR) != null;
-            string crashDumpFolder = Environment.GetEnvironmentVariable(CRASH_DUMP_FOLDER_ENVIRONMENT_VAR);
+            string? crashDumpFolder = Environment.GetEnvironmentVariable(CRASH_DUMP_FOLDER_ENVIRONMENT_VAR);


For these two env vars and the ones in the other files, should we throw if they're not found?

carlossanlop · 2024-08-29T18:39:56Z

src/tests/Common/helixpublishwitharcade.proj

@@ -401,7 +400,7 @@
      <XUnitLogCheckerArgs Condition="'$(TestWrapperTargetsWindows)' != 'true'">$(XUnitLogCheckerArgs) %24HELIX_DUMP_FOLDER</XUnitLogCheckerArgs>
      <XUnitLogCheckerArgs Condition="'$(TestWrapperTargetsWindows)' == 'true'">$(XUnitLogCheckerArgs) %25HELIX_DUMP_FOLDER%25</XUnitLogCheckerArgs>

-      <XUnitLogCheckerCommand>dotnet $(XUnitLogCheckerHelixPath)XUnitLogChecker.dll $(XUnitLogCheckerArgs)</XUnitLogCheckerCommand>
+      <XUnitLogCheckerCommand>$(XUnitLogCheckerHelixPath)XUnitLogChecker$(ExeSuffix) $(XUnitLogCheckerArgs)</XUnitLogCheckerCommand>


So you're now putting XUnitLogChecker.exe in the root helix path. Nice.

carlossanlop · 2024-08-29T18:45:12Z

src/libraries/tests.proj

@@ -767,7 +767,8 @@
  <ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(IsXUnitLogCheckerSupported)' == 'true'">
    <ProjectReference
      Include="$(RepoRoot)src\tests\Common\XUnitLogChecker\XUnitLogChecker.csproj"
-      AdditionalProperties="%(AdditionalProperties);Configuration=Release;OutDir=$(XUnitLogCheckerLibrariesOutDir)" />
+      Targets="Publish"
+      AdditionalProperties="%(AdditionalProperties);_IsPublishing=true;Configuration=Release;PublishDir=$(XUnitLogCheckerLibrariesOutDir);ROOTFS_DIR=$(ROOTFS_DIR);RuntimeIdentifier=$(OutputRID)" />


Correct me if I'm wrong: the 'Runtime Identifier' additional property here is the equivalent of using the CLI command argument -r <rid>?

Yup, that's correct

carlossanlop · 2024-08-29T18:46:31Z

src/tests/build.proj

-      Targets="Build" />
+      Targets="Publish"
+      Properties="_IsPublishing=true;Configuration=Release;PublishDir=$(XunitTestBinBase)/XUnitLogChecker;RuntimeIdentifier=$(RuntimeIdentifier)"
+      Condition="$(IsXUnitLogCheckerSupported)" />


This is not on you obviously, but I wish we had standardized names for arguments like the RID. In tests.proj, it's OutputRID, and here it is RuntimeIdentifier.

Yeah, I'm not sure what the reasoning is here. I would have to do some archeology to figure out our naming convention on this.

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>

jakobbotsch · 2024-08-31T15:53:16Z

@agocke The Fuzzlyn pipeline run is failing on Linux after this PR (not 100% sure it's because of this PR, but seems related?): https://dev.azure.com/dnceng-public/public/_build/results?buildId=794868&view=results

2024-08-31T14:38:39.3302256Z   XUnitLogChecker -> /__w/1/s/artifacts/tests/coreclr/linux.x64.Release/Common/XUnitLogChecker/XUnitLogChecker/XUnitLogChecker.dll
2024-08-31T14:38:39.3696459Z   Generating native code
2024-08-31T14:38:52.0982028Z clang : error : invalid linker name in argument '-fuse-ld=bfd' [/__w/1/s/src/tests/Common/XUnitLogChecker/XUnitLogChecker.csproj]
2024-08-31T14:38:52.2613950Z /home/cloudtest_azpcontainer/.nuget/packages/microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/build/Microsoft.NETCore.Native.targets(368,5): error MSB3073: The command ""clang" "/__w/1/s/artifacts/tests/coreclr/obj/linux.x64.Release/Managed/Common/XUnitLogChecker/XUnitLogChecker/native/XUnitLogChecker.o" -o "/__w/1/s/artifacts/tests/coreclr/linux.x64.Release/Common/XUnitLogChecker/XUnitLogChecker/native/XUnitLogChecker" -Wl,--version-script=/__w/1/s/artifacts/tests/coreclr/obj/linux.x64.Release/Managed/Common/XUnitLogChecker/XUnitLogChecker/native/XUnitLogChecker.exports -Wl,--export-dynamic -gz=zlib -fuse-ld=bfd /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libbootstrapper.o /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libRuntime.WorkstationGC.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libeventpipe-disabled.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libRuntime.VxsortEnabled.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libstandalonegc-disabled.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/sdk/libstdc++compat.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/framework/libSystem.Native.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/framework/libSystem.Globalization.Native.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/framework/libSystem.IO.Compression.Native.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/framework/libSystem.Net.Security.Native.a /home/cloudtest_azpcontainer/.nuget/packages/runtime.linux-x64.microsoft.dotnet.ilcompiler/9.0.0-preview.7.24405.7/framework/libSystem.Security.Cryptography.Native.OpenSsl.a -g -Wl,-rpath,'$ORIGIN' -Wl,--build-id=sha1 -Wl,--as-needed -pthread -ldl -lz -lrt -lm -pie -Wl,-pie -Wl,-z,relro -Wl,-z,now -Wl,--eh-frame-hdr -Wl,--discard-all -Wl,--gc-sections" exited with code 1. [/__w/1/s/src/tests/Common/XUnitLogChecker/XUnitLogChecker.csproj]

Any idea what needs to be done?

agocke · 2024-08-31T16:24:06Z

Looks like the bfd linker isn’t installed on that machine. We can use lld instead by setting linker flavor. We just need to find the conditions in which it needs to be set. Local builds often have bfd so that’s the default. Maybe we can condition on running in ci.

jakobbotsch · 2024-08-31T18:49:19Z

I think perhaps the problem is that we should be passing -cross to src/tests/build.sh in the pipeline. Let me try that out.

am11 · 2024-09-03T06:54:14Z

Looks like the bfd linker isn’t installed on that machine.

bfd is installed, which is for per-arch and it is installed for the host arch, and lld is not installed (which is multi arch). That's the problem. We added UsePublishedCrossgen2=false bailout property for similar reasons in running running test infra. Or we could just install lld by running eng/install-native-dependencies.sh script in the pipeline (with sudo if $USER is not root).

agocke · 2024-09-03T14:45:54Z

lld is not installed

~~Yes, sorry, that’s what I meant.~~

Actually, this is using the cross build image now so lld should be installed, no?

am11 · 2024-09-03T16:22:50Z

Actually I came here from Jan’s PR, which indicates superpmi test leg isn’t using the container https://dev.azure.com/dnceng-public/public/_build/results?buildId=795908&view=logs&jobId=b65b05e1-c985-593c-6ffc-1fb9e008884e&j=b65b05e1-c985-593c-6ffc-1fb9e008884e&t=a3a60a19-d0c2-5a1d-1fde-5ca19d3821ea

not sure if lld is installed on that host VM.

This breaks the dependency on having dotnet installed when running tests.

NativeAOT the XUnitLogChecker

e3b702f

This breaks the dependency on having dotnet installed.

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 21, 2024

dotnet-policy-service bot assigned agocke Jun 21, 2024

This was referenced Jun 21, 2024

GC/Regressions/v2.0-beta2/452950 failed in CI #103494

Closed

FileSystemWatcher_InternalBufferSize_SynchronizingObject test failed in CI #103495

Closed

agocke added 2 commits June 21, 2024 11:11

Add LinkerFlavor=lld in the case that we're doing a cross build

44a6ae1

Use lld for xunitlogchecker

1c6e909

This was referenced Jun 22, 2024

Failed USB connection via port 54050, error 61, in tvOS arm64 Release AllSubsets_Mono #82637

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

agocke added 5 commits June 21, 2024 17:08

Try removing log checker entirely

222f40e

Publish instead of build

8da3453

Try to publish to parent directory

eac9c40

Force restore for Xunitlogchecker

f98ee2f

Include RID in properties passed to publish

6f6d23a

am11 added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 22, 2024

Remove subdir

fdd5f11

carlossanlop reviewed Jun 24, 2024

View reviewed changes

dotnet-policy-service bot closed this Jul 25, 2024

carlossanlop reopened this Jul 25, 2024

agocke and others added 3 commits August 20, 2024 19:26

Merge branch 'main' into aot-logchecker

27ecb7b

Fix cross building

c8fb8a8

Skip building xunitlogchecker if not supported

1640b4e

Only include in helix payload if supported

7555fa2

build-analysis bot mentioned this pull request Aug 23, 2024

XmlSerializerTests.Xml_DerivedIXmlSerializable is failing on linux-musl legs #106865

Closed

Print XUnitLogCheckerSupported value

75e73dd

build-analysis bot mentioned this pull request Aug 23, 2024

System.Threading.ThreadPool.Tests fail in CI with System.Net.Sockets.SocketException #106371

Open

Check IsXunitLogCheckerSupported in helixpublish script

08791d0

agocke marked this pull request as ready for review August 23, 2024 23:09

carlossanlop approved these changes Aug 29, 2024

View reviewed changes

agocke and others added 2 commits August 29, 2024 13:47

Update eng/testing/RunnerTemplate.sh

8de2c1d

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>

Update eng/testing/RunnerTemplate.cmd

f10a4e3

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>

agocke merged commit 2297f60 into dotnet:main Aug 30, 2024
151 checks passed

agocke deleted the aot-logchecker branch August 30, 2024 19:14

jakobbotsch mentioned this pull request Aug 31, 2024

Pass $(crossArg) to src/tests/build.sh in Antigen/Fuzzlyn pipelines #107229

Merged

agocke mentioned this pull request Sep 11, 2024

Should we AOT the XunitLogChecker tool? #94270

Closed

jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024

NativeAOT the XUnitLogChecker (dotnet#103795)

9c3af14

This breaks the dependency on having dotnet installed when running tests.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NativeAOT the XUnitLogChecker #103795

NativeAOT the XUnitLogChecker #103795

agocke commented Jun 21, 2024

dotnet-policy-service bot commented Jun 22, 2024

carlossanlop left a comment

carlossanlop Jun 24, 2024

agocke Jun 24, 2024

dotnet-policy-service bot commented Jul 25, 2024

agocke commented Aug 24, 2024

agocke commented Aug 26, 2024

agocke commented Aug 29, 2024

carlossanlop left a comment

carlossanlop Aug 29, 2024

agocke Aug 29, 2024

carlossanlop Aug 29, 2024

carlossanlop Aug 29, 2024

carlossanlop Aug 29, 2024

agocke Aug 29, 2024

carlossanlop Aug 29, 2024

agocke Aug 29, 2024

jakobbotsch commented Aug 31, 2024

agocke commented Aug 31, 2024

jakobbotsch commented Aug 31, 2024

am11 commented Sep 3, 2024 •

edited

Loading

agocke commented Sep 3, 2024 •

edited

Loading

am11 commented Sep 3, 2024

		var crashReport = JsonNode.Parse(contents)!;
		var threads = (JsonArray)crashReport["payload"]!["threads"]!;

NativeAOT the XUnitLogChecker #103795

NativeAOT the XUnitLogChecker #103795

Conversation

agocke commented Jun 21, 2024

dotnet-policy-service bot commented Jun 22, 2024

carlossanlop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-policy-service bot commented Jul 25, 2024

agocke commented Aug 24, 2024

agocke commented Aug 26, 2024

agocke commented Aug 29, 2024

carlossanlop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobbotsch commented Aug 31, 2024

agocke commented Aug 31, 2024

jakobbotsch commented Aug 31, 2024

am11 commented Sep 3, 2024 • edited Loading

agocke commented Sep 3, 2024 • edited Loading

am11 commented Sep 3, 2024

am11 commented Sep 3, 2024 •

edited

Loading

agocke commented Sep 3, 2024 •

edited

Loading