denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.01k stars 5.39k forks source link

Make uploading canary build less racy #25464

Open dsherret opened 2 months ago

dsherret commented 2 months ago

It's currently possible for a CI run of an older commit than canary to finish later and overwrite the newer canary version.

We should update the canary upload to get the current canary commit and only upload if that commit is in its git history.

bartlomieju commented 2 months ago

I'm not sure the description here is correct - we should still upload the binary for particular canary to a proper directory but not update files like https://dl.deno.land/canary-aarch64-apple-darwin-latest.txt.

Is that what you meant @dsherret?

dsherret commented 2 months ago

This is done unconditionally:

https://github.com/denoland/deno/blob/eb8ee95f08186c948e5b83501cedd59d6e3b4ef2/.github/workflows/ci.yml#L489-L490

Sequence of events:

  1. Commit A lands.
  2. Commit B lands.
  3. Commit B's CI finishes quickly and uploads to canary.
  4. Commit A finally finishes and uploads to canary.

Now canary is on Commit A rather than Commit B.

bartlomieju commented 2 months ago

Changes reverted in https://github.com/denoland/deno/pull/25733 because CI is consistently failing with exit code 1 after a successful upload.