incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Olemis Lang <ole...@gmail.com>
Subject Re: [VOTE] Release Apache Bloodhound 0.1.0 (incubating) (RC1)
Date Fri, 03 Aug 2012 07:56:58 GMT
Hi Ethan !

Thanks for your message . Some comments below.

On 8/2/12, Ethan Jucovy <ethan.jucovy@gmail.com> wrote:
> -1
>
[...]

>
> Specifically, Bloodhound already depends on a patched copy of Trac which
> lives in the Bloodhound SVN repository under
> https://svn.apache.org/repos/asf/incubator/bloodhound/trunk/trac/ and is
> distributed with the release.

yes you are right . Some changes have been needed in order to make
Bloodhound work. Each and every one of them can be considered a last
recourse alternative . We know (or have a good idea) of the
implications of patching Trac source code , and duplicate effort
needed if those patches do not make their way upstream so we try to
avoid doing that . Indeed , if you take a look at Bloodhound plugins
code and pending patches submitted to the issue tracker you'll notice
that many of the APIs have been strongly influenced by the decision of
not to modify Trac source code (if you need a particular example ,
please take a look at #94 and multiproduct plugin ; they both would
require major refactorings of Trac core , but we have been lucky so
far and worked successfully around current limitations)

> This raises a number of questions:
>
>  * Some of the files in Bloodhound's modified copy of Trac core contain
> "Licensed to the Apache Software Foundation" headers.
>  (trac/utils/tests/introspection.py and trac/util/introspection.py are the
> ones I've noticed, but I didn't grep the repository.)

well I did , and this is what I found (a summary of course ;)

1. a lot of files (e.g. docs) contain Apache word as a reference to httpd
    web server so I'll filter that part as I see that content in Trac too
    and I consider that mentioning product name this way should
    not be a violation of license statement (... and if they are
    like I said, please forward your request to trac-dev ML since
    they wrote that part , not us) ... so I filtered grep result
    set this way

{{{
#!sh

$ grep -r Apache . | grep -v "\.svn" | grep -v "default-pages" | grep Apache
./INSTALL: * A web server capable of executing CGI/FastCGI scripts, or Apache
./trac/htdocs/js/excanvas.js:// Licensed under the Apache License,
Version 2.0 (the "License");
./trac/util/introspection.py:#  Licensed to the Apache Software
Foundation (ASF) under one
./trac/util/introspection.py:#  to you under the Apache License,
Version 2.0 (the
./trac/util/tests/introspection.py:#  Licensed to the Apache Software
Foundation (ASF) under one
./trac/util/tests/introspection.py:#  to you under the Apache License,
Version 2.0 (the
./trac/web/main.py:    # SCRIPT_URL is an Apache var containing the
URL before URL rewriting
./trac/web/_fcgi.py:            # this with Apache's mod_env [not
loaded by default in OS X
./UPGRADE:handler in the Apache HTTPD configuration::

}}}

2. Beyond that , the first file with an Apache license statement is
    ./trac/htdocs/js/excanvas.js . It was committed to Trac's repos
    for the first time in trac:changeset:10330 . It's Google's as
    I can see in trac:browser:/trunk/trac/htdocs/js/excanvas.js .
    I also noticed this

{{{
#!sh

$ ls -l trac/htdocs/js/excanvas.js
-rwxrwxrwx 1 billgates billgates 18220 2012-07-02 08:50
trac/htdocs/js/excanvas.js

}}}

3. We move forward . The second file is
    ./trac/util/introspection.py . I think we committed it to
    support the case of sub-classing TicketModule and
    some other stuff . TBH they are quite unlikely to
    be accepted upstream sue to the fact that it
    contains helper functions not used by Trac itself ,
    in first place , and also their at aimed at doing
    something that Trac developer (almost ?) never do
    themeselves , and afaicr they do not recommend (i.e.
    subclass existing components) . Besides

{{{
#!sh

-rwxrwxrwx 1 billgates billgates 1344 2012-07-04 22:30
trac/util/introspection.py

}}}

4. finally consider the fact that we have applied bigger
    modifications on other files and did not change license
    headers at all .

so ...

> I think this is
> contrary to previous claims that Bloodhound's developers would keep all
> Trac modifications BSD-licensed,

oh no ! you are not accurate here . My conclusions are :

1. you have one of Google's files (> 10kB) in Trac
    repos containing AL v2 icense header .
    How many times did you complain about that ?
2. there's another file we introduced in there (< 2kB)
    containing code that
    2.1 is never going to make its  journey upstream
          IMO,
    2.2 afaics no Trac developer did anything about it .

so my questions are (my curiosity prevails here) :

1. what's the reason for so much noise ?
2. if we suggest a patch upstream containing a whole
    new file we did from scratch . Why is it that you
    complain about it whereas Google's source code
    released under the terms of the same license
    is accepted with pleasure ?

<OT>
in any case Gary , would you mind to consider moving that file to
dashboard plugin ?
</OT>

> and that the individual authors would
> retain their copyright over these lines of code, for the sake of keeping
> Trac's BSD license intact for all upstream patches[1].

that's the case even if we had no time yet to announce our patches in
trac-dev ... afaicr

but let's focus on my previous question once again

3. in case we are the original authors (the project I mean)
    like in (2) why can't we propose a file with AL v2 header ?

[...]
>
>  * What tag/revision of Trac core is the code diverging from?  Where is
> this documented?

yes, we need to improve on this . Actually maybe for 0.2.0

> How often do Bloodhound's developers merge in upstream
> changes, and what is their policy for deciding when to merge upstream
> changes?
>

so far afaicr the only update in vendor branch was made in order to
close a ticket related to BatchModify plugin . So if there's no such
policy at the time I suggest this one based on what's happened so far
:

1- at a given time (e.g. project creation) if trac-dev is unstable we
    merge trunk right away
2- if there are relevant modifications upstream allowing as to close
    a ticket then merge them right away .
3- once stable Trac releases are announced stick to using them
    for as long as possible (<= in order to test Bloodhound code
    using stable release deployed by Trac users in target servers)
4- if we decide to release a new tarball and it works with latest
    Trac stable release , that's the one.
5- goto step 2

... I think this deserves a whole new thread forked from here , if you follow .

>  * The patched copy of Trac seems to have a few-hundred-line diff from the
> official Trac distribution.  (I'm not sure the best way to measure this.)
>  Where are these patches (and their intentions, and the Bloodhound code
> that relies on each patch) being tracked and documented as core patches?
>  What procedures are in place to ensure that each core patch is being
> tracked and documented?
>

Gary mentioned something about using subversion vendor branch best
practices for this purpose . The rationale for doing so should be
documented in the issue tracker . Besides every (recent) log message
contains a reference to a ticket number . A tour around the issue
tracker might be all you need . Isn't that enough ?

>  * Have all of these patches been submitted upstream to the Trac core
> community?

No , so far we have had no time to do so , but that was on the schedule ;)

> Where are the upstream submissions being tracked and
> documented?

<joke>
objection !
there's no child born yet , there's neither birth date nor a record ,
we all are still innocent your Honor
</joke>

> (I tried several searches on trac.edgewall.org and couldn't
> find any such tickets.)  What procedures are in place to ensure that each
> core patch is filed upstream, and to track the upstream status of each core
> patch?
>

it seems to me that not all patches will go upstream , mainly because
some of them will never be committed by trac-dev (due to their quality
standards, policies, etc ...)

>  * If the Trac core developers reject a patch or suggest an alternate
> approach, will Bloodhound's developers take that advice and rework their
> own code?  What procedures are being developed to ensure that this will
> happen, rather than the expedient route of continuing to rely on a patched
> Trac copy?
>

This already happened and you mention the example below , so , I will
not provide further details .

In general , like I mentioned before , we avoid patching Trac core so
much as possible . If we finally do so then it's a last recourse
decision . That means that the patch is really important for us to get
something done . As a consequence if trac-dev does not accept the
patch we should keep it in there anyways . The only thing I suggest is
to create a ticket in a particular milestone (e.g. upstream_reject) to
watch for this , because there's a chance that e.g. three years later
Trac evolves and fortunately there will be a way to do the same thing
using new or modified APIs .

I hope you can see that this decision does not mean we are evil , at least IMO .

>  * It looks like Bloodhound currently depends on three upstream Trac
> plugins -- where are Bloodhound's issues filed against those plugins being
> tracked?
>

afaicr there's no such issue yet . The only one I recall is #74 and it
was fixed by updating Trac's built-in copy in vendor branch . If this
happens then

1. user reports an issue in Bloodhound issue tracker
2. we analyze it and determine if it's an error in third-party plugin
3. we forward the issue to plugin's issue tracker and close
    Bloodhound's as wontfix , providing all the details so that
    it will be possible for users to follow the discussion in
    forwarded ticket page .
4. we are not responsible anymore

... at least that's the way (I think) it should work as long as we do
not include code for third-party plugins in ASF repository and take
responsibility for maintaining it .

> I'm aware of only one instance where an upstream patch was discussed with
> the Trac community so far[5].  In that instance, a Trac developer said the
> relevant plugin code was in error and suggested an alternate approach[6]
> that would patch the faulty plugin code instead of working around it with a
> core patch.  This was filed but has not yet been acted on[7], and
> Bloodhound's developers did not all seem to agree that the advice from
> upstream should be followed[8, 9]; instead of patching the plugin code or
> fixing it upstream, the Bloodhound release continues to use the rejected
> core Trac patch.
>

TBH plugin code was not in error , I mean there was no semantic nor
even a programming error . In few words the answer was «we changed
Trac code for a good reason and what everybody used to do before will
not work now . you have to over-complicate your code and stick to our
new policies (<= do not enumerate extension points in component's
initializer if it implements target interface)» . I honestly don't
agree with trac-dev decision so , in the end we decided that such a
situation (i.e. enumerating ExtensionPoint objects in initializers of
classes implementing target interface) will happen quite often in our
code and preferred not to over-complicate plugin code , use locks ,
etc, etc (oh my !) ... Nonetheless there's a ticket opened somewhere
in ThemeEnginePlugin issue tracker. If we ever manage to find out a
generic , clean and simple solution not relying on a core patch , we
will try to revert our changes and use it once we test everything is
ok with it . The way I see it that has not happened yet .

> The plugin in question is well known and hasn't been maintained by its
> original author for more than two years (hence its incompatibility with
> Trac trunk) so this is a good example of why these questions matter.

Exactly . So what shall we do ?

> If
> Bloodhound's development workflows prioritized following advice from Trac
> core and submitted a good patch against ThemeEngine (maintaining a vendor
> branch // patch queue // short-lived fork of the plugin code in the
> meantime) then one of Bloodhound's developers could probably adopt
> maintenance for the plugin through existing Trac community channels pretty
> easily, which would benefit the wider Trac community.
>

That'd be cool if we were 100 people but that's not the case . Myself
, I have enough with my own plugins , and Bloodhound , and ... The
solution indeed must come from the community . We cannot adopt all
hacks and expect to fix them . That's not realistic . Right now
unfortunately we don't have the resources to maintain
ThemeEnginePlugin . Sorry . If somebody just volunteers and we finally
don't need our patch anymore then maybe there's a chance to remove it
after all ... that's not the case now afaics .

OTOH we *REALLY* needed ThemeEnginePlugin in order to customize web UI
so , I repeat the question ... what shall we do ? wait 2 more years in
order to release 0.1.0rc1 ?

[...]
>
> [1]
> http://www.mail-archive.com/bloodhound-dev@incubator.apache.org/msg00003.html
> [2]
> http://mail-archives.apache.org/mod_mbox/incubator-bloodhound-dev/201201.mbox/%3C4F07380E.7060505@wandisco.com%3E
> [3] http://osdir.com/ml/trac-dev/2012-01/msg00023.html
> [4]
> http://mail-archives.apache.org/mod_mbox/incubator-general/201201.mbox/%3C96BC3F76-A04A-4382-9CC1-547CA6757F9C@dslextreme.com%3E
> [5]
> http://old.nabble.com/Create-new-ticket-vs-reopen--9418-(if-necessary--)-td33164546.html
> [6]
> http://old.nabble.com/Re%3A-Create-new-ticket-vs-reopen--9418-%28if-necessary--%29-p33176708.html
> [7] https://issues.apache.org/bloodhound/ticket/27
> [8]
> http://mail-archives.apache.org/mod_mbox/incubator-bloodhound-dev/201204.mbox/%3CCAGMZAuPywg_QLUfiL4MEAR-57at22V8Oe=Yhr1+Hy3m8sxOexQ@mail.gmail.com%3E
> [9] http://trac-hacks.org/ticket/9580#comment:3
>
[...]

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Mime
View raw message