utils: use enumerations to locate build locations #71442
Conversation
|
@swift-ci please test Windows platform |
| return "$BinaryCache\" + ($Arch.BuildID + $Project.value__) | ||
| } | ||
|
|
||
| enum HostComponent { |
There was a problem hiding this comment.
Will this renumber the directory names at all? Would it be useful to add the corresponding numbers next to each enum elements as comments like
BuildToosl # 0
Compilers # 1
...
?
There was a problem hiding this comment.
No, the numbers should be left in tact (but, the beauty is that we don't need to synchronize those after this - everything should be left in a consistent state.
utils/build.ps1
Outdated
| $Targets = @("default", "test-llbuild") | ||
| $TestingDefines = @{ | ||
| FILECHECK_EXECUTABLE = "$BinaryCache\0\bin\FileCheck.exe"; | ||
| FILECHECK_EXECUTABLE = (Join-Path -Path (Join-Path -Path (Get-HostProjectBinaryCache BuildTools) -ChildPath "bin") -ChildPath "FileCheck.exe"); |
There was a problem hiding this comment.
The Join-Path seem a little overkill throughout?
| FILECHECK_EXECUTABLE = (Join-Path -Path (Join-Path -Path (Get-HostProjectBinaryCache BuildTools) -ChildPath "bin") -ChildPath "FileCheck.exe"); | |
| FILECHECK_EXECUTABLE = "$(Get-HostProjectBinaryCache BuildTools)\bin\FileCheck.exe"; |
There was a problem hiding this comment.
Would be nice to be able to support other platforms some day (e.g. Linux)
There was a problem hiding this comment.
We could create a Join-Path variant that takes an arbitrary number of components and doesn't require labels so it's tidier.
There was a problem hiding this comment.
or use [IO.Path]::Combine($foo, $bar, $baz)
There was a problem hiding this comment.
Let me try switching to [IO.Path]::Combine ... if we can use builtins, I'd prefer that.
| } | ||
|
|
||
| function Get-HostProjectCMakeModules([HostComponent]$Project) { | ||
| return "$BinaryCache\$($Project.value__)\cmake\modules" |
There was a problem hiding this comment.
All remaining instances of hard-coded values in this file are to $BinaryCache\1 or $BinaryCache\7, so how about defining two globals after this function definition:
$CompilersBinaryCache = Get-HostProjectBinaryCache Compilers
$DriverBinaryCache = Get-HostProjectBinaryCache DriverThere was a problem hiding this comment.
Oh, didn't catch that they were in the same function - totally seems reasonable.
There was a problem hiding this comment.
I'm not sure if the usages are in the same function, I was suggesting creating global variables for them. (but even better if they can be locals)
| BUILD_SHARED_LIBS = "NO"; | ||
| SwiftASN1_DIR = "$BinaryCache\10\cmake\modules"; | ||
| SwiftCrypto_DIR = "$BinaryCache\8\cmake\modules"; | ||
| SwiftCrypto_DIR = (Get-HostProjectCMakeModules Crypto); |
There was a problem hiding this comment.
Parens are probably not necessary on the RHS of an assignment.
There was a problem hiding this comment.
Oh, it shouldn't confuse things to have Get-HostProjectCMakeModules Crypto;? I definitely would prefer to remove the parenthesis then.
There was a problem hiding this comment.
Seems that they are needed :( - maybe you can try to clean them up subsequently? I'd love to get that noise removed.
| function Get-ProjectBinaryCache($Arch, [Target]$Target) { | ||
| return "$BinaryCache\" + ($Arch.BuildID + $Target.value__) | ||
| function Get-TargetProjectBinaryCache($Arch, [TargetComponent]$Project) { | ||
| return "$BinaryCache\" + ($Arch.BuildID + $Project.value__) |
There was a problem hiding this comment.
Oh wow, value__ is the name of the hidden field that .NET generates in structs derived from System.Enum per standard. Cool trick!
Rather than hardcode the build numbers, use named parameters to locate them during the build process.
|
@swift-ci please test Windows platform |
|
@swift-ci please test Linux platform |
|
@swift-ci please test macOS platform |
Rather than hardcode the build numbers, use named parameters to locate them during the build process.