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.

Sign up for GitHub

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

Remove bash dependency from init-compiler #11359

Merged
merged 4 commits into from
Dec 20, 2022
Merged

Remove bash dependency from init-compiler #11359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7d751bb
Remove bash dependency from init-compiler
am11 Oct 21, 2022
5ad6cc5
Remove dependency on pipeline-logging-functions.sh
am11 Oct 21, 2022
03e270b
Merge branch 'main' into feature/shell-scripts
am11 Dec 20, 2022
8969f8b
Update comment
am11 Dec 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 37 additions & 44 deletions eng/common/native/init-compiler.sh
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
#!/usr/bin/env bash
#!/bin/sh
#
# This file detects the C/C++ compiler and exports it to the CC/CXX environment variables
#
# NOTE: some scripts source this file and rely on stdout being empty, make sure to not output anything here!

if [[ "$#" -lt 3 ]]; then
if [ -z "$build_arch" ] || [ -z "$compiler" ]; then
echo "Usage..."
echo "init-compiler.sh <script directory> <Architecture> <compiler>"
echo "Specify the script directory."
echo "build_arch=<ARCH> compiler=<NAME> init-compiler.sh"
echo "Specify the target architecture."
echo "Specify the name of compiler (clang or gcc)."
exit 1
fi

nativescriptroot="$1"
build_arch="$2"
compiler="$3"

case "$compiler" in
clang*|-clang*|--clang*)
# clangx.y or clang-x.y
version="$(echo "$compiler" | tr -d '[:alpha:]-=')"
parts=(${version//./ })
majorVersion="${parts[0]}"
minorVersion="${parts[1]}"
if [[ -z "$minorVersion" && "$majorVersion" -le 6 ]]; then
majorVersion="${version%%.*}"
[ -z "${version##*.*}" ] && minorVersion="${version#*.}"

if [ -z "$minorVersion" ] && [ -n "$majorVersion" ] && [ "$majorVersion" -le 6 ]; then
minorVersion=0;
fi
compiler=clang
Expand All @@ -33,23 +28,20 @@ case "$compiler" in
gcc*|-gcc*|--gcc*)
# gccx.y or gcc-x.y
version="$(echo "$compiler" | tr -d '[:alpha:]-=')"
parts=(${version//./ })
majorVersion="${parts[0]}"
minorVersion="${parts[1]}"
majorVersion="${version%%.*}"
[ -z "${version##*.*}" ] && minorVersion="${version#*.}"
compiler=gcc
;;
esac

cxxCompiler="$compiler++"

. "$nativescriptroot"/../pipeline-logging-functions.sh
Copy link
Member

Choose a reason for hiding this comment

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

@dotnet/roslyn-infrastructure This is deleting eng system telemetry. Why was this telemetry added in the first place? Are we ok with deleting 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.

I think it was added to satisfy validation in eng/configure-toolset.sh, that has an ignore list which I have updated below.

Note that when we invoke build.sh in runtime and diagnostics repos, which call multiple scripts before reaching init-compiler.sh, we use plain echo or print commands in all those scripts. This script only has a few error cases, which we don't encounter in the CI, so I replaced Write-PipelineTelemetryError with echo.

Copy link
Member

Choose a reason for hiding this comment

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

We do not encounter these error cases in the CI when everything is working correctly. We may encounter these error cases in the CI on infrastructure updates or when things break for some reason. IIRC, it is why this telemetry was added in the first place - to allow eng system team to see that there is a problem in some repo after the update.

Copy link
Member Author

@am11 am11 Oct 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

When it was added, this script was only used by arcade cmake SDK. I deduplicated that code in dotnet/runtime#59018 just to avoid duplication and not for telemetry. The one invovled in the build had no stake in telemetry data. I don't think out of all the other possible errors in 20+ scripts involved in the build (which use echo), there is anything special about the few in this script.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I agree with @am11, the telemetry here is not super valuable, I've never actually seen it reporting a failure.

@am11 would you mind resolving the merge conflicts? I think this is good to go.


# clear the existing CC and CXX from environment
CC=
CXX=
LDFLAGS=

if [[ "$compiler" == "gcc" ]]; then cxxCompiler="g++"; fi
if [ "$compiler" = "gcc" ]; then cxxCompiler="g++"; fi

check_version_exists() {
desired_version=-1
Expand All @@ -66,74 +58,75 @@ check_version_exists() {
echo "$desired_version"
}

if [[ -z "$CLR_CC" ]]; then
if [ -z "$CLR_CC" ]; then

# Set default versions
if [[ -z "$majorVersion" ]]; then
if [ -z "$majorVersion" ]; then
# note: gcc (all versions) and clang versions higher than 6 do not have minor version in file name, if it is zero.
if [[ "$compiler" == "clang" ]]; then versions=( 15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5 )
elif [[ "$compiler" == "gcc" ]]; then versions=( 12 11 10 9 8 7 6 5 4.9 ); fi

for version in "${versions[@]}"; do
parts=(${version//./ })
desired_version="$(check_version_exists "${parts[0]}" "${parts[1]}")"
if [[ "$desired_version" != "-1" ]]; then majorVersion="${parts[0]}"; break; fi
if [ "$compiler" = "clang" ]; then versions="15 14 13 12 11 10 9 8 7 6.0 5.0 4.0 3.9 3.8 3.7 3.6 3.5"
elif [ "$compiler" = "gcc" ]; then versions="12 11 10 9 8 7 6 5 4.9"; fi

for version in $versions; do
_major="${version%%.*}"
[ -z "${version##*.*}" ] && _minor="${version#*.}"
desired_version="$(check_version_exists "$_major" "$_minor")"
if [ "$desired_version" != "-1" ]; then majorVersion="$_major"; break; fi
done

if [[ -z "$majorVersion" ]]; then
if [ -z "$majorVersion" ]; then
if command -v "$compiler" > /dev/null; then
if [[ "$(uname)" != "Darwin" ]]; then
Write-PipelineTelemetryError -category "Build" -type "warning" "Specific version of $compiler not found, falling back to use the one in PATH."
if [ "$(uname)" != "Darwin" ]; then
echo "Warning: Specific version of $compiler not found, falling back to use the one in PATH."
fi
CC="$(command -v "$compiler")"
CXX="$(command -v "$cxxCompiler")"
else
Write-PipelineTelemetryError -category "Build" "No usable version of $compiler found."
echo "No usable version of $compiler found."
exit 1
fi
else
if [[ "$compiler" == "clang" && "$majorVersion" -lt 5 ]]; then
if [[ "$build_arch" == "arm" || "$build_arch" == "armel" ]]; then
if [ "$compiler" = "clang" ] && [ "$majorVersion" -lt 5 ]; then
if [ "$build_arch" = "arm" ] || [ "$build_arch" = "armel" ]; then
if command -v "$compiler" > /dev/null; then
Write-PipelineTelemetryError -category "Build" -type "warning" "Found clang version $majorVersion which is not supported on arm/armel architectures, falling back to use clang from PATH."
echo "Warning: Found clang version $majorVersion which is not supported on arm/armel architectures, falling back to use clang from PATH."
CC="$(command -v "$compiler")"
CXX="$(command -v "$cxxCompiler")"
else
Write-PipelineTelemetryError -category "Build" "Found clang version $majorVersion which is not supported on arm/armel architectures, and there is no clang in PATH."
echo "Found clang version $majorVersion which is not supported on arm/armel architectures, and there is no clang in PATH."
exit 1
fi
fi
fi
fi
else
desired_version="$(check_version_exists "$majorVersion" "$minorVersion")"
if [[ "$desired_version" == "-1" ]]; then
Write-PipelineTelemetryError -category "Build" "Could not find specific version of $compiler: $majorVersion $minorVersion."
if [ "$desired_version" = "-1" ]; then
echo "Could not find specific version of $compiler: $majorVersion $minorVersion."
exit 1
fi
fi

if [[ -z "$CC" ]]; then
if [ -z "$CC" ]; then
CC="$(command -v "$compiler$desired_version")"
CXX="$(command -v "$cxxCompiler$desired_version")"
if [[ -z "$CXX" ]]; then CXX="$(command -v "$cxxCompiler")"; fi
if [ -z "$CXX" ]; then CXX="$(command -v "$cxxCompiler")"; fi
fi
else
if [[ ! -f "$CLR_CC" ]]; then
Write-PipelineTelemetryError -category "Build" "CLR_CC is set but path '$CLR_CC' does not exist"
if [ ! -f "$CLR_CC" ]; then
echo "CLR_CC is set but path '$CLR_CC' does not exist"
exit 1
fi
CC="$CLR_CC"
CXX="$CLR_CXX"
fi

if [[ -z "$CC" ]]; then
Write-PipelineTelemetryError -category "Build" "Unable to find $compiler."
if [ -z "$CC" ]; then
echo "Unable to find $compiler."
exit 1
fi

# Only lld version >= 9 can be considered stable. lld doesn't support s390x.
if [[ "$compiler" == "clang" && "$majorVersion" -ge 9 && "$build_arch" != "s390x" ]]; then
if [ "$compiler" = "clang" ] && [ -n "$majorVersion" ] && [ "$majorVersion" -ge 9 ] && [ "$build_arch" != "s390x" ]; then
if "$CC" -fuse-ld=lld -Wl,--version >/dev/null 2>&1; then
LDFLAGS="-fuse-ld=lld"
fi
Expand Down
1 change: 1 addition & 0 deletions eng/configure-toolset.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ function Test-FilesUseTelemetryOutput {
require_telemetry_exclude_files=(
'eng/common/build.sh'
'eng/common/cibuild.sh'
'eng/common/native/init-compiler.sh'
'eng/common/cross/tizen-build-rootfs.sh'
'eng/common/cross/tizen-fetch.sh'
'eng/common/cross/build-android-rootfs.sh'
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.CMake.Sdk/build/Microsoft.DotNet.CMake.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
<PropertyGroup>
<_NativeScriptsDir>$([MSBuild]::NormalizeDirectory('$(RepositoryEngineeringDir)', 'common', 'native'))</_NativeScriptsDir>
<CMakeCompilerSearchScript>
. &quot;$([MSBuild]::MakeRelative('$(MSBuildProjectDirectory)', '$(_NativeScriptsDir)/init-compiler.sh'))&quot; &quot;$(_NativeScriptsDir)&quot; &quot;$(Platform)&quot; &quot;$(CMakeCompilerToolchain)&quot;
build_arch=&quot;$(Platform)&quot; compiler=&quot;$(CMakeCompilerToolchain)&quot; . &quot;$([MSBuild]::MakeRelative('$(MSBuildProjectDirectory)', '$(_NativeScriptsDir)/init-compiler.sh'))&quot;
</CMakeCompilerSearchScript>
</PropertyGroup>
</Target>
Expand Down