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

Added checks for main dependencies on runtime's build scripts #39052

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions build.cmd
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
@echo off
setlocal

call "%~dp0eng\vs_tools_check.cmd"

set _args=%*
if "%~1"=="-?" set _args=-help

Expand Down
30 changes: 30 additions & 0 deletions eng/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,32 @@ function Get-Help() {
Write-Host "For more information, check out https://github.com/dotnet/runtime/blob/master/docs/workflow/README.md"
}

function Assert-InstalledDependency($dependencyName)
{
try {
Get-Command -Name $dependencyName -ErrorAction Stop >$null 2>&1
}
catch {
Write-Host "$dependencyName is required to build this repo. Make sure to install it and try again."
Write-Host "For a full list of requirements, see https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/windows-requirements.md"
exit 1
}
}

function Assert-GitLongPathsEnabled()
{
# This needs to be in a variable. Otherwise, Invoke-Command complains about
# an incompatible cast.
$gitscript = [scriptblock]::Create("git config --get core.longpaths")
$longpaths = Invoke-Command -scriptblock $gitscript

if (-Not $longpaths) {
Write-Host "Git Long Paths must be enabled to build this repo."
Write-Host "For a full list of requirements, see https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/windows-requirements.md"
exit 1
}
}

if ($help -or (($null -ne $properties) -and ($properties.Contains('/help') -or $properties.Contains('/?')))) {
Get-Help
exit 0
Expand All @@ -110,6 +136,10 @@ if ($subset -eq 'help') {
exit 0
}

Assert-InstalledDependency("CMake")
Assert-InstalledDependency("Git")
Copy link
Member

Choose a reason for hiding this comment

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

Git isn't a dependency. The repo should be buildable without any source control provider. #47298 tracks protecting this scenario in CI.

Assert-GitLongPathsEnabled

if ($vs) {
. $PSScriptRoot\common\tools.ps1

Expand Down
15 changes: 15 additions & 0 deletions eng/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,21 @@ showSubsetHelp()
"$scriptroot/common/build.sh" "-restore" "-build" "/p:Subset=help" "/clp:nosummary"
}

assertInstalledDependency()
{
dependency="$(echo "$1" | awk '{print tolower($0)}')"
location="$(command -v $dependency)"

if [ -z "$location" ]; then
echo "$dependency is required to build this repo. Make sure to install it and try again."
echo "For a full list of requirements, see https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/linux-requirements.md"
Copy link
Member

Choose a reason for hiding this comment

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

The message refers to linux-requirements.md, but the current OS can be OSX.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. I will add a check and point to OSX when it's such case.

exit 1
fi
}

assertInstalledDependency 'CMake'
assertInstalledDependency 'Git'
Copy link
Member

@ViktorHofer ViktorHofer Sep 30, 2020

Choose a reason for hiding this comment

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

Git isn't a dependency. Sourcebuild needs to work without git.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, it is. The build uses it to extract it the commit hash that we embed into the binaries.

Copy link
Member

Choose a reason for hiding this comment

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

Is it done unconditionally, or can this behavior be suppressed by configuration? I believe that there are configuration options to suppress this behavior.

Copy link
Member

Choose a reason for hiding this comment

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

You are right, I've tried to uninstall git and the build completed ok, just no version info got embedded into the binaries.

Copy link
Member Author

Choose a reason for hiding this comment

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

So we just check for CMake then?

Copy link
Member

Choose a reason for hiding this comment

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

yes


arguments=''
cmakeargs=''
extraargs=''
Expand Down
40 changes: 40 additions & 0 deletions eng/vs_tools_check.cmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
@if not defined _echo @echo off
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to add an argument to the existing string instead of duplicating the whole existing script except the one line that calls VsDevCmd.bat ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I hadn't seen this before Jan. Yes, I think this is a good idea to try. I also wasn't very comfortable with duplicating the code but wasn't able to think of another approach. Will try this.


REM This script checks and ensures there is a working Visual Studio installation
REM alongside its dev tools. This is a requirement to build the runtime repo.
REM All passed arguments are ignored
REM Script will return 0 if a VS installation is found, and 1 if any problems
REM cause it to fail.

:: Default to highest Visual Studio version available

:: For VS2017 and later, multiple instances can be installed on the same box.
:: SxS and VS1*0COMNTOOLS are no longer set as global environment variables and
:: are instead only set if the user has launched the Visual Studio Developer
:: Command Prompt.

:: Following this logic, we will default to the Visual Studio toolset associated
:: with the active Developer Command Prompt. Otherwise, we will query VSWhere to
:: locate the later version of Visual Studio available on the machine. Finally,
:: we will fail the script if not supported instance can be found.

if defined VisualStudioVersion (
goto skip_setup
)

set _VSWHERE="%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe"
if exist %_VSWHERE% (
for /f "usebackq tokens=*" %%i in (`%_VSWHERE% -latest -prerelease -property installationPath`) do set _VSCOMNTOOLS=%%i\Common7\Tools
goto call_vs
)

:call_vs
if not exist "%_VSCOMNTOOLS%" (
echo %__MsgPrefix%Error: Visual Studio 2019 is required to build this repo. Make sure to install it and try again.
echo For a full list of requirements, see https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/windows-requirements.md
exit /b 1
)

:skip_setup

exit /b 0