Fix GitHub actions updater crashing and cloning the wrong repository #6052
Conversation
|
As a note, this does not happen in every repository with actions pinned to refs. A classic situation when this does happen is:
|
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-github-actions-crash
branch
3 times, most recently
from
b0161a3 to
dc66eea
Compare
|
I added some specs with VCR. My concern is whether these specs would start failing once more commits land in the sample repository, since these specs are actually cloning the repo to exercise the code changed here. I think no, because latest commit information is fetched from the Still these specs rely on cloning an external repo, which could get brittle. I'm not sure how to approach testing this otherwise. I thought of adding some smoke tests, but this particular situation needs the target repository and the repository where the updated action lives to be different repositories 🤔. In general, I think this is good enough but I'm happy to hear more opinions. |
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-github-actions-crash
branch
from
2061651 to
95c6f95
Compare
|
I found and fixed one more issue where refs not tracking the default branch of an action were not being updated properly. Also I added an optimization to not clone the action's repository if already pinned to the tip of a branch. I think this should be good to go now. |
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-github-actions-crash
branch
from
95c6f95 to
43a1234
Compare
github_actions/spec/dependabot/github_actions/update_checker_spec.rb
Outdated
Show resolved
Hide resolved
Since we're now able to bump both refs and versions (in comments) at the same time, `dependency.version` may not always hold a ref now. So always rely on metadata here.
After fixing the previous crash, I noticed that we were actually cloning the _repository_ being updated (the one that includes the workflow files), not the repository of the dependency being updated (the repository of the action itself). I have no idea how this somewhat seemed to work in some cases before.
The previous logic did not really work with shallow forks. In other for all branches to be reachable, we need to clone the whole repository. Also, unless we actually checkout the branches, we need to use `git branch --remote --contains`. There's still further work that could be done here, for example, to disambiguate multiple branches including the commit by checking whether one of them includes the others, and upgrade to the most recent in that case. But that's for another time.
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-github-actions-crash
branch
from
43a1234 to
a17e9c9
Compare
deivid-rodriguez
deleted the
deivid-rodriguez/fix-github-actions-crash
branch
This PR wants to fix two issues:
After Update version comments for SHA-pinned GitHub Actions #5951, we seem to have started using the
GitCommitChecker#head_commit_for_current_branchmethod whendependency.versionis an actual numbered version and not a ref. This is probably because with Update version comments for SHA-pinned GitHub Actions #5951 we have to deal with both numeric and ref versions in the same update. The current implementation of this method does not support this. I expect 90612fdda8a2471b66e78f341a649b013f6309a5 to fix this issue.Even with the above fix, updates for pinned refs still don't work as expected and we either get a different crash or a downgrade. The reason for this seems to lie in Properly upgrade github actions pinned to specific commits #5576. In that PR, in order to be able to properly detect whether actions pinned to arbitrary references are out of date, we started cloning the whole repository. However, I think the implementation there was incorrect and it started cloning the repository being updated (the one that includes the workflow files), not the repository of the dependency being updated (the repository of the action itself). I expect 721bd2df6cdd8462ddb6b3ab0adfd76978487831 to fix this issue.
With this I get proper updates of repositories pinned to specific references.
Fixes #6014.
Fixes #6045.