[JENKINS-70303] Remove leading and trailing spaces from refspec

[Jagrutiti]

JENKINS-70303 - Remove leading and trailing spaces from refspec

If a refspec is included in the checkout scm definition of a multibranch Pipeline using JGit for checkout and the refspec has a leading space character, the checkout fails with the JGit message

Remote does not have  available for fetch.

The same refspec with leading spaces does not affect command line git checkout. It works in either case with command line git. Leading space handling by command line git is preferred rather than the way that the git client plugin honors the leading spaces and passes them to JGit.

Command line git does not see the issue because extra spaces between arguments of a command line program are generally not an issue. Lucky circumstances that allow the CLI git checkout to be better behaved than the JGit checkout.

Checklist

• [x] I have read the CONTRIBUTING doc
• [x] I have referenced the Jira issue related to my changes in one or more commit messages
• [x] I have added tests that verify my changes
• [x] Unit tests pass locally with my changes
• [x] I have added documentation as necessary
• [x] No Javadoc warnings were introduced with my changes
• [x] No spotbugs warnings were introduced with my changes
• [x] I have interactively tested my changes

Types of changes

What types of changes does your code introduce?

• [x] Bug fix (non-breaking change which fixes an issue)

[Jagrutiti]

I have tested this PR using mvn clean install. All the test cases in CliGitAPIImplTest.java pass.

How shall I write a test case of my own? I mean how do I test refspecs?

[Jagrutiti]

Thanks very much for the pull request. The two cases seem reasonable at first glance. The automated tests that you'll create should help confirm that they are sufficient to resolve the issue.

Have you tested this interactively based on the description in the issue report?

I think that you can add an automated test in the GitClientCloneTest. It already has the test_clone_refspecs() test that defines a new RefSpec. You can modify that test to sometimes include leading or trailing spaces on the RefSpec or you can copy that test and test for spaces in the RefSpec in the copied test.

I've provided a sample of the way I would write the test and pushed it to your branch as 2b04ed4 . The test is currently failing for JGit. I think that means there are more places that will need to be changed.

I also added the Issue annotation to the modified test with a comment explaining the change.

I found that location by using git grep RefSpec -- src/test to search for existing uses of RefSpec in the tests. That test seemed like the easiest one to extend for this case. You should do the same search and confirm that my choice was reasonable. There may be other tests that assert RefSpec contents that could be extended in a similar way.

I will work on this. Thanks.

[MarkEWaite]

I've included the issue description in the pull request description because it helps reviewers to have the issue described clearly in the pull request, without requiring that they open the bug report. That's my personal preference rather than a standard across the entire Jenkins organization, but since I'm one of the maintainers of this plugin, I applied my preference.

[Jagrutiti]

I've included the issue description in the pull request description because it helps reviewers to have the issue described clearly in the pull request, without requiring that they open the bug report. That's my personal preference rather than a standard across the entire Jenkins organization, but since I'm one of the maintainers of this plugin, I applied my preference.

I noticed. I had added the description in the PR title itself.

[Jagrutiti]

I was not able to fix the test case for jgit and jgitapache. I know the test executes this part of the code. https://github.com/jenkinsci/git-client-plugin/blob/31ecb75f53a9b222d0145b774572a87714dd5403/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java#L1547-L1565

[MarkEWaite]

I was not able to fix the test case for jgit and jgitapache. I know the test executes this part of the code.

At the location where you're looking, the RefSpec object already includes the leading space. It seems like it would be better to look at the locations where RefSpec objects are constructed. Trim the spaces before passing the string to the RefSpec constructor.

I'm making a guess by that idea, but I think it is a guess that is worth exploring.

[MarkEWaite]

Interactive testing of the pull request shows that a leading space in the refspec from the UI still fails to clone. The job definition is:

<project>
  <actions/>
  <description/>
  <keepDependencies>false</keepDependencies>
  <properties/>
  <scm class="hudson.plugins.git.GitSCM" plugin="[email protected]">
    <configVersion>2</configVersion>
    <userRemoteConfigs>
      <hudson.plugins.git.UserRemoteConfig>
        <name>origin</name>
        <refspec> +refs/heads/3.11.x:refs/remotes/origin/3.11.x</refspec>
        <url>https://github.com/jenkinsci/git-client-plugin</url>
      </hudson.plugins.git.UserRemoteConfig>
    </userRemoteConfigs>
    <branches>
      <hudson.plugins.git.BranchSpec>
        <name>3.11.x</name>
      </hudson.plugins.git.BranchSpec>
    </branches>
    <doGenerateSubmoduleConfigurations>false</doGenerateSubmoduleConfigurations>
    <gitTool>jgit</gitTool>
    <submoduleCfg class="empty-list"/>
    <extensions/>
  </scm>
  <canRoam>true</canRoam>
  <disabled>false</disabled>
  <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
  <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
  <triggers/>
  <concurrentBuild>false</concurrentBuild>
  <builders/>
  <publishers/>
  <buildWrappers/>
</project>

[Jagrutiti]

hudson.plugins.git.UserRemoteConfig

I have found this link for UserRemoteConfig: https://javadoc.jenkins.io/plugin/git/hudson/plugins/git/UserRemoteConfig.html

But not able to find where it is called in the code.

[Jagrutiti]

Hi @MarkEWaite

I have updated almost all the references I found for refspecs.

One of the test cases is failing because of:

ERROR]   GitClientFetchTest.test_fetch:256 » JGitInternal Exception caught during execution of merge command. org.eclipse.jgit.errors.MissingObjectException: Missing unknown dadd5f9e276467580ddb00c2403eec0e9501a6e3
[ERROR]   GitClientFetchTest.test_fetch:256 » JGitInternal Exception caught during execution of merge command. org.eclipse.jgit.errors.MissingObjectException: Missing unknown cb6a175eac031a7acf8270842a6eeda2ef1f27ad

I used debugger. It fails at this line:

newAreaWorkspace.getGitClient().merge().setRevisionToMerge(bareCommit3).execute();

but then the next line:

assertThat("bare3 != newArea3", newAreaWorkspace.getGitClient().revParse("HEAD"), is(bareCommit3)); has the following id as shown by the debugger.

AnyObjectId[224664d7fc3196baca45c60b06a6b269cea76932]

What should I fix here?