Question: Git Branch Source and the traits / behaviours

320 views
Skip to first unread message

Stephen Connolly

unread,
Jun 23, 2017, 9:29:20 AM6/23/17
to jenkin...@googlegroups.com, Mark Waite
So before we cut a GA release of the JENKINS-43507 changes I thought it might be a good idea to resolve one question.

To understand the context of this, we first need to acknowledge https://issues.jenkins-ci.org/browse/JENKINS-33445 

There are some users who would like the Branch source to also return Tags

Now in a sense this is perfectly reasonable. I think there are some things that need to be ironed out first before we can enable discovery of tags. To give an example, you probably do not want to trigger a build storm of all your old tags if you recreate the multibranch project... in fact if you were using the pipeline to deploy to production when it detects the build is on a tag... this would be a very bad plan.

So before we enable the discovery of tags, we need to solve some problems...

But now, let's take it as read that we have solved those problems... this means that the Git source could be configured into one of four possible states:
  1. Stupid state: discovers nothing
  2. Branches only
  3. Tags only
  4. Both branches and tags
The natural way, given the conventions in the GitHub and Bitbucket behaviours / traits is that we would have one behaviour for discovering branches and one for discovering tags...

Well right now GitSCMSource has neither, it just discovers branches always.

That means we would have to grandfather in the discovery of branches.

So if we read a GitSCMSource from disk and it has no discovery traits we would then assume it is not configured thus by the user intent on it being in the stupid state... rather they want to discover branches... the constructor would have to always add the discovery trait from the start and people would need to configure away branch discovery... it seems like a bit of a mess to me.

So what I am thinking is that we just add a Git specific "Discover Branches" behaviour to the plugin before the 3.4.0 GA release. The legacy constructor would inject it as would readResolve and the UI would always propose it when creating via the UI

It does mean that the UI will always start with Discover Branches present (taking up vertical space that is effectively useless until we get Discover Tags) but I think it will make the addition of Discover Tags much less risky

WDYT?

-Stephen

Mark Waite

unread,
Jun 23, 2017, 9:51:25 AM6/23/17
to Stephen Connolly, jenkin...@googlegroups.com
I like the idea very much.  It gives a simple place to extend the user interface in the future, and moves towards allowing independent treatment of tags and branches.

I just had a discussion (in a pull request) with someone who wants to treat a newly tagged SHA1 as meaning "build this again, even if it was built before".  I'm not very fond of that particular use model, since the git plugin treats a SHA1 that has already been built as "done", and that use model says it is the combination of SHA1 and tag which is "done", but I believe that I understand it.

I that that a user might do that, since there is meaning in a tag, and one meaning may include "build this tag, whether or not you've built this SHA1 before".
 
-Stephen

Stephen Connolly

unread,
Jun 23, 2017, 10:01:49 AM6/23/17
to Mark Waite, jenkin...@googlegroups.com


On 23 June 2017 at 14:50, Mark Waite <mark.ea...@gmail.com> wrote:
Well this is where some of the ideas I have about pre-changes prior to enabling tag discovery land... and also that I suspect tag discovery may have more than one behaviour / trait implementation and the different implementations being provided by different plugins

But the GitSCMSource needs to come from a place where it is enabled in advance for this support
 
 
-Stephen

Joseph P

unread,
Jun 23, 2017, 10:24:02 AM6/23/17
to Jenkins Developers, mark.ea...@gmail.com
Adding discover branch trait makes a lot of sense when other traits will join in.
So take the migration part now.

And if your need more vertical space, just flip your monitor into porTrait.. pun intended. :D

Stephen Connolly

unread,
Jun 23, 2017, 10:41:29 AM6/23/17
to jenkin...@googlegroups.com, Mark Waite
Doesn't look too bad in the default new-instance state:

Inline images 1

I'll roll a new set of alpha's after I finish writing the migration tests

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/c76e7027-5bbc-42f4-9ee6-bc2fa1eed920%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Jesse Glick

unread,
Jun 23, 2017, 12:43:29 PM6/23/17
to Jenkins Dev
On Fri, Jun 23, 2017 at 9:29 AM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> That means we would have to grandfather in the discovery of branches.
>
> So if we read a GitSCMSource from disk and it has no discovery traits we
> would then assume it is not configured thus by the user intent on it being
> in the stupid state... rather they want to discover branches

Surely there is some sort of `readResolve` that will get called during
this big-bang migration and you can use that as the place to insert
the “Discover Branches” trait.

> the constructor would have to always add the discovery trait from the start

That seems like the expected behavior to me—a natural default.

I am questioning why we need a “Discover Branches” trait at all. Is
there some use case for discovering tags but _not_ branches? I cannot
think of any. Which argues for a single “Discover Tags” trait, absent
by default, hence no problems here. If and when some weird person
really decides they need this, you could always add a “Do Not Discover
Branches” trait.

Jesse Glick

unread,
Jun 23, 2017, 12:46:09 PM6/23/17
to Jenkins Dev
…or, if and when implementing that RFE, introduce a single trait
“Customize Discovered References” which would have its own checkboxes
for branches, tags, and maybe even some sort of pattern match for refs
or whatever.

Stephen Connolly

unread,
Jun 23, 2017, 12:46:54 PM6/23/17
to jenkin...@googlegroups.com
But ultimately I want to make the Discover Branches trait shared across all... in which case since GitHub and Bitbucket already have that trait the counter-trait would be non-intuitive
 

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Stephen Connolly

unread,
Jun 23, 2017, 12:47:28 PM6/23/17
to jenkin...@googlegroups.com
God no... we are not going back to checkbox hell
 

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Stephen Connolly

unread,
Jun 23, 2017, 12:49:26 PM6/23/17
to jenkin...@googlegroups.com
On 23 June 2017 at 17:43, Jesse Glick <jgl...@cloudbees.com> wrote:
On Fri, Jun 23, 2017 at 9:29 AM, Stephen Connolly
<stephen.alan.connolly@gmail.com> wrote:
> That means we would have to grandfather in the discovery of branches.
>
> So if we read a GitSCMSource from disk and it has no discovery traits we
> would then assume it is not configured thus by the user intent on it being
> in the stupid state... rather they want to discover branches

Surely there is some sort of `readResolve` that will get called during
this big-bang migration and you can use that as the place to insert
the “Discover Branches” trait.

The point is that we cannot - unlike for the 3.3.0 -> 3.4.0 readResolve - rely on the traits field being null... instead we would need to introduce some "version" field in order to determine that this was a pre-discovery trait instance vs a post-discovery trait instance where the discover branches trait is explicitly excluded and some other discovery trait was added from an extension plugin
 

> the constructor would have to always add the discovery trait from the start

That seems like the expected behavior to me—a natural default.

I am questioning why we need a “Discover Branches” trait at all. Is
there some use case for discovering tags but _not_ branches? I cannot
think of any. Which argues for a single “Discover Tags” trait, absent
by default, hence no problems here. If and when some weird person
really decides they need this, you could always add a “Do Not Discover
Branches” trait.
--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

Jesse Glick

unread,
Jun 23, 2017, 1:36:15 PM6/23/17
to Jenkins Dev
On Fri, Jun 23, 2017 at 12:46 PM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> ultimately I want to make the Discover Branches trait shared across
> all... in which case since GitHub and Bitbucket already have that trait

Huh? I thought you are talking about introducing some new trait across
all the branch source plugins now.

> we are not going back to checkbox hell

Seems more intuitive to me than separate traits for each kind of
reference. Whether the physical UI is checkboxes or not, I do not
care. My point is that the default behavior of a branch source should
be to enumerate branches, as it always has done and which is all the
vast majority of users need; and if in the future you need to amend
that behavior there should be a trait with UI TBD specifying the
alternate set of references to look for.
Reply all
Reply to author
Forward
0 new messages