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

Fix _MergedWrapperMarker to not include markers without corresponding execution scripts #85476

Merged

Conversation

BruceForstall
Copy link
Member

Don't include markers without execution scripts

Don't include markers without execution scripts
@dotnet-issue-labeler 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 Apr 27, 2023
@ghost ghost assigned BruceForstall Apr 27, 2023
@BruceForstall
Copy link
Member Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall BruceForstall added area-Infrastructure-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 27, 2023
@ghost
Copy link

ghost commented Apr 27, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Don't include markers without execution scripts

Author: BruceForstall
Assignees: BruceForstall
Labels:

area-Infrastructure-coreclr, needs-area-label

Milestone: -

@BruceForstall BruceForstall changed the title Try to fix _MergedWrapperMarker Fix _MergedWrapperMarker to not include markers without corresponding execution scripts Apr 27, 2023
@BruceForstall
Copy link
Member Author

This is a slight fix to #85441

@BruceForstall
Copy link
Member Author

I verified that the only groups missing from a Pri-0 Linux-x64 test run now are:

HardwareIntrinsics_Arm_r
HardwareIntrinsics_Arm_ro

which is what we want, since they were the ones failing before.

@BruceForstall
Copy link
Member Author

Failure in outerloop is #85081 (which is supposed to be disabled).

@ivdiazsa
Copy link
Member

I've a question I left in a comment, but otherwise it LGTM! Thanks for fixing this Bruce

@BruceForstall
Copy link
Member Author

fwiw, on a linux-x64 dev box you might build using:

	./build.sh -subset clr+libs -arch x64 -c checked
	./src/tests/build.sh checked x64

But on the CI, the build process is more like this (with many things happening on separate machines):

	./build.sh -subset clr.nativeprereqs -arch x64  -c checked
	./src/coreclr/build-runtime.sh checked x64 --keepnativesymbols
	./build.sh -subset tools+libs+libs.tests -configuration Debug -arch x64 -framework net8.0  -testscope innerloop /p:ArchiveTests=true
	./build.sh -subset clr.corelib+clr.nativecorelib+clr.nativeaotlibs+clr.tools+clr.packages+clr.paltestlist -arch x64 -c checked
	./src/tests/build.sh -log:Managed allTargets skipnative skipgeneratelayout skiptestwrappers checked x64 /p:TargetOS=AnyOS
	mkdir ~/gh/runtime/artifacts/tests/coreclr/linux.x64.Checked
	cp -r ~/gh/runtime/artifacts/tests/coreclr/AnyOS.x64.Checked/* ~/gh/runtime/artifacts/tests/coreclr/linux.x64.Checked
	./src/tests/build.sh skipmanaged skipgeneratelayout checked x64
	./src/tests/build.sh copynativeonly -log:Native checked x64 /p:LibrariesConfiguration=Debug
	./src/tests/build.sh buildtestwrappersonly -log:Wrappers checked x64 /p:LibrariesConfiguration=Debug
	./src/tests/build.sh generatelayoutonly -log:Layout checked x64 /p:LibrariesConfiguration=Debug

@BruceForstall BruceForstall merged commit 0b9ff15 into dotnet:main Apr 27, 2023
@BruceForstall BruceForstall deleted the FixMergedWrapperMarkerGroup branch April 27, 2023 22:51
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for landing the real fix Bruce!

@ghost ghost locked as resolved and limited conversation to collaborators Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants