Skip to content

Conversation

@jmgrosen
Copy link

It seems like sometimes the Cygwin homepage does not contain its version in the format setup-ocaml expects. This causes the tool cache to complain about an empty version: https://github.com/semgrep/semgrep-proprietary/actions/runs/20381063339/job/58571399243?pr=5267 and perhaps cause higher-level caching issues too.

This PR

  1. Removes the usage of the Cygwin version in the tool cache, because any later action invocations that want Cygwin from the tool cache might as well use the same version as the first. (This assumes that the tool cache is ephemeral, as is the case on GitHub-hosted runners. We are not currently concerned about self-hosted runners that have non-ephemeral tool caches set up.)
  2. Explicitly throws an error when the Cygwin version cannot be identified, so that we can better track if this still occurs (for the actions/cache use).

Copy link

@yosefAlsuhaibani yosefAlsuhaibani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! I wonder if we can log what we got from get(cygwin.com) to confirm if it was us getting rate limited.

if (version !== null) {
return version;
} else {
throw new Error("Couldn't parse Cygwin version from homepage");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: is there a way to inspect errors like in CI in datadog? just interested in case we want to look up how many times this has come up.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could start sending CI logs to Datadog, but I think that's expensive. We can at least monitor how often the setup-ocaml step specifically fails.

@yosefAlsuhaibani
Copy link

Also it seems like the upstream has abandoned ubuntu-slim from it's matrix, so I am not that worried that ubuntu-slim is failing.

@jmgrosen jmgrosen merged commit c0776a2 into master Dec 24, 2025
21 of 23 checks passed
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.

3 participants