@arpoch @MarkEWaite @justinharringa:matrix.org I have a doubt related to our decision of performing the binding on the git client plugin, instead of Git Plugin. (Sorry for doing this at this stage)
context: As I was looking more into how the Git Plugin resolves a git tool in a multiple nodes/different environment scenario, I can see that the Git Plugin is responsible for interacting with a
Run instance, that is, the git plugin is the place where a Jenkins job interacts with the git apis. We get the git apis from the git client plugin, it works as a utility plugin for the git plugin. The git plugin is supposed to be responsible for working with the Jenkins build and it has various utilities to handle git tool resolution or any other situation where we need to extract such information from the running Jenkins context.
doubt: I am in no way suggesting to shift the implementation at this stage but I can see that we already have a way to resolve a git tool on nodes in the git plugin, I suspect we need to replicate the same in the git client plugin (which, technically, should be unaware of what happens with the Jenkins context).
The reason why we don't have git tool resolution at the git client plugin reconfirms the fact that it is not supposed to work with the Jenkins build/instance.
isAtleastVersionmethod has package-private scope, keeping this in mind I have not made any commits for the ssh binding yet.
resolveGitToolmethod resolves/finds Git Tool by name. So if using
resolveGitToolfor our specific use case then the gitTool name used will be
Defaultas we are not taking any input from the user regarding the git Tool Name so we can't make any assumptions on what name the user will set for the git tool thus we can't supply the gitTool name for
resolveGitToolmethod which comes down to using
gitToolNameis type specific, which only matches the type with the type of tool required irrespective of name set by the user. Both the implementation
gitToolNameare somewhat same only differs in the approach of the find the git tool. Also the time complexity of both the impl will be linear once the
forNodemethod used in the
gitToolNamemethod is removed since it's not required their. So I think it's better to make a separate method in git plugin under
GitUtilsclass which resolves the git Tool name based on the type rather than name.
CliGitAPIImplclass and it seem reasonable as well since it is exclusively for cli git, also I have not added any additional test case but updated the existing
assertVersionOutputmethod to check the
compareLeastGitVersionmethod as well.
Looking forward to the git credentials binding project office hours in about 30 minutes. I've spent an hour or two testing the most recent build and have good results to share.
gitToolName, Pipeline and Freestyle jobs are working great in all the tests that I have run
gitToolName, the job fails (Freestyle and Pipeline)
Great work @arpoch !
gitToolNameto be an optional argument and to use default if an invalid value is provided. See MarkEWaite/git-plugin@4f8e4d8
doFillGitToolItemsin GitSCM provides us with names of the git tools available for the user to choose from. Could we use the same logic but return a gitTool instance instead and check it's type and decide on whether to proceed authentication or not, will it still require
resolveGitToolmethod usage. When using
resolveGitToolmethod we check for a node specific gitTool but if the user is the one selecting the gitTool then would it be too assumptious to say that they must have configured it on the agents as well.