Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Improve System.Native build experience for new platforms#36926

Merged
stephentoub merged 1 commit into
dotnet:masterfrom
am11:sys.native/improvements
Apr 18, 2019
Merged

Improve System.Native build experience for new platforms#36926
stephentoub merged 1 commit into
dotnet:masterfrom
am11:sys.native/improvements

Conversation

@am11
Copy link
Copy Markdown
Member

@am11 am11 commented Apr 16, 2019

  1. From top-level script, we can pass (only*) one extra argument to build-native.sh via BuildNativeClang as MSBuild property; /p:BuildNativeClang. This PR renames it to BuildNativeCompiler so it doesn't look confusing when selecting GCC compiler: corefx/build.sh /p:BuildNativeCompiler=-gcc.
  2. When libssl devel package is not installed on the system, the default CMake's "library not found" message is unhelpful. This patch aligns the error handling of NOT OPENSSL_FOUND with those of KRB5 and CURL etc., so user knows which missing package to install.

* Currently there is no way to pass more than one arguments via BuildNativeCompiler, as they get stripped somewhere in eng directory in corefx/build.sh -> ... -> corefx/src/Native/build-native.sh call chain. For example, we cannot do:

./build.sh /p:BuildNativeCompiler="cmakeargs -DSOME_CLR_PROPERTY=SomeValue cmakeargs -DCMAKE_C_FLAGS=-march=native" release cross arm64

because a pass-through script strips away the quotes.

@am11
Copy link
Copy Markdown
Member Author

am11 commented Apr 16, 2019

/cc @janvorli, @safern, @ViktorHofer

@ViktorHofer
Copy link
Copy Markdown
Member

Thanks!

@am11
Copy link
Copy Markdown
Member Author

am11 commented Apr 17, 2019

Can this be merged? :)

@stephentoub stephentoub merged commit 65f07a9 into dotnet:master Apr 18, 2019
@am11 am11 deleted the sys.native/improvements branch April 18, 2019 09:13
@BruceForstall
Copy link
Copy Markdown
Contributor

@am11 @ViktorHofer @stephentoub Note that the renaming of BuildNativeClang broke our corefx-in-coreclr-CI testing, which uses this argument (for Linux arm32, where we must specify the clang version).

If possible, please remember to notify us for any future change to this to avoid breaking us again.

@am11
Copy link
Copy Markdown
Member Author

am11 commented Apr 19, 2019

Sorry about that @BruceForstall, I was only grep'ing the CoreFX repo. Maybe we can try sharing/centralizing them via arcade (if possible). :)

@safern
Copy link
Copy Markdown
Member

safern commented Apr 20, 2019

@BruceForstall sorry about that. Sure thing next time we will let you know next time.

@karelz karelz added this to the 3.0 milestone May 4, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants