add support for cloning Git repository via SSH rather than HTTPS #300
Conversation
merge v0.1.1 into main
release v0.2.0
release v0.3.0
release v0.4.0
release v0.5.0
merge develop into main for release v0.6.0
tasks/build.py
Outdated
| @@ -407,6 +422,17 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): | |||
| error_stage = _ERROR_CURL | |||
| return curl_output, curl_error, curl_exit_code, error_stage | |||
|
|
|||
| git_diff_cmd = ' '.join([ | |||
| f"git fetch origin pull/{pr.number}/head:{pr.number}", | |||
| f"git diff HEAD pr{pr.number} > {pr.number}.diff", | |||
There was a problem hiding this comment.
if you're now creating the diff like this, shouldn't you remove the curl_cmd above?
There was a problem hiding this comment.
Yes, of course, I'll tackle that too
There was a problem hiding this comment.
@smoors made a good point (in Slack) about this change: this doesn't take into account that there may be changes in the main branch that are not included in the PR branch yet.
So, a better approach would be to:
- fetch the PR branch
- check out the PR branch
git merge main- swap back to
mainbranch git diffgit apply
There was a problem hiding this comment.
This procedure hasn't been implemented yet, right?
A few thoughts:
mainshould probably be the target branch of the PR, for example, for EESSI/2023.06 that would be 2023.06-software.eessi.io?- the
git diffafter the swap back to "main" is really just the plaingit diffor rathergit diff PR_branch?
There was a problem hiding this comment.
to determine base branch for PR (the target branch), maybe git merge-base is useful...
There was a problem hiding this comment.
I'll change this so the new approach based on git is only used when clone_via is set to 'ssh' (so nothing changes for existing bots that listen to a public repo).
That gives us some freedom to experiment with the situation where the target branch has been updated since the branch for the PR was created.
There was a problem hiding this comment.
I've updated this in a99ae60 to only use the git diff approach when clone_git_repo_via is used.
It's still doing only git fetch + git diff, so you could still have issues when the PR branch is way behind on the target branch, I'll try to look into that next.
|
This is working as expected now, so marking it ready-for-review. Hopefully @trz42 has time to take a look? |
There was a problem hiding this comment.
Works as intended. See trz42/software-layer#78
The optional settings have to be commented. The intention with listing them, is more to document that they are optional and it could be easily changed in the future if we wish so.
I think, it might be good to add some comments (README.md and app.cfg.example) about what is needed if the ssh key has not a standard name and if it uses a passphrase.
…king it required Co-authored-by: Thomas Röblitz <[email protected]>
Co-authored-by: Thomas Röblitz <[email protected]>
…app.cfg.example Co-authored-by: Thomas Röblitz <[email protected]>
boegel
force-pushed
the
git_clone_via_ssh
branch
from
181d29c to
a27452d
Compare
boegel
force-pushed
the
git_clone_via_ssh
branch
2 times, most recently
from
c03e52b to
299b9e8
Compare
…are-layer into git_clone_via_ssh
boegel
force-pushed
the
git_clone_via_ssh
branch
from
299b9e8 to
80efece
Compare
boegel
force-pushed
the
git_clone_via_ssh
branch
from
80efece to
a99ae60
Compare
|
I haven't actually re-tested this after the changes I've made, and some more changes are required to the |
This is useful when the target repository of PRs that the bot should act on is a private repository.
Opt-in via this in bot configuration file (
app.cfg):(draft PR since this needs actual testing, and perhaps more changes)