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 singlefilehost build in non-portable mode #42415

Merged
1 commit merged into from
Sep 23, 2020

Conversation

omajid
Copy link
Member

@omajid omajid commented Sep 17, 2020

The singlefilehost needs to follow libraries build in terms of how it links to OpenSSL: if it's a non-portable build, the singlefilehost needs to link to OpenSSL via linker arguments.

The installer also needs to have FEATURE_DISTRO_AGNOSTIC_SSL defined just like it is defined for the libraries build.

Fixes: #41768

@ghost
Copy link

ghost commented Sep 17, 2020

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

@ghost
Copy link

ghost commented Sep 17, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@omajid
Copy link
Member Author

omajid commented Sep 18, 2020

Should I look into adding a CI configuration that exercises runtime build in non-portable mode to catch such regressions earlier for next time around?

@omajid omajid marked this pull request as ready for review September 18, 2020 12:10
@dagood
Copy link
Member

dagood commented Sep 18, 2020

Should I look into adding a CI configuration that exercises runtime build in non-portable mode to catch such regressions earlier for next time around?

We're planning to do this with arcade-powered source-build. The plan is end-to-end, even--so it also validates dotnet/installer being able to take in a non-portable RID, etc. There's a bit more info at dotnet/source-build#1722.

That said, getting a non-portable config into dotnet/runtime would still be useful in the short term, because we obviously don't have arcade-powered source-build yet. 🙂

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.

LGTM, thank you!

@@ -191,6 +191,12 @@ else()
# )
endif ()

# Additional requirements for System.Security.Cryptography.Native.OpenSsl
message("FEATURE_DISTRO_AGNOSTIC_SSL: ${FEATURE_DISTRO_AGNOSTIC_SSL}")
Copy link
Member

Choose a reason for hiding this comment

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

A nit - can you please remove this message?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

The singilefilehost needs to follow the libraries build in terms of how
it links to OpenSSL: if it's a non-portable build, the singlefilehost
needs to link to OpenSSL via linker arguments.

The installer also needs to have FEATURE_DISTRO_AGNOSTIC_SSL defined
just like it is defined for the libraries build.

Fixes: dotnet#41768
@omajid omajid force-pushed the singlefilehost-non-portable-ssl branch from 7b213c3 to 3d325d4 Compare September 21, 2020 15:09
@omajid
Copy link
Member Author

omajid commented Sep 21, 2020

Thanks for the quick review and merging this!

@janvorli
Copy link
Member

I have noticed that two test legs have failed. I have restarted them.

@ghost
Copy link

ghost commented Sep 23, 2020

Hello @janvorli!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit eac3423 into dotnet:master Sep 23, 2020
omajid added a commit to omajid/dotnet-source-build that referenced this pull request Sep 23, 2020
Backport dotnet/runtime#42415 and carry it as a
patch for now.
omajid added a commit to omajid/dotnet-source-build that referenced this pull request Sep 23, 2020
Backport dotnet/runtime#42415 and carry it as a
patch for now.
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
This pull request was closed.
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.

Source-build OpenSSL linker errors
5 participants