Skip to content

Conversation

@tmat
Copy link
Member

@tmat tmat commented Feb 3, 2020

Ensures that we don't leave temp files behind in global TEMP dir on CI machine.
UPDATE: ADO cleans Agent.TempDirectory. So setting TEMP is not needed in order to get clean temp dir.

Prepare-TempDir and maybe other infra seem to expect TEMP to be set to $TempDir.

dotnet/arcade#4744 reverted setting the variable in tools.ps1 due to issues it caused in other repos.

dotnet/arcade#4744 reverted setting the variable in `tools.ps1` due to issues it caused in other repos.
@tmat tmat requested a review from a team as a code owner February 3, 2020 05:26
@tmat
Copy link
Member Author

tmat commented Feb 3, 2020

@JoeRobich PTAL

@sharwell
Copy link
Contributor

sharwell commented Feb 3, 2020

Ensures that we don't leave temp files behind in global TEMP dir on CI machine.

No reasonable CI persists this state between independent executions (doing so allows a build to corrupt an unrelated future build, eliminating all notions of build repeatability). Is there an ETA on correcting this for our builds?

@tmat
Copy link
Member Author

tmat commented Feb 3, 2020

@sharwell
I am not positive that ADO cleans the %TEMP% dir.

The ADO docs do not mention anything about cleaning directories other than the working directory created by ADO. The goal of this PR is to make sure we write temp files to working directory so that the cleanup is guaranteed.

We actually use the fact that %UserProfile%\.nuget is not cleared in between builds to avoid re-downloading packages.

IDK how would ADO know to not clean nuget cache but clean temp dir.

Co-Authored-By: Sam Harwell <sam@tunnelvisionlabs.com>
@jaredpar
Copy link
Member

jaredpar commented Feb 3, 2020

The ADO docs do not mention anything about cleaning directories other than the working directory created by ADO.

They also guarantee cleanup of Agent.TempDirectory. This is relied on by the runtime team for cleaning up dotnet installs.

@tmat
Copy link
Member Author

tmat commented Feb 3, 2020

@jaredpar Thanks. Found it in docs: https://docs.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml

@tmat
Copy link
Member Author

tmat commented Feb 3, 2020

OK, so we don't need to set TEMP to artifacts\tmp to get it cleaned. But Prepare-TempDir and maybe other infra seem to expect TEMP to be set $TempDir, so I'd leave it set for now.

@tmat tmat merged commit 7b1a5b5 into dotnet:master Feb 3, 2020
@tmat tmat deleted the Temp branch February 3, 2020 21:35
333fred added a commit to 333fred/roslyn that referenced this pull request Feb 6, 2020
* dotnet/master: (918 commits)
  Pure null test on unconstrained type (dotnet#41384)
  Fix file headers, bootsrap build issues.
  Make `elementLocations` accept an array of nullable locations in the public api.
  Learn from non-null tests on as operator (dotnet#41419)
  Use Microsoft.NET.Sdk.WindowsDesktop for XAML projects (dotnet#40234)
  Address feedback.
  Introduce `GetRequiredBinder`.
  Add missing `NotNullWhen` annotations.
  Annotate more of the binder
  Add version of zlib library to deterministic build inputs (dotnet#41385)
  [master] Update dependencies from dotnet/arcade (dotnet#41354)
  Simplify
  Simplify
  Do not simplify interpolation expressions on implicit receivers.
  Fix local function crash + add tests
  Substituted symbol equality (dotnet#41123)
  Fix test failures
  Rename from IncludeNonNullableReferenceTypeModifier to IncludeNotNullableReferenceTypeModifier (dotnet#41332)
  Set TEMP environment variable to $TempDir on CI machines (dotnet#41361)
  Document placeholders
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants