Extending SCMTrigger

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Extending SCMTrigger

Matt Tucker-4
Extending SCMTrigger

I need a version of SCMTrigger that supports excludes of certain patterns (because our build process updates a version descriptor in the project and commits, which causes SCMTrigger to think there's been a change, which triggers a build…). I thought to start with extending the existing SCMTrigger to add that functionality before working it into a patch. The trouble I'm running into, though, is this:

public class SCMTrigger extends Trigger<SCMedItem> {
. . .
    public DescriptorImpl getDescriptor() {
        return DESCRIPTOR;
    }

    public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl();

    public static final class DescriptorImpl extends TriggerDescriptor {
    . . .

I can't extend this DescriptorImpl in my own class because it's final, and if I write my own descriptor (which I don't really want to do because there's actually a lot of functionality in there), I can't override getDescriptor() because the return types would be incompatible.

Is making the descriptor classes final a normal pattern? Is there any reason it's necessary, or that keeping them extendable would cause a problem? As things are now, I have to completely rewrite the SCMTrigger class to get the functionality I need.

Reply | Threaded
Open this post in threaded view
|

Re: Extending SCMTrigger

Brendt
Matt Tucker-4 wrote
I need a version of SCMTrigger that supports excludes of certain
patterns (because our build process updates a version descriptor in the
project and commits, which causes SCMTrigger to think there's been a
change, which triggers a build...). I thought to start with extending
the existing SCMTrigger to add that functionality before working it into
a patch. The trouble I'm running into, though, is this:

public class SCMTrigger extends Trigger<SCMedItem> {
. . .
    public DescriptorImpl getDescriptor() {
        return DESCRIPTOR;
    }

    public static final DescriptorImpl DESCRIPTOR = new
DescriptorImpl();

    public static final class DescriptorImpl extends TriggerDescriptor {
    . . .

I can't extend this DescriptorImpl in my own class because it's final,
and if I write my own descriptor (which I don't really want to do
because there's actually a lot of functionality in there), I can't
override getDescriptor() because the return types would be incompatible.

Is making the descriptor classes final a normal pattern? Is there any
reason it's necessary, or that keeping them extendable would cause a
problem? As things are now, I have to completely rewrite the SCMTrigger
class to get the functionality I need.
From my understanding, it is not SCMTrigger's responsibility to know whether or not to kick off a build.  It pushes that to the SCM's implementation of pollChanges(...).  I have already hacked up something to work with CVS as a repo (see issue 848).  I have submitted a patch, but haven't had word back yet as to whether or not that will go into the core of Hudson.  Kohsuke, have you had a chance to look into that yet?

B
Reply | Threaded
Open this post in threaded view
|

RE: Re: Extending SCMTrigger

Matt Tucker-4
Brendt wrote:
> From my understanding, it is not SCMTrigger's responsibility to know
> whether or not to kick off a build.  It pushes that to the SCM's
> implementation of pollChanges(...).  I have already hacked up
> something to work with CVS as a repo (see issue 848).  I have
> submitted a patch, but haven't had word back yet as to whether or not
> that will go into the core of Hudson.  Kohsuke, have you had a chance
> to look into that yet?

After I wrote that email, I discovered that all the work is done in
SubversionSCM (and friends), but that really pushes the problem down to
that layer instead, since I can't extend that one either because of the
whole final Descriptor thing.

For my particular case, I decided to try to fix the issue by having a
build step that calls SCM->pollChanges(), which should update the
current version cache before the build finishes, which should prevent
the trigger from picking up changes that were made during the build.

But in the general case, it seems like I ought to be able to extend
these base classes (like SCMTrigger and SubversionSCM) to add my own
functionality, and I'm wondering what the basis was for the design
decision that prevents me from doing so.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Extending SCMTrigger

Kohsuke Kawaguchi
Administrator

Interesting point.

I guess the problem is that DescriptorImpl class is really not written
to allow for sub-typing. For one thing, your version would ideally have
to extend from SCMDescriptor<YourSubversionSCM>, although may be you can
cheat around this.

It's not clear to me what the design pattern in general should be to
enable subtyping, but as a starter, I committed rev.10232 and removed
final. Let me know if you hit another problem.


Matt Tucker wrote:

> Brendt wrote:
>> From my understanding, it is not SCMTrigger's responsibility to know
>> whether or not to kick off a build.  It pushes that to the SCM's
>> implementation of pollChanges(...).  I have already hacked up
>> something to work with CVS as a repo (see issue 848).  I have
>> submitted a patch, but haven't had word back yet as to whether or not
>> that will go into the core of Hudson.  Kohsuke, have you had a chance
>> to look into that yet?
>
> After I wrote that email, I discovered that all the work is done in
> SubversionSCM (and friends), but that really pushes the problem down to
> that layer instead, since I can't extend that one either because of the
> whole final Descriptor thing.
>
> For my particular case, I decided to try to fix the issue by having a
> build step that calls SCM->pollChanges(), which should update the
> current version cache before the build finishes, which should prevent
> the trigger from picking up changes that were made during the build.
>
> But in the general case, it seems like I ought to be able to extend
> these base classes (like SCMTrigger and SubversionSCM) to add my own
> functionality, and I'm wondering what the basis was for the design
> decision that prevents me from doing so.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Extending SCMTrigger

Lloyd Chang
Hi Matt, I came across something similar earlier thinking about how to
extend the URL Change Trigger.

Options for you to pursue:

a) extend SCMTrigger using Kohsuke's change r10232

b) If org.tmatesoft.svn support "svn up --ignore-externals" (I don't
see a way to specify "--ignore-externals" in Hudson's Subversion
configuration though), you can make your version description commits
into svn:external.  Build machines would ignore commits of the version
descriptor.

c) If you don't mind living with 1 extra build after each success, you
can write your script to...

src = your sources, verdesc = your version descriptor

hudson triggered by src + verdesc
hudson builds changes
if verdesc.base = null, then svn commit verdesc, and copy verdesc to
verdesc.base
if verdesc.base != null, then svn diff verdesc with verdesc.base, and
svn commit verdesc (but don't commit verdesc.base) only if diff =
true, then copy verdesc to verdesc.base

d) Unfortunately, svn:ignore won't do what you want, but there is an
svn:template feature request for the situation you described, filed at
http://subversion.tigris.org/issues/show_bug.cgi?id=2858

If my chicken scratch above is unclear, let me know.  -- Lloyd

On Thu, Jun 19, 2008 at 7:10 PM, Kohsuke Kawaguchi
<[hidden email]> wrote:

>
> Interesting point.
>
> I guess the problem is that DescriptorImpl class is really not written to
> allow for sub-typing. For one thing, your version would ideally have to
> extend from SCMDescriptor<YourSubversionSCM>, although may be you can cheat
> around this.
>
> It's not clear to me what the design pattern in general should be to enable
> subtyping, but as a starter, I committed rev.10232 and removed final. Let me
> know if you hit another problem.
>
>
> Matt Tucker wrote:
>>
>> Brendt wrote:
>>>
>>> From my understanding, it is not SCMTrigger's responsibility to know
>>> whether or not to kick off a build.  It pushes that to the SCM's
>>> implementation of pollChanges(...).  I have already hacked up
>>> something to work with CVS as a repo (see issue 848).  I have
>>> submitted a patch, but haven't had word back yet as to whether or not
>>> that will go into the core of Hudson.  Kohsuke, have you had a chance
>>> to look into that yet?
>>
>> After I wrote that email, I discovered that all the work is done in
>> SubversionSCM (and friends), but that really pushes the problem down to
>> that layer instead, since I can't extend that one either because of the
>> whole final Descriptor thing.
>>
>> For my particular case, I decided to try to fix the issue by having a
>> build step that calls SCM->pollChanges(), which should update the
>> current version cache before the build finishes, which should prevent
>> the trigger from picking up changes that were made during the build.
>>
>> But in the general case, it seems like I ought to be able to extend
>> these base classes (like SCMTrigger and SubversionSCM) to add my own
>> functionality, and I'm wondering what the basis was for the design
>> decision that prevents me from doing so.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>
> --
> Kohsuke Kawaguchi
> Sun Microsystems                   [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]