Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-21342

CliGitAPIImpl is hard-coded to use LocalLauncher

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      We have a custom Jenkins plugin that executes git commands using GitAPI, for example launchCommand("pull", myRemoteRepo, myBranch).
      When a build is launched on a slave the build fails because the git commands attempt to run on the Jenkins master instead of the slave.

      Our workaround was to replace the LocalLauncher created by CliGitAPIImpl with the Launcher provided by Jenkins during setup (using reflection), so that builds on slaves have a RemoteLauncher. It would be preferable to have a constructor in GitAPI that takes an instance of Launcher so that we do not have to code against CliGitAPIImpl directly.

        Attachments

          Issue Links

            Activity

            Hide
            ndeloof Nicolas De Loof added a comment -

            GitAPI is deprecated, prefer GitClient. GitClient already handle remoting transparently.

            Also, git-client plugin is designed to abstract git operations and let us provide alternate implementation (running command line cli is fragile), first one begin JGit with significant limitations, maybe a libgit2 JNA wrapper will be implemented on day or the other...

            Show
            ndeloof Nicolas De Loof added a comment - GitAPI is deprecated, prefer GitClient. GitClient already handle remoting transparently. Also, git-client plugin is designed to abstract git operations and let us provide alternate implementation (running command line cli is fragile), first one begin JGit with significant limitations, maybe a libgit2 JNA wrapper will be implemented on day or the other...
            Hide
            dmjacobsen Doug Jacobsen added a comment -

            Hello,

            I have a BuildWrapper which modifies the Launcher in order to allow processes to function properly on the system (adjusting environment and command line per local policy requirements).

            Since CliGitAPIImpl uses it's own LocalLauncher, instead of accepting the Launcher from the the Job instance, the build wrapper is ignored - and thus the commands crash.

            Would it be possible for CliGitAPIImpl to use the build launcher instead of instantiating it's own LocalLauncher?

            Thanks,
            Doug

            p.s., please note my use of CliGitAPIImpl is not direct via the API, rather it is coming from use of the Git Plugin to clone/pull/checkout a repo setup in the job.

            Show
            dmjacobsen Doug Jacobsen added a comment - Hello, I have a BuildWrapper which modifies the Launcher in order to allow processes to function properly on the system (adjusting environment and command line per local policy requirements). Since CliGitAPIImpl uses it's own LocalLauncher, instead of accepting the Launcher from the the Job instance, the build wrapper is ignored - and thus the commands crash. Would it be possible for CliGitAPIImpl to use the build launcher instead of instantiating it's own LocalLauncher? Thanks, Doug p.s., please note my use of CliGitAPIImpl is not direct via the API, rather it is coming from use of the Git Plugin to clone/pull/checkout a repo setup in the job.
            Hide
            markewaite Mark Waite added a comment -

            Can you provide a pull request to the git-client-plugin (with tests) for your proposed change?

            Show
            markewaite Mark Waite added a comment - Can you provide a pull request to the git-client-plugin (with tests) for your proposed change?
            Hide
            rgerard Rusty Gerard added a comment -

            My PR didn't change the default behavior but did allow a BuildWrapper to instantiate the client with the build's launcher:

            https://github.com/jenkinsci/git-client-plugin/pull/70

            Show
            rgerard Rusty Gerard added a comment - My PR didn't change the default behavior but did allow a BuildWrapper to instantiate the client with the build's launcher: https://github.com/jenkinsci/git-client-plugin/pull/70
            Hide
            markewaite Mark Waite added a comment -

            Sorry, I didn't realize Nicolas had already reviewed and closed your pull request. He's correct that GitAPI is deprecated, and your pull request seems to add another deprecated constructor. I'm unwilling to go against Nicolas' judgment. He has significantly more experience with the git-client-plugin.

            Show
            markewaite Mark Waite added a comment - Sorry, I didn't realize Nicolas had already reviewed and closed your pull request. He's correct that GitAPI is deprecated, and your pull request seems to add another deprecated constructor. I'm unwilling to go against Nicolas' judgment. He has significantly more experience with the git-client-plugin.
            Hide
            dmjacobsen Doug Jacobsen added a comment -

            Hi again,

            I'm understand that the GitAPI is deprecated - but since it is still used by the Git Plugin, not having this pull request means that the Git Plugin isn't usable on systems or use-cases requiring build wrappers. Is there an alternative to the Git Plugin which doesn't use the now-deprecated GitAPI?

            Thanks,
            Doug

            Show
            dmjacobsen Doug Jacobsen added a comment - Hi again, I'm understand that the GitAPI is deprecated - but since it is still used by the Git Plugin, not having this pull request means that the Git Plugin isn't usable on systems or use-cases requiring build wrappers. Is there an alternative to the Git Plugin which doesn't use the now-deprecated GitAPI? Thanks, Doug
            Show
            ndeloof Nicolas De Loof added a comment - please see https://github.com/jenkinsci/git-client-plugin/pull/186
            Hide
            jglick Jesse Glick added a comment -

            This is a bit older but JENKINS-30600 has more activity and is closer to what people are likely to search for.

            Show
            jglick Jesse Glick added a comment - This is a bit older but JENKINS-30600 has more activity and is closer to what people are likely to search for.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              rgerard Rusty Gerard
              Votes:
              2 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: