-
Notifications
You must be signed in to change notification settings - Fork 20
add support for cloning Git repository via SSH rather than HTTPS #300
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
Merged
Merged
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
e96ec76
Merge pull request #228 from EESSI/develop
boegel 66ba1f9
Merge pull request #234 from EESSI/develop
boegel 6ba9625
Merge pull request #240 from EESSI/develop
boegel fa91fcd
Merge pull request #259 from EESSI/develop
laraPPr a5b40e9
Merge pull request #270 from EESSI/develop
Neves-P 357f9df
Merge pull request #285 from EESSI/develop
boegel a444bcc
add support for cloning Git repository via SSH rather than HTTPS
boegel 6dcdd80
fix use of config.BUILDENV_SETTING_CLONE_GIT_REPO_VIA
boegel 3f2cfcb
also use clone_git_repo_via in app.cfg.example
boegel 37c06af
mark clone_git_repo_via as optional + add logging
boegel 4b046f6
fix get_build_env_cfg to get clone_git_repo_via value from buildenv i…
boegel 45bd556
remove curl command to obtain PR diff + fix _ERROR_GIT_CLONE constant
boegel b19fe23
fix return value for 'git diff' command in download_pr function
boegel 5c51665
make sure that download_comment is defined in comment_download_pr
boegel a1c2635
fix 'git diff' command in download_repo
boegel 02f3125
fix branch name used by 'git diff' command
boegel 47e2076
Merge branch 'main' into git_clone_via_ssh
boegel c7a98f2
comment out clone_git_repo_via setting in REQUIRED_CONFIG to avoid ma…
boegel 7da317d
fix order in REQUIRED_CONFIG (recommended settings before optional ones)
boegel 293ce3c
add more info on use case of clone_git_repo_via set to 'ssh'
boegel 668915c
also add more info on use case of clone_git_repo_via set to 'ssh' to …
boegel 882354c
Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…
boegel a99ae60
only use 'git diff' to obtain PR diff when 'ssh' is used to clone rep…
boegel 4d9dc0e
Merge pull request #306 from EESSI/develop
boegel 9b582ab
git diff from merge-base instead of from HEAD
smoors 1ea33d9
Merge pull request #1 from smoors/pr300
boegel df1a258
Merge branch 'develop' into git_clone_via_ssh
boegel 46d14a9
Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…
boegel f4c0940
update README
smoors 95c574b
Merge pull request #319 from EESSI/develop
bedroge 4021c20
Merge pull request #2 from smoors/pr300
boegel 4a3dd94
Merge branch 'develop' into git_clone_via_ssh
boegel e30cd5b
Merge branch 'git_clone_via_ssh' of github.com:boegel/eessi-bot-softw…
boegel a985803
Merge branch 'main' into git_clone_via_ssh
boegel 1381689
Merge branch 'develop' into git_clone_via_ssh
boegel 750b8a4
add pr_diff_failure and pr_diff_tip to required configuration settings
boegel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,8 @@ | |
_ERROR_CURL = "curl" | ||
_ERROR_GIT_APPLY = "git apply" | ||
_ERROR_GIT_CHECKOUT = "git checkout" | ||
_ERROR_GIT_CLONE = "curl" | ||
_ERROR_GIT_CLONE = "git clone" | ||
_ERROR_PR_DIFF = "pr_diff" | ||
_ERROR_NONE = "none" | ||
|
||
# other constants | ||
|
@@ -171,6 +172,10 @@ def get_build_env_cfg(cfg): | |
log(f"{fn}(): load_modules '{load_modules}'") | ||
config_data[config.BUILDENV_SETTING_LOAD_MODULES] = load_modules | ||
|
||
clone_git_repo_via = buildenv.get(config.BUILDENV_SETTING_CLONE_GIT_REPO_VIA, None) | ||
log(f"{fn}(): clone_git_repo_via '{clone_git_repo_via}'") | ||
config_data[config.BUILDENV_SETTING_CLONE_GIT_REPO_VIA] = clone_git_repo_via | ||
|
||
return config_data | ||
|
||
|
||
|
@@ -379,7 +384,7 @@ def clone_git_repo(repo, path): | |
return (clone_output, clone_error, clone_exit_code) | ||
|
||
|
||
def download_pr(repo_name, branch_name, pr, arch_job_dir): | ||
def download_pr(repo_name, branch_name, pr, arch_job_dir, clone_via=None): | ||
""" | ||
Download pull request to job working directory | ||
|
||
|
@@ -388,6 +393,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): | |
branch_name (string): name of the base branch of the pull request | ||
pr (github.PullRequest.PullRequest): instance representing the pull request | ||
arch_job_dir (string): working directory of the job to be submitted | ||
clone_via (string): mechanism to clone Git repository, should be 'https' (default) or 'ssh' | ||
|
||
Returns: | ||
None (implicitly), in case an error is caught in the git clone, git checkout, curl, | ||
|
@@ -400,7 +406,29 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): | |
# - 'git checkout' base branch of pull request | ||
# - 'curl' diff for pull request | ||
# - 'git apply' diff file | ||
clone_output, clone_error, clone_exit_code = clone_git_repo(f'https://github.com/{repo_name}', arch_job_dir) | ||
log(f"Cloning Git repo via: {clone_via}") | ||
if clone_via in (None, 'https'): | ||
repo_url = f'https://github.com/{repo_name}' | ||
boegel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pr_diff_cmd = ' '.join([ | ||
'curl -L', | ||
'-H "Accept: application/vnd.github.diff"', | ||
'-H "X-GitHub-Api-Version: 2022-11-28"', | ||
f'https://api.github.com/repos/{repo_name}/pulls/{pr.number} > {pr.number}.diff', | ||
]) | ||
elif clone_via == 'ssh': | ||
repo_url = f'git@github.com:{repo_name}.git' | ||
pr_diff_cmd = ' && '.join([ | ||
f"git fetch origin pull/{pr.number}/head:pr{pr.number}", | ||
f"git diff $(git merge-base pr{pr.number} HEAD) pr{pr.number} > {pr.number}.diff", | ||
]) | ||
else: | ||
clone_output = '' | ||
clone_error = f"Unknown mechanism to clone Git repo: {clone_via}" | ||
clone_exit_code = 1 | ||
error_stage = _ERROR_GIT_CLONE | ||
return clone_output, clone_error, clone_exit_code, error_stage | ||
|
||
clone_output, clone_error, clone_exit_code = clone_git_repo(repo_url, arch_job_dir) | ||
if clone_exit_code != 0: | ||
error_stage = _ERROR_GIT_CLONE | ||
return clone_output, clone_error, clone_exit_code, error_stage | ||
|
@@ -417,24 +445,18 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): | |
error_stage = _ERROR_GIT_CHECKOUT | ||
return checkout_output, checkout_err, checkout_exit_code, error_stage | ||
|
||
curl_cmd = ' '.join([ | ||
'curl -L', | ||
'-H "Accept: application/vnd.github.diff"', | ||
'-H "X-GitHub-Api-Version: 2022-11-28"', | ||
f'https://api.github.com/repos/{repo_name}/pulls/{pr.number} > {pr.number}.diff', | ||
]) | ||
log(f'curl with command {curl_cmd}') | ||
curl_output, curl_error, curl_exit_code = run_cmd( | ||
curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False | ||
log(f'obtaining PR diff with command {pr_diff_cmd}') | ||
pr_diff_output, pr_diff_error, pr_diff_exit_code = run_cmd( | ||
pr_diff_cmd, "obtain PR diff", arch_job_dir, raise_on_error=False | ||
) | ||
if curl_exit_code != 0: | ||
error_stage = _ERROR_CURL | ||
return curl_output, curl_error, curl_exit_code, error_stage | ||
if pr_diff_exit_code != 0: | ||
error_stage = _ERROR_PR_DIFF | ||
return pr_diff_output, pr_diff_error, pr_diff_exit_code, error_stage | ||
|
||
git_apply_cmd = f'git apply {pr.number}.diff' | ||
log(f'git apply with command {git_apply_cmd}') | ||
git_apply_output, git_apply_error, git_apply_exit_code = run_cmd( | ||
git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False | ||
git_apply_cmd, "apply patch", arch_job_dir, raise_on_error=False | ||
) | ||
if git_apply_exit_code != 0: | ||
error_stage = _ERROR_GIT_APPLY | ||
|
@@ -481,6 +503,12 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e | |
download_comment = (f"```{download_pr_error}```\n" | ||
f"{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_GIT_APPLY_FAILURE]}" | ||
f"\n{download_pr_comments_cfg[config.DOWNLOAD_PR_COMMENTS_SETTING_GIT_APPLY_TIP]}") | ||
elif error_stage == _ERROR_PR_DIFF: | ||
download_comment = (f"```{download_pr_error}```\n" | ||
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]}") | ||
Comment on lines
+508
to
+509
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These settings should probably be added to the list of required settings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 750b8a4 |
||
else: | ||
download_comment = f"```{download_pr_error}```" | ||
|
||
download_comment = pr_comments.create_comment( | ||
repo_name=base_repo_name, pr_number=pr.number, comment=download_comment, req_chatlevel=ChatLevels.MINIMAL | ||
|
@@ -639,8 +667,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): | |
log(f"{fn}(): job_dir '{job_dir}'") | ||
|
||
# TODO optimisation? download once, copy and cleanup initial copy? | ||
clone_git_repo_via = build_env_cfg.get(config.BUILDENV_SETTING_CLONE_GIT_REPO_VIA) | ||
download_pr_output, download_pr_error, download_pr_exit_code, error_stage = download_pr( | ||
base_repo_name, base_branch_name, pr, job_dir | ||
base_repo_name, base_branch_name, pr, job_dir, clone_via=clone_git_repo_via, | ||
) | ||
comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage) | ||
# prepare job configuration file 'job.cfg' in directory <job_dir>/cfg | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.