Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure test ref exists in clone #45

Closed
lsf37 opened this issue Feb 18, 2021 · 4 comments
Closed

Make sure test ref exists in clone #45

lsf37 opened this issue Feb 18, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@lsf37
Copy link
Member

lsf37 commented Feb 18, 2021

There is a race condition in PRs where the PR head (GITHUB_REF) might be set for an action, and shortly after, but before the repo is cloned inside the action, this ref disappears, e.g. because of a force-push, or because the PR has been merged with "rebase + merge" or "squash + merge", esp for very long-running actions that are optional (like in l4v on the MCS branch).

I think seL4/util_libs#67 is an instance of that.

This issue is to investigate if there is a way to make sure that the test ref is fetched explicitly, even if it is not connected any more and would otherwise not be included in a git clone.

Hopefully we've factored out things enough that this doesn't need to be done in much more than one location..

@lsf37 lsf37 added the enhancement New feature or request label Feb 18, 2021
@lsf37 lsf37 self-assigned this Feb 18, 2021
@lsf37
Copy link
Member Author

lsf37 commented Feb 18, 2021

It looks like a simple

git fetch <remote> <ref-sha>

does this in modern git versions, so for the price of an additional fetch, this should work fine.

@axel-h
Copy link
Member

axel-h commented Feb 18, 2021

I think it could also be an acceptable that if one force-pushes a commit, the old one is just gone and any testing would stop with an error. Rationale is, that if somebody force-pushes, this is either a clear statement "my prev commit is to be discarded" or a bad commit accident. Pulling commits from the GIT database, which are not referenced by a branch seem a delicate thing to me. because it yields the danger that you are working in commit that may get purged eventually anyway. Default is 30 day I think. So whatever the tests do there is not reproducible.
I have to blame myself for opening this issues, because I did a force-push there, then forgot that I did this and than complained about some strange messages about invalid commit IDs. So maybe the most simple solution here is, that we just have better messages that say this this might come from a force push. In addition we would dig into the GIT database to see if the commit is there and print another message that says this. So there is a clear differentiation between this well understood issues and some more obscure error that we yet have to understand.

@lsf37
Copy link
Member Author

lsf37 commented Feb 18, 2021

True, this could indeed lead to an even more difficult to diagnose error.

I'll experiment a little -- maybe we can actually use this command to figure out if the ref still exists on GitHub: basically you'd first check if the ref exists in the clone (as opposed to relying on it as we do now), then if it isn't there, you try to fetch it, and if that fail, you can give an error message that says "ref XXX not found, possibly lost in a force push or rebase", or something like that.

Will treat this as lower priority, though, so it might take a while.

@lsf37
Copy link
Member Author

lsf37 commented Jan 7, 2022

I think I'll abandon this one. It's been working fairly reliably so far.

Happy to reopen if it becomes a more pressing issue again.

@lsf37 lsf37 closed this as completed Jan 7, 2022
@lsf37 lsf37 added the wontfix This will not be worked on label Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants