Hi Ethan ! Thanks for your message . Some comments below.
On 8/2/12, Ethan Jucovy <[email protected]> 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/[email protected]/msg00003.html > [2] > http://mail-archives.apache.org/mod_mbox/incubator-bloodhound-dev/201201.mbox/%[email protected]%3E > [3] http://osdir.com/ml/trac-dev/2012-01/msg00023.html > [4] > http://mail-archives.apache.org/mod_mbox/incubator-general/201201.mbox/%[email protected]%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+hy3m8sxo...@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:
