alexanderlink edited a comment on Bug JENKINS-20767

Hello Mark,

(Pull Request https://github.com/jenkinsci/git-client-plugin/pull/129)

I tested git-client 1.7.0 together with git 2.1.0 (I assume newer versions are also affected, but I'll check).
I obviously missed something during tests. We have the same issue we had with namespace branches again with the "master" branch. The CI job is triggered again and again.
The problem is that "master" still matches for "refs/heads/master", "refs/heads/abc/master", "refs/heads/xyz/master", etc. Since git ls-remote lists the refs alphabetically the first entry might be the correct one by chance - or not.
I extended the test GitAPITestCase.test_getHeadRev_namespaces().

When I tried to fix this issue I recognized that there are several additional problems with the "Branch Specifier" handling!
First of all you have to be aware that it is possible to create weird branch names containing words which should be reserved by Git but are not.
E.g. you can create a branch "refs/heads/master" (being refs/heads/refs/heads/master) or a branch "origin/master" (being refs/heads/origin/master), etc.
A remote name also can contain weird stuff and slashes! E.g. a remote can be called "origin/master"!

Even without this knowledge it was quite hard to definitely "guess" what is meant by a configured "Branch Specifier". Even git itself has problems with ambiguous specs. Try the following in a repo having a remote "origin" and branch "master":
git branch origin/master
git checkout origin/master
-> warning: refname 'origin/master' is ambiguous.

Therefore it is necessary to specify concrete rules which (from my perspective) did not exist yet.
>From my perspective several potential variations have to be checked and depending on if it makes sense in the current repo and a rev could be identified the matching result is taken.
In org.jenkinsci.plugins.gitclient.LegacyCompatibleGitAPIImpl.normalizeBranchSpec(String) you can see how the proposed rules look like:

  • If a branch spec starts with "refs/" try to get the revision for exactly this String.
  • If a branch spec starts with "remote/" check if a remote with this name exists and build String "refs/heads/<branchName" definite for ls-remote
  • If both was not the case check again if the branch spec starts with a valid remote (e.g. "origin/master")
  • As ultimate fallbacks try "refs/heads/<branchSpec>" to check if a branch named like this exists (remember the weird possible names - we should not spend too much thoughts, just try)
  • ... "refs/heads<branchSpec>" if wildcards are used (e.g. "*/master" -> "refs/heads*/master". "refs/heads/*/master" would not match the "master" branch)
  • ... and simply "<branchSpec>" as final fallback.

The patterns are tried starting with the upper one. Ther first one which results in a definite result is considered to be the correct one.

Furthermore I now throw an AmbiguousResultException in case the result is ambiguous.
Until now simply the first entry of "git ls-remote" is taken which is way too randomly considering the alphabetical sorting.

I hope you see my points. I spent over a week now to understand and fix those issues
Maybe the tests and changes in the Pull Request show what I mean.

Thanks a lot and kind regards,
Alex

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

--
You received this message because you are subscribed to the Google Groups "Jenkins Issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to