-
Notifications
You must be signed in to change notification settings - Fork 382
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
Add workflow for auto updating submodules and submitting a PR #302
base: main
Are you sure you want to change the base?
Conversation
@agorararmard, if I don't get it wrong, this is already supported by https://dependabot.com/. It used to be a (free) third-party service, but it was then bought by GitHub. It's now a built-in service and it's the tool used internally for tracking security/critical update warnings. Therefore, I don't think a custom workflow is necessary. Maintainers just need to enable it in the settings. |
@umarcor For various complicated reasons, we need to use a custom action here. |
if: ${{ contains(github.ref, 'master') }} | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actions/checkout has options for getting submodules too. See https://github.com/actions/checkout/#usage.
It might make sense to suggest option submodules: remote
to be added upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't hurt I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
submodules: remote
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW We probably want a "Treeless clone" otherwise the worker will run out of disk space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mithro, in this PR git submodule update --remote
is executed after checkout, in order to update all submodules to their heads/master/main. That is required because actions/checkout does not support such feature. Hence, I was suggesting that maintainers of the action might want to add the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mithro: Yeah, with the current setup a Github-hosted runner will definitely run out of disk space. But would a treeless clone help here tho (I'm trying it out locally right now)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mithro, @umarcor: I'm afraid a treeless clone won't cut it (unless I'm doing it wrong). We will need a self-hosted runner to do this. However, since this workflow will only run on internal pushes, there are no security concerns regarding the protection of the self-hosted runner against pull-requests. In other words, this can be any machine.
After running the script to update submodules, after git clone --filter=tree:0 ....
:
du -sh skywater-pdk/
53G skywater-pdk/
uses: peter-evans/create-pull-request@v3 | ||
with: | ||
token: ${{ secrets.GITHUB_TOKEN }} | ||
commit-message: Update All Submodules to remote latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make the commit message and the title match. Maybe these params can be created in a previous step and then used here. That would allow to customize the message/body depending on which submodules were updated.
Moreover, hub
is preinstalled in the virtual environments. Might be worth having a look at it, instead of dependending on a third-party Action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make the Commit message look similar to the ones in this pull request --> #233
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was the Python code which generated that status message,
log = subprocess.check_output(['git', 'log', '--graph', '--pretty=short', '--decorate=short', '--decorate-refs=remotes/origin', '--shortstat', old_version+'..'+new_version], cwd=submod).decode('utf-8')
output = ['Updating submodules on {} UTC'.format(now), '']
for lib, ver, ov, nv in changes:
if ov == nv:
continue
output.append('''\
- Updating [`{lib}` {ver}](https://foss-eda-tools.googlesource.com/skywater-pdk/libs/{lib}/+/{ov}..{nv}) to {nv}'''.format(
lib=lib, ver=ver, ov=ov, nv=nv))
output.append('')
output.append(get_submod_info())
print('\n'.join(output))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the workflow, the required change would be:
- id: generate
run: echo 'BODY=any_text' >> $GITHUB_ENV
- uses: peter-evans/create-pull-request@v3
with:
body: ${{ env.body }}
See https://github.com/google/verible/blob/master/.github/workflows/verible-ci.yml.
Or, use hub
, or pyGitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
41f0f88
to
83838f1
Compare
83838f1
to
3433fd9
Compare
…PR + add workflow_dispatch option
This simple workflow will run whenever there is a push to master and with a corn job. It will update all submodules to the latest commit in their remote and submit a PR to master from the auto-created/auto-deleted branch.