This caused some deployment issues in CB, and there is a hot fix that CB needs that got trapped in between cleanup waves. Reverting so CB can get unblocked.
Untrack codeql temp dir from nugetdownloader
NugetDownloader don't get the gloablUntrackedScopes. Let nugetdownlaoder get the codeql path from environment and untrack it
Related work items: #2133978
The use of Try* prefix for `TrySetExecutePermissionIfNeeded` is confusing. Typically, Try method returns simply `bool` where the `false` value indicate that the action is not successful.
In the case of `TrySetExecutePermissionIfNeeded`, the return value of `true` and `false` have some meanings, i.e., whether `chmod` may be called or not. So, the use of Try prefix is inappropriate here.
Also:
- Simplify method signatures in LocalDiskContentStore.
* Removed now unnecessary 'dependentPackageIdsToSkip'
* Removed remaining unnecessary 'withQualifier' and the general netstandard2.0 forcing that happened on cache specs side
* Reorganized asptnetcore assembly references. This could be simplified further in the future, but now it is more self-contained.
* Some other minor cleanups
Next step is trying to make sure dependency versions are the right ones and try to turn on the flag that enforces this from the nuget resolver.
Revert "Merged PR 765049: DScript spec cleanup wave 2
* Removed now unnecessary 'dependentPackageIdsToSkip'
* Removed remaining unnecessary 'withQualifier' and the general netstandard2.0 forcing that happened on cache specs side
* Reorganized asptnetcore assembly references. This could be simplified further in the future, but now it is more self-contained.
* Some other minor cleanups
Next step is trying to make sure dependency versions are the right ones and try to turn on the flag that enforces this from the nuget resolver."
Reverted commit `2b01690d`.
Rename references to "public" to "external" for the builds that run with `microsoftInternal=0`, meant for external publishing and consumption. Additionally, remove the visibility of the secrets that are used to access internal resources in the jobs that are building and testing these external bits.
Related work items: #2140170
* Removed now unnecessary 'dependentPackageIdsToSkip'
* Removed remaining unnecessary 'withQualifier' and the general netstandard2.0 forcing that happened on cache specs side
* Reorganized asptnetcore assembly references. This could be simplified further in the future, but now it is more self-contained.
* Some other minor cleanups
Next step is trying to make sure dependency versions are the right ones and try to turn on the flag that enforces this from the nuget resolver.
For some observations we report the root process of the pip upon encountering an access violation, and this can be misleading to a user that wants to allow-list the access based on process name. This has happened in the past. This PR makes the message more generic so someone reading the message doesn't jump easily to conclusions about which specific process performed the access.
Mostly cache specs, but also touching some shared stuff:
* Unified some Nuget packages (System.Collections.Immutable, System.Memory, etc), and bumped versions to the latest. This triggered a bunch of other updates...
* Removed a lot of unnecessary 'withQualifier'
* Simplified some references by fixing the 'runtimes' folder behavior on the Nuget resolver (which forced a bunch of 'withWinRuntime' calls
More cleanup passes are possible, but those to come later
We need to always hash files when we're uploading to ensure the `ContentHash` matches that we're supposed to be uploading. However, in most code paths we're already hashing the files when inserting into the local, so we've already done a check to ensure the file hash matches. In such cases, we can avoid hashing the file twice.
QB inserts the same file hash from 3 different paths at the exact same time. Because we don't limit concurrency per hash at the ephemeral layer, the 3 requests start at the same time.
The 1st request starts up and inserts into the local FSCS, then spends 2s uploading to Azure Storage.
The other 2 requests start up, they both fall into the optimization this PR deletes. At this point, we have: 1 request uploading, 2 other requests completed with Success to insert due to the heuristic.
Because QB deduplicates the file uploads based on hash at some layer, it decides that the target has been uploaded, and proceeds to schedule a target in a different worker.
That different worker doesn't need to check for cache hit because the targets it depends on were built on the same build, so it just proceeds to Place the content. We try to copy the file from the other workers in the build, but they all timeout (we've got extremely low timeouts here on purpose), so we fall onto storage. Because this races with the file upload, and the file upload isn't done, storage says "this doesn't exist".
We fail the build!
This failed 4 prod builds today. Please note, removing the optimization actually improves our situation:
1. The case this optimization is meant to optimize will be caught with the call to AllowPersistentPutElisionAsync, which is lock-free lookup in memory and so a _priori_ should be quite fast.
2. Calling AllowPersistentPutElision is actually what we truly want to achieve (avoiding the upload if we know it's already in storage).
3. The removal of this heuristic means we no longer need to ensure there's a unique cache location for every build. We can always re-use the same cache and the ephemeral layer won't give incorrect results.
In the most recent scenarios, we are giving the engines a list of URIs which include the SAS token. We should support this scenario without requiring them to parse anything.
Make sure blob cache creation blows up if a container failed to be created. We need to allow for the `CreateIfnotExists` call to fail in case we only have read-only permissions. However, if it failed and the container in fact does not exist, the cache should blow up. Hopes are that the engines will be able to handle this error and proceed without the remote cache in dev builds.
This PR:
- Makes gRPC clients use a single connection pool
- Eliminates a lot of gRPC options being passed around that aren't actually used or plumbed
- Simplifies the gRPC configuration to follow similar settings as AnyBuild does
- Gets us one step closer to enabling gRPC encryption by always enabling server-side encryption on a separate port, and allowing usage of grpcs://, https:// etc in MachineLocation to denote HTTPS connections.
- Makes all instances actually try to enable gRPC encryption on the server side. If it fails it fails, but we try
Validation: https://cloudbuild.microsoft.com/user/jubayard_20240126152912
Serge Mera
Oleksii Kononenko
Qi Wang
Marcelo Lynch 🧉
Julian Bayardo
Iman Narasamdya
Sahiti Chandramouli
CloudDev Build Account
Pasindu Gunasekara 🍣
Juan Carlos Guzman Islas
Semih Okur
Michael Pysson