Skip to content

Conversation

@am11
Copy link
Member

@am11 am11 commented Feb 14, 2022

Fixes #65152

@ghost ghost added area-AssemblyLoader-coreclr community-contribution Indicates that the PR has been added by a community member labels 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: -

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 am11 force-pushed the feature/host/linux-musl branch from 6fa61b4 to 3858aa7 Compare February 15, 2022 15:10
@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