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

Add linux-musl entry for FALLBACK_HOST_RID #65339

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

am11
Copy link
Member

@am11 am11 commented Feb 14, 2022

Fixes #65152

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 14, 2022
@ghost
Copy link

ghost commented Feb 14, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #65152

Author: am11
Assignees: -
Labels:

area-AssemblyLoader-coreclr

Milestone: -

@@ -70,6 +70,11 @@ if(CLR_CMAKE_HOST_OS STREQUAL Linux)
COMMAND bash -c "source ${LINUX_ID_FILE} && echo \$ID"
OUTPUT_VARIABLE CLR_CMAKE_LINUX_ID
OUTPUT_STRIP_TRAILING_WHITESPACE)

execute_process(
COMMAND bash -c "if strings \"${CMAKE_SYSROOT}/usr/bin/ldd\" 2>&1 | grep -q musl; then echo musl; fi"
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @janvorli, @vitek-karas, I have used same detection mechanism as we have used in:

if "${rootfsDir}/usr/bin/ldd" --version 2>&1 | grep -q musl ||
strings "${rootfsDir}/usr/bin/ldd" 2>&1 | grep -q musl; then
which was also ported to https://github.com/dotnet/install-scripts/blob/11b4eebe23d871c074364940d301c3eb53e7c7ec/src/dotnet-install.sh#L188

as fragile as it seems, it just works.

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

Looks good, but I definitely no expert in the CMake magic - so let's wait on @janvorli to take a look as well.

/cc @VSadov

@vitek-karas
Copy link
Member

Actually - spoke too soon. The version-less windows RID breaks some tests. I looked at one of them (PortableComponentOnSelfContainedAppRidAssetResolution) and it's basically the test depending on the specific RID we fallback to. It should be pretty straightforward to modify the test to expect the new behavior (or change the setup of the test to accommodate the change).

That said - maybe we should split these changes into two different PRs.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

The change looks good, but I leave the fallback rid resolution decision to the host folks.

@vitek-karas
Copy link
Member

@am11 could you please remove the win RID change and let's merge the linux-musl part on its own.
I'm fine with the win change as well, but it will need some test tweaks, so let's not block on that for now.

@am11
Copy link
Member Author

am11 commented Feb 15, 2022

@vitek-karas, I have removed the win commit for later time.

BTW, looks like we need to unescape linefeeds in failing test logs:
image

@vitek-karas
Copy link
Member

I checked offline with others that all the failures in the CI run are unrelated (and mostly known problems which are being worked on).

@vitek-karas
Copy link
Member

Re escaping newlines - I haven't looked into that yet, but I suspect it's some weird interaction with the testing framework.

@vitek-karas vitek-karas merged commit c1dba41 into dotnet:main Feb 16, 2022
@vitek-karas
Copy link
Member

Thanks a lot @am11!

@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-AssemblyLoader-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading issue when RID is linux-musl-x64 on Alpine 3.15
3 participants