GitSCM: "tag:" now adds "tags/" for checkout step about bob

[jkloetzke]

This was actually a glitch which got silently fixed with this commit. It should have never worked and actually never has on a Jenkins. There "tags/" was already added since the beginning.

If I understand correctly you have a multiPackage where you set a variable that is used for the common checkoutSCM. This variable could point to any tag, branch or commit? In this case we could introduce a new "ref" property. It could have the following format and that would unify the "branch", "tag" and "commit" properties:

• -> commit
• refs/heads/ -> branch
• refs/tags/ -> tag

Would that match your use case?

[kolewu]

Yes.

I've tried to remember what the reason was, we are using tag: -- maybe it was, because branch: didn't had the error. But isn't there a different behavior for tag: and branch:? Such as branch: always updating and tag not because a tag is deterministic?

[jkloetzke]

Good point. That is an important distinction that I forgot.

I'll prepare a prototype. Not sure that I'll have it ready this week, though.

[jkloetzke]

I've pushed a prototype of the new 'rev' property to a topic branch (git-scm-rev-property). It implements a new property that unifies the branch/tag/commit properties. It does retain all properties of the existing ones, e.g. "refs/tags/..." is considered deterministic. The documentation is also available on the branch.

Could you please have a look if it works for you? If yes I would add some more unit tests and merge the feature.

[kolewu]

I have tested the prototype and it works for what we are doing.

If you had created a (WIP)-Pull-Request, it would have been easier to comment on the code ...

I don't think it's a good idea to only accept commit-IDs of full length:

            rev = spec["rev"]
            if rev.startswith("refs/heads/"):
                self.__branch = rev[11:]
            elif rev.startswith("refs/tags/"):
                self.__tag = rev[10:]
            elif len(rev) == 40:
                self.__commit = rev
            else:
                raise ParseError("Invalid rev format: " + rev)

The following test for hex numbers is sufficient as far as I am concerned.

[jkloetzke]

Cool. A pull request would certainly have made it easier. I will add some test and merge it...

Regarding the commit-ID: even though git accepts prefixes of commit IDs they have to be unambiguous. This depends on the context. One could argue that >64bits of commit ID are unique enough but I'm not sure where to draw the line. I would like to keep the full commit id for that reason. BTW, the regex is testing for 40 characters too.