Skip to content

check the commit is remotely available #696

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 1 commit into from
Apr 30, 2025
Merged

Conversation

glehmann
Copy link
Contributor

before asking koji to pull a commit it may not be able to retrieve

@glehmann glehmann requested review from stormi and ydirson April 24, 2025 10:10
Copy link
Member

@stormi stormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/remotly/remotely/

Copy link
Contributor

@ydirson ydirson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • this is only when not pushing a temporary commit, where we assumed it was already pushed; this might be worth to explain in the commit message

  • there is a "remotly" typo in the commit message (and PR)

  • the random -tlpz suffix is puzzling :)

Comment on lines 39 to 40
def check_commit_is_available(hash):
subprocess.check_output(['git', 'branch', '-r', '--contains', hash])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I read "is_available" as "commit is locally present", but the commit messages adds "remote" (better function naming, and docstring, would help)
  • the command used would in fact only fail if the commit is not locally present:
    user@work-vates:xcp (master =)$ git branch -r --contains f7f53d8163b820d4f133840b93c149ac43a3206c
    user@work-vates:xcp (master =)$ echo $?
    0
    user@work-vates:xcp (master =)$ git branch -r --contains deccfa331b78491d565f6daf1a1cc89f9b76984d
    error: no such commit deccfa331b78491d565f6daf1a1cc89f9b76984d
    user@work-vates:xcp (master =)$ 
    

@glehmann glehmann force-pushed the gln/koji-remote-check-tlpz branch from 384cd2b to 4e45ef2 Compare April 24, 2025 12:46
@glehmann
Copy link
Contributor Author

glehmann commented Apr 24, 2025

* the random `-tlpz` suffix is puzzling :)

maybe it's not random ;-)

@glehmann glehmann changed the title check the commit is remotly available check the commit is remotely available Apr 24, 2025
@glehmann glehmann requested a review from ydirson April 24, 2025 12:50
@glehmann glehmann force-pushed the gln/koji-remote-check-tlpz branch from 4e45ef2 to 2674d97 Compare April 25, 2025 08:51
Comment on lines 39 to 43
def check_commit_is_available_remotely(dirpath, hash):
with cd(dirpath):
subprocess.check_output(['git', 'branch', '-r', '--contains', hash])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function still does not work as advertised for the reason exposed in my second bullet point:

[builder@81dcb03d8599 samba]$ /data/src/xcpng/xcp/scripts/koji/koji_build.py --scratch v8.3-u-ydi1 .
koji  build  --scratch  v8.3-u-ydi1  git+https://github.com/xcp-ng-rpms/samba?#ab8f57a8b8bf52a5b90b82f6383042cad59c2da4
Created task: 82908
Task info: http://koji.xcp-ng.org/taskinfo?taskID=82908
...
82908 build (v8.3-u-ydi1, /xcp-ng-rpms/samba:ab8f57a8b8bf52a5b90b82f6383042cad59c2da4): open (koji.xcp-ng.org) -> FAILED: BuildError: Error running GIT command "git reset --hard ab8f57a8b8bf52a5b90b82f6383042cad59c2da4", see checkout.log for details

checkout.log:

$ git reset --hard ab8f57a8b8bf52a5b90b82f6383042cad59c2da4
fatal: Could not parse object 'ab8f57a8b8bf52a5b90b82f6383042cad59c2da4'.

before asking koji to pull a commit it may not be able to retrieve.
The check is only done when not using a pre/test build commit. In that
case we are already assured that the commit is on the remote repository.

Signed-off-by: Gaëtan Lehmann <[email protected]>
@glehmann glehmann force-pushed the gln/koji-remote-check-tlpz branch from 2674d97 to a24751c Compare April 25, 2025 13:06
@glehmann glehmann requested a review from ydirson April 25, 2025 13:59
@glehmann glehmann merged commit e591558 into master Apr 30, 2025
1 check passed
@glehmann glehmann deleted the gln/koji-remote-check-tlpz branch April 30, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants