Don't overwrite binary logs #66559
Don't overwrite binary logs #66559
Conversation
|
@dotnet/roslyn-infrastructure PTAL |
| @@ -166,13 +168,21 @@ function Process-Arguments() { | |||
| $script:applyOptimizationData = $false | |||
| } | |||
|
|
|||
| if ($binaryLogName -ne "") { | |||
| $script:binaryLog = $true | |||
There was a problem hiding this comment.
Initially I required specifying both options so -binaryLog -binaryLogName "Build.binlog" but that just looked too verbose to me. Decided it was concise to allow -binaryLogName to imply -binaryLog
Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future.
| @@ -180,7 +180,7 @@ function Process-Arguments() { | |||
| } | |||
|
|
|||
| if ($binaryLog -and ($binaryLogName -eq "")) { | |||
| $binaryLogName = "Build.binlog" | |||
| $script:binaryLogName = "Build.binlog" | |||
There was a problem hiding this comment.
Well because you asked ...
It's because when powershell reads a variable it will check function scope, then script scope. But if you write a variable it will write it in the function scope, even if one exists at script scope. So you need to be very explicit that you're writing a script scoped variable here.
There was a problem hiding this comment.
Should the $binaryLogName just above be $script:binaryLogName as well then?
There was a problem hiding this comment.
The uses on 182 are reads. Those will fall back to script scope. Line 228 is creating a new variable $binaryLogPath which is intentionally function level scope.
Why don't we always use script: when accessing script variables and instead rely on fallback rules? I don't have a good argument for that other than that's just kinda how it's written in a lot of cases. It's like asking why we require this. for extension methods but not for normal instance methods.
azure-pipelines.yml
Outdated
| @@ -207,7 +207,7 @@ stages: | |||
| - src/Compilers/* | |||
| - src/Dependencies/* | |||
|
|
|||
| - job: Correctness_Build_Artifacts | |||
| - job: Correctness_Build_Artifacts | |||
There was a problem hiding this comment.
Well that's why the job hasn't started yet ... invalid YML
|
The correctness legs have all passed. Force merging so we can unblock the rest of our active PRs. |
ghost
added this to the 
* Don't overwrite binary logs Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future. * enable trace for diff issue * Work around NuGet static graph restore bug NuGet/Home#12373
* Don't overwrite binary logs Several of our legs were calling `build.ps1 -binaryLog` in a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future. * enable trace for diff issue * Work around NuGet static graph restore bug NuGet/Home#12373
Several of our legs were calling
build.ps1 -binaryLogin a row and that re-uses the binary log file name which causes an overwrite. This change allows us to specify the name and adds a warning if we do accidentally overwrite in the future.