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

Make dependency on lld optional #85667

Merged
merged 7 commits into from
May 4, 2023
Merged

Make dependency on lld optional #85667

merged 7 commits into from
May 4, 2023

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented May 2, 2023

Fixes #84934. This detects availability of lld similarly to how we do it for the native components in init-compiler.sh:

if "$CC" -fuse-ld=lld -Wl,--version >/dev/null 2>&1; then
LDFLAGS="-fuse-ld=lld"
fi

I tested this in ci (see third commit) by temporarily running the runtime-dev-innerloop pipeline on an Ubuntu 22.04 image I uploaded. After this goes in I will update the dotnet-buildtools-prereqs image to remove lld and then use it in runtime-dev-innerloop.

@ghost
Copy link

ghost commented May 2, 2023

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

Issue Details

null

Author: sbomer
Assignees: sbomer
Labels:

area-Infrastructure-coreclr

Milestone: -

@sbomer sbomer marked this pull request as ready for review May 3, 2023 00:51
@sbomer sbomer requested review from am11, agocke and jkotas May 3, 2023 00:52
@@ -570,4 +569,28 @@
Condition="'$(_WillCLRTestProjectBuild)' == 'true' and '$(CLRTestKind)' == 'BuildAndRun'"
DependsOnTargets="ComputeResolvedFilesToPublishList;LinkNative" />

<Target Name="LocateNativeCompiler"
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be a lot of duplication of this logic. Would it be possible to have this logic in a shared file under eng and import it in all places that need it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah - I've been meaning to share this and other logic that is already duplicated. There are some subtle differences, so I wanted to see if I could get it working first.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to work on the sharing in a follow-up if that sounds ok to you. This fix will unblock some other things I can work on in parallel.

@sbomer sbomer requested a review from jkotas May 3, 2023 22:49
@sbomer sbomer merged commit 34f9ba6 into dotnet:main May 4, 2023
sbomer added a commit to dotnet/dotnet-buildtools-prereqs-docker that referenced this pull request May 4, 2023
This is no longer required with
dotnet/runtime#85667, and will let us switch
to the Ubuntu 22.04 images for the runtime-dev-innerloop runs.
@sbomer sbomer deleted the lldOptional branch May 11, 2023 18:34
@ghost ghost locked as resolved and limited conversation to collaborators Jun 10, 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.

Make dependency on lld optional during runtime build
2 participants