add support for cloning Git repository via SSH rather than HTTPS by boegel · Pull Request #300 · EESSI/eessi-bot-software-layer

boegel · 2025-02-06T13:56:31Z

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):

[buildenv]
clone_git_repo_via = 'ssh'

(draft PR since this needs actual testing, and perhaps more changes)

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

…n configuration

smoors · 2025-02-06T14:56:03Z


+    git_diff_cmd = ' '.join([
+        f"git fetch origin pull/{pr.number}/head:{pr.number}",
+        f"git diff HEAD pr{pr.number} > {pr.number}.diff",


if you're now creating the diff like this, shouldn't you remove the curl_cmd above?

Yes, of course, I'll tackle that too

@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 main branch

git diff

git apply

This procedure hasn't been implemented yet, right?

A few thoughts:

main should probably be the target branch of the PR, for example, for EESSI/2023.06 that would be 2023.06-software.eessi.io?

the git diff after the swap back to "main" is really just the plain git diff or rather git diff PR_branch?

to determine base branch for PR (the target branch), maybe git merge-base is useful...

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.

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.

created a PR to this PR using git merge-base:
boegel#1

boegel · 2025-02-06T16:37:30Z

This is working as expected now, so marking it ready-for-review.

Hopefully @trz42 has time to take a look?

trz42

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 <trz42@users.noreply.github.com>

Co-authored-by: Thomas Röblitz <trz42@users.noreply.github.com>

…app.cfg.example Co-authored-by: Thomas Röblitz <trz42@users.noreply.github.com>

…are-layer into git_clone_via_ssh

…ository

boegel · 2025-02-21T16:25:06Z

I haven't actually re-tested this after the changes I've made, and some more changes are required to the git diff approach to take into account that the target branch of a PR may have been updated since the PR was opened, so I'm marking this as work-in-progress for now...

merge develop into main for release v0.7.0

git diff from merge-base instead of from HEAD

…are-layer into git_clone_via_ssh

smoors · 2025-05-04T10:22:53Z

this works as expected now that we're creating a diff from the merge-base instead of from the HEAD, which produces exactly the same diff as the one downloaded directly from api.github.com.

one thing that doesn't work with SSH is the bot: status command as that still requires using the github api. might be good to add a note about this.

smoors · 2025-05-22T11:56:06Z

one thing that doesn't work with SSH is the bot: status command as that still requires using the github api. might be good to add a note about this.

@boegel README updated in boegel#2

merging `develop` into `main` for release v0.8.0

update README

boegel · 2025-05-28T11:15:28Z

@trz42 This should be good to go now, can you take a look?

It doesn't change anything in the default behavior, but extra feature it adds is essential for working with private repositories, since those can't be cloned via HTTPS.

…are-layer into git_clone_via_ssh

trz42

Minor comment.

Can you also pull in upstream changes, thus one could test this PR and the PR for allowing no spaces between bot: and build?

trz42 · 2025-06-20T11:09:20Z

+                                f"{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_PR_DIFF_FAILURE]}"
+                                f"\n{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_PR_DIFF_TIP]}")


These settings should probably be added to the list of required settings.

fixed in 750b8a4

trz42

Tested via a pull request to a private repo. Works as intended.

Nice work @boegel and @smoors !

boegel and others added 11 commits November 14, 2023 19:41

Merge pull request EESSI#228 from EESSI/develop

e96ec76

merge v0.1.1 into main

Merge pull request EESSI#234 from EESSI/develop

66ba1f9

release v0.2.0

Merge pull request EESSI#240 from EESSI/develop

6ba9625

release v0.3.0

Merge pull request EESSI#259 from EESSI/develop

fa91fcd

release v0.4.0

Merge pull request EESSI#270 from EESSI/develop

a5b40e9

release v0.5.0

Merge pull request EESSI#285 from EESSI/develop

357f9df

merge develop into main for release v0.6.0

add support for cloning Git repository via SSH rather than HTTPS

a444bcc

fix use of config.BUILDENV_SETTING_CLONE_GIT_REPO_VIA

6dcdd80

also use clone_git_repo_via in app.cfg.example

3f2cfcb

mark clone_git_repo_via as optional + add logging

37c06af

fix get_build_env_cfg to get clone_git_repo_via value from buildenv i…

4b046f6

…n configuration

smoors reviewed Feb 6, 2025

View reviewed changes

boegel added 6 commits February 6, 2025 16:02

remove curl command to obtain PR diff + fix _ERROR_GIT_CLONE constant

45bd556

fix return value for 'git diff' command in download_pr function

b19fe23

make sure that download_comment is defined in comment_download_pr

5c51665

fix 'git diff' command in download_repo

a1c2635

fix branch name used by 'git diff' command

02f3125

Merge branch 'main' into git_clone_via_ssh

47e2076

boegel marked this pull request as ready for review February 6, 2025 16:37

boegel requested a review from trz42 February 6, 2025 16:37

poksumdo mentioned this pull request Feb 7, 2025

simple test showing contents of job dir and diff file if any trz42/software-layer#78

Open

trz42 requested changes Feb 7, 2025

View reviewed changes

Comment thread README.md

Comment thread app.cfg.example Outdated

Comment thread eessi_bot_event_handler.py Outdated

trz42 reviewed Feb 13, 2025

View reviewed changes

Comment thread README.md

comment out clone_git_repo_via setting in REQUIRED_CONFIG to avoid ma…

c7a98f2

…king it required Co-authored-by: Thomas Röblitz <trz42@users.noreply.github.com>

boegel commented Feb 21, 2025

View reviewed changes

Comment thread eessi_bot_event_handler.py Outdated

boegel and others added 3 commits February 21, 2025 14:15

fix order in REQUIRED_CONFIG (recommended settings before optional ones)

7da317d

add more info on use case of clone_git_repo_via set to 'ssh'

293ce3c

Co-authored-by: Thomas Röblitz <trz42@users.noreply.github.com>

also add more info on use case of clone_git_repo_via set to 'ssh' to …

668915c

…app.cfg.example Co-authored-by: Thomas Röblitz <trz42@users.noreply.github.com>

boegel force-pushed the git_clone_via_ssh branch from 181d29c to a27452d Compare February 21, 2025 13:54

boegel force-pushed the git_clone_via_ssh branch 2 times, most recently from c03e52b to 299b9e8 Compare February 21, 2025 13:58

Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…

882354c

…are-layer into git_clone_via_ssh

boegel force-pushed the git_clone_via_ssh branch from 299b9e8 to 80efece Compare February 21, 2025 14:04

only use 'git diff' to obtain PR diff when 'ssh' is used to clone rep…

a99ae60

…ository

boegel force-pushed the git_clone_via_ssh branch from 80efece to a99ae60 Compare February 21, 2025 14:08

boegel marked this pull request as draft February 21, 2025 16:25

boegel and others added 5 commits March 13, 2025 14:50

Merge pull request EESSI#306 from EESSI/develop

4d9dc0e

merge develop into main for release v0.7.0

git diff from merge-base instead of from HEAD

9b582ab

Merge pull request #1 from smoors/pr300

1ea33d9

git diff from merge-base instead of from HEAD

Merge branch 'develop' into git_clone_via_ssh

df1a258

Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…

46d14a9

…are-layer into git_clone_via_ssh

update README

f4c0940

bedroge and others added 3 commits May 23, 2025 14:20

Merge pull request EESSI#319 from EESSI/develop

95c574b

merging `develop` into `main` for release v0.8.0

Merge pull request #2 from smoors/pr300

4021c20

update README

Merge branch 'develop' into git_clone_via_ssh

4a3dd94

boegel marked this pull request as ready for review May 28, 2025 11:14

boegel added 2 commits May 28, 2025 13:43

Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…

e30cd5b

…are-layer into git_clone_via_ssh

Merge branch 'main' into git_clone_via_ssh

a985803

trz42 requested changes Jun 20, 2025

View reviewed changes

boegel added 2 commits June 20, 2025 14:13

Merge branch 'develop' into git_clone_via_ssh

1381689

add pr_diff_failure and pr_diff_tip to required configuration settings

750b8a4

trz42 approved these changes Jun 23, 2025

View reviewed changes

trz42 merged commit 50c21d7 into EESSI:develop Jun 23, 2025
4 checks passed

boegel deleted the git_clone_via_ssh branch June 24, 2025 06:52

trz42 mentioned this pull request Jun 27, 2025

Using the bot with a private repo #235

Closed

		f"{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_PR_DIFF_FAILURE]}"
		f"\n{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_PR_DIFF_TIP]}")

Conversation

boegel commented Feb 6, 2025

Uh oh!

Uh oh!

Uh oh!

smoors Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smoors Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel commented Feb 6, 2025

Uh oh!

trz42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

boegel commented Feb 21, 2025

Uh oh!

smoors commented May 4, 2025

Uh oh!

smoors commented May 22, 2025

Uh oh!

boegel commented May 28, 2025

Uh oh!

trz42 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trz42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

smoors Feb 6, 2025 •

edited

Loading

smoors Feb 6, 2025 •

edited

Loading

boegel Feb 21, 2025 •

edited

Loading