- Notifications
You must be signed in to change notification settings - Fork2.5k
Implement push options on push#6439
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Push options are an optional capability of a git server. If they arelisted in the servers capabilities, when the client lists push-optionsin it's list of capabilities, then the client sends it's list of pushoptions followed by a flush-pkt.So, If we have any declared push options, then we will list it as aclient capability, and send the options.If the request contains push options but the server has no pushoptions capability, then error.
We found that the best way to test push options was to receive them via a git hooks script, and output them to a location that the CI tests could read to check and see if the push options were interpreted by git correctly.Co-Authored-By: pireads <pireads@gmail.com>Co-Authored-By: lotdeef <lcfisch2@asu.edu>Co-Authored-By: PSI497 <497.psi.497@gmail.com>
Also, push options for local push can't be supported until hooks are implemented for local repos, i didn't look into this too much. It should be added separately i think to prevent this PR growing too much |
If you look into hooks it could be a good idea to check the security issue mentioned in#6400 to see if it applies. |
Since we use `git_push_options` as the options structure to our`git_push` command, much like we do everywhere else, "push_options"becomes ambiguous. "remote_options" isn't much better for us. Call them"remote_push_options", which is still quite bad, and not particularlyinsightful for end users, but at least something that we can discussunambiguously.
Keep the push options tests more constrained to our CI environment;writing files within the test sandbox instead of outside of it. Providethe path to the output file in the test data. In addition, add therepository to the test resources instead of recreating the hooks everytime.
Thank you so much for pulling this together. This is great work by you and the other contributors. I apologize that it took so long to address this PR, but I love the simplifications that you made. I merged Thanks again - I think that this is a nice feature and I appreciate you and everybody else who went above and beyond to add integration tests. 🙏 |
This PR implements passing push options on push if the server supports the capability. Push options are sent after the initial handshakehttps://github.com/git/git/blob/master/Documentation/gitprotocol-pack.txt#L563
I implemented this because SourceHut uses push-options to skip CI, but after looking at#5335 I see that GitLab has the same feature. Gitea implemented a small amount of functionality using push-options, but it's not as critical for it.
Unfortunately I wasn't aware of PR#6281 but I did find it before I implemented any tests, so I have rebased and improved the tests suite from that PR.
This PR adds a field to the push_options struct called push_options, this might seem a bit confusing, but push-options are the name of the options in the Git CLI. Though the name could be shortened to options if that was clearer.
The test suite can only be run on focal or newer Ubuntu's. This is because older versions of the git CLI doesn't support receiving push options (or doesn't advertise the capability). When testing libgit2 against focal, I found that SSH doesn't work, it looks like an incompatibility between libssh2 and openssh, with the logs up to debug3 I was able to spot some differences between the ssh client an libssh2, but i really don't understand ssh well enough to know why the connection is rejected. I tried upgrading libssh2 to 1.10.0, but that didn't help.
fixes#5335