-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Checkout a specific git commit #1153
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
||
if err == nil && len(parts) > 2 { | ||
// ... retrieving the commit being pointed by HEAD | ||
_, err := r.Head() |
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 you please add more explanation around whyr.Head()
is necessary?
It would be great to add some tests around this integration.
Please see your existing Git Context integration test.
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.
@why-xn Thank you for your contribution!
In order to make sure, this integration is not broken, please add
- either unit tests or
- Integration tests
Can you also explain how feature will benefit the community?
Is the idea to build on some older commit in the git workspace?
I would find something like this useful in a CI pipeline, where the kaniko task might end being run after another commit was pushed. In that case, it should be possible to run kaniko in such a way that the image built comes from the commit the rest of the pipeline is expecting, rather than whatever commit happend to get pushed later. |
Thanks for the explanation @tomprince. Not aware how other pipeline orchestrator works. I am most familiar with tekton and it supports running a pipeline at a git commit which other tasks in pipeline (in this case kaniko will run on) |
I will note that I probably won't use this, since we have additional constraints that means we need to wrap kaniko, but might use something like this without those contraints. I'm not sure of @why-xn setup, but I know the setup we use does not support persistent volumes between different tasks in a pipeline, so would want to pin the commit here. |
Hope this get merged. I build very simple [git push -> webhook -> build] pipeline in my company.
|
@why-xn Seems like review is requested. Any updates on this? |
@why-xn Can you add some unit tests? |
Confirming that this would be a very nice and useful feature to have. Thanks for considering this ! |
Description
I have added support for checking out to a specific git commit. For this, the git url pattern should be like below.
git://[repository url][#reference][#commit-id]
Reviewer Notes
Release Notes