From f2ee179123e44c2c4acaa42a491b1c439e536928 Mon Sep 17 00:00:00 2001 From: Ivan Diaz Date: Thu, 2 Nov 2023 14:08:58 -0700 Subject: [PATCH 1/6] Successfully replaced pushd/popd with CMake's -S/-B in gen-buildsys.sh for native and clr. Now, looking into removing other instances if possible. --- eng/native/gen-buildsys.sh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/eng/native/gen-buildsys.sh b/eng/native/gen-buildsys.sh index b39448626fb3a..ffb3c43a9c660 100755 --- a/eng/native/gen-buildsys.sh +++ b/eng/native/gen-buildsys.sh @@ -104,9 +104,6 @@ if [[ "$host_arch" == "wasm" ]]; then fi fi -# We have to be able to build with CMake 3.6.2, so we can't use the -S or -B options -pushd "$2" - $cmake_command \ --no-warn-unused-cli \ -G "$generator" \ @@ -114,6 +111,6 @@ $cmake_command \ "-DCMAKE_INSTALL_PREFIX=$__CMakeBinDir" \ $cmake_extra_defines \ $__UnprocessedCMakeArgs \ - "$1" + -S "$1" \ + -B "$2" -popd From c0635f4359e23558f29acaf47b2a920aab89b5b6 Mon Sep 17 00:00:00 2001 From: Ivan Diaz Date: Thu, 2 Nov 2023 15:20:59 -0700 Subject: [PATCH 2/6] Removed redundant pushd/popd in src/native/libs/build-native.cmd --- src/native/libs/build-native.cmd | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/native/libs/build-native.cmd b/src/native/libs/build-native.cmd index b1a23666457a7..13e7334a73c53 100644 --- a/src/native/libs/build-native.cmd +++ b/src/native/libs/build-native.cmd @@ -102,10 +102,8 @@ echo %MSBUILD_EMPTY_PROJECT_CONTENT% > "%__artifactsDir%\obj\native\Directory.Bu :: Regenerate the VS solution -pushd "%__IntermediatesDir%" call "%__repoRoot%\eng\native\gen-buildsys.cmd" "%__sourceRootDir%" "%__IntermediatesDir%" %__VSVersion% %__BuildArch% %__TargetOS% %__ExtraCmakeParams% if NOT [%errorlevel%] == [0] goto :Failure -popd :BuildNativeProj :: Build the project created by Cmake From 01ec2a23c5792bbdc8ebfaff4b59be32054db7b2 Mon Sep 17 00:00:00 2001 From: Ivan Diaz Date: Fri, 3 Nov 2023 10:48:03 -0700 Subject: [PATCH 3/6] Experimenting with removing 'pushd/popd' for Wasm/Wasi. --- eng/native/build-commons.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/eng/native/build-commons.sh b/eng/native/build-commons.sh index 1cd4ea0ed1231..0775ad861d42f 100755 --- a/eng/native/build-commons.sh +++ b/eng/native/build-commons.sh @@ -207,14 +207,14 @@ build_native() local exit_code if [[ "$__StaticAnalyzer" == 1 ]]; then - pushd "$intermediatesDir" + # pushd "$intermediatesDir" buildTool="$SCAN_BUILD_COMMAND -o $__BinDir/scan-build-log $buildTool" echo "Executing $buildTool $target -j $__NumProc" "$buildTool" $target -j "$__NumProc" exit_code="$?" - popd + # popd else cmake_command=cmake if [[ "$build_arch" == "wasm" && "$__TargetOS" == "browser" ]]; then @@ -225,13 +225,13 @@ build_native() else # For non-wasm Unix scenarios, we may have to use an old version of CMake that doesn't support # multiple targets. Instead, directly invoke the build tool to build multiple targets in one invocation. - pushd "$intermediatesDir" + # pushd "$intermediatesDir" echo "Executing $buildTool $target -j $__NumProc" "$buildTool" $target -j "$__NumProc" exit_code="$?" - popd + # popd fi fi From a4ee72f310f9b50b02bfd3fda4e159d8a9ec6808 Mon Sep 17 00:00:00 2001 From: Ivan Diaz Date: Fri, 3 Nov 2023 13:31:00 -0700 Subject: [PATCH 4/6] Restored pushd/popd to eng/native/build-commons.sh because it is actually not directly related to CMake. --- eng/native/build-commons.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/eng/native/build-commons.sh b/eng/native/build-commons.sh index 0775ad861d42f..1cd4ea0ed1231 100755 --- a/eng/native/build-commons.sh +++ b/eng/native/build-commons.sh @@ -207,14 +207,14 @@ build_native() local exit_code if [[ "$__StaticAnalyzer" == 1 ]]; then - # pushd "$intermediatesDir" + pushd "$intermediatesDir" buildTool="$SCAN_BUILD_COMMAND -o $__BinDir/scan-build-log $buildTool" echo "Executing $buildTool $target -j $__NumProc" "$buildTool" $target -j "$__NumProc" exit_code="$?" - # popd + popd else cmake_command=cmake if [[ "$build_arch" == "wasm" && "$__TargetOS" == "browser" ]]; then @@ -225,13 +225,13 @@ build_native() else # For non-wasm Unix scenarios, we may have to use an old version of CMake that doesn't support # multiple targets. Instead, directly invoke the build tool to build multiple targets in one invocation. - # pushd "$intermediatesDir" + pushd "$intermediatesDir" echo "Executing $buildTool $target -j $__NumProc" "$buildTool" $target -j "$__NumProc" exit_code="$?" - # popd + popd fi fi From 360a3dbdff14eb0c7c5d2e4aae74dcea2e18d396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20K=C3=B6plinger?= Date: Wed, 29 Nov 2023 17:11:11 +0100 Subject: [PATCH 5/6] Add explicit exit code to gen-buildsys.sh Make sure we don't forget to return the cmake exist code, see https://github.com/dotnet/runtime/pull/95408. --- eng/native/gen-buildsys.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/native/gen-buildsys.sh b/eng/native/gen-buildsys.sh index ffb3c43a9c660..521cfe10512df 100755 --- a/eng/native/gen-buildsys.sh +++ b/eng/native/gen-buildsys.sh @@ -114,3 +114,4 @@ $cmake_command \ -S "$1" \ -B "$2" +exit $? From 0659f6b512e40b0470db5dda2bc854e3f98d3e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20K=C3=B6plinger?= Date: Wed, 29 Nov 2023 17:13:54 +0100 Subject: [PATCH 6/6] Replace with comment instead --- eng/native/gen-buildsys.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/native/gen-buildsys.sh b/eng/native/gen-buildsys.sh index 521cfe10512df..4c1ed0943c45a 100755 --- a/eng/native/gen-buildsys.sh +++ b/eng/native/gen-buildsys.sh @@ -114,4 +114,4 @@ $cmake_command \ -S "$1" \ -B "$2" -exit $? +# don't add anything after this line so the cmake exit code gets propagated correctly