Skip to content

Conversation

itowlson
Copy link
Collaborator

@itowlson itowlson commented Oct 9, 2025

…hopefully.

This was the line that seemed to make the difference between green and red in #3296. I'm creating a new PR so as to preserve the #3296 history while we evaluate/test, but also get a cleanish run to reassure myself that this does make the necessary difference.

If it does, @tschneidereit or other GH cache experts need to make a call on whether it's going to mess up our billing or whatever. But the current situation is untenable so if this isn't an acceptable solution then we'll need to figure out what is.

@itowlson itowlson marked this pull request as ready for review October 9, 2025 04:13
@tschneidereit
Copy link
Contributor

I would be really very surprised if my caching changes really caused this: I made them after realizing that we were almost always evicting caches before even using them once.

Then again, this is caching. It is CI. It is a combination caching and CI. So who knows!

@itowlson
Copy link
Collaborator Author

itowlson commented Oct 9, 2025

@tschneidereit I am also surprised. And yet after weeks of Ubuntu testing needing multiple re-runs, I made this change and it completed first time. It could be that using cache disguises some sort of unrelated issue, and the true fix is for that unrelated issue? (Like you say: CI, who knows.) And of it could be that this is not a fix and I just had a freakish run of good luck with these two PRs! I dunno. I just want to not have to do repeated re-run cycles on every push.

My suggestion is that unless we either:

  1. Know a compelling reason not to re-enable cache in this one context; or
  2. Have another avenue we'd like to explore

then we take this PR and at least see if it helps.

@tschneidereit
Copy link
Contributor

One thing maybe worth doing is to delete the cache entry and retrigger the run. If that also succeeds, I think it's pretty unlikely to be a fluke, and we should definitely land this.

I would in general say land it, except I find it so utterly baffling that the act of storing a cache entry would make something pass that didn't before. And looking at the log, as I'd expect there's no cache hit, so the only functional difference here really seems to be that after everything has run, the cache entry is created.

@tschneidereit
Copy link
Contributor

Ooh, except that's not the case, as I realize now, right after sending the previous comment!

At the very least, the cache action also sets CARGO_INCREMENTAL=0, but it might make different changes as well. That's the right thing to do in CI regardless of whether we cache or not, and certainly is a meaningful change in build config.

@itowlson
Copy link
Collaborator Author

itowlson commented Oct 9, 2025

Oho! Good find. I've just kicked off the re-run after deleting the cache. Then I can try it without cache but with CARGO_INCREMENTAL=0. Thanks!

ETA: #3298

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.

2 participants