Now the dilemma comes because what he's got there now seems to be a
major improvement over what we're currently running. I've gotten
requests from multiple directions recently to replace our LXR with his
version. But Timeless is very scared of making a crash landing of it
because it hasn't gone through much in the way of review process (the
patches haven't been separated apart for indivual inspection, and it'd
be very hard to do so now with there being so many changes). Heck, I'm
a bit leery of that myself. But on the other hand, it does seem like a
huge improvement, and maybe we should just go for it.
His current version is running at
http://mxr-test.landfill.bugzilla.org/ . Several people I know use
that one in deference to the production one on http://lxr.mozilla.org/
because they like it better. It takes up an insane amount of disk
space on landfill, and we've been having trouble trying to keep the
disk from filling up lately.
What kinds of opinions have we got on this? Should we just land what
he's got? Should I deploy it as a separate install on the production
servers and not overwrite the existing stuff in CVS? Or should we just
ignore it until he can get his patches in order? (It's likely going to
get kicked off landfill anyway because we need the disk space back)
--
Dave Miller http://www.justdave.net/
System Administrator, Mozilla Corporation http://www.mozilla.com/
Yea, it's a bit unfortunate. However, because it's a tool and not an
app like Firefox or thunderbird, I say let someone give the diff a
onceover and check it in all at once. We certainly know by now that
it's at least as stable and correct as it ever was, The only reason it
uses so much disk space is that it indexes a few things that lxr does
not (Gnome, Eclipse, WebKit, OpenOffice, etc, etc.)
On the other hand, I think it has some changes that I authored (or at
least instigated), so I ought to find some time to take those through
the review process.
db48x
So, regarding the first 2 points, maybe I'm ignorant or missing
something - but isn't the point of CVS that we can, in fact, go back if
we want to?
Personally, I feel that, given mxr/mxr-test *has* had a fair amount of
dogfooding done, we should just land the changes, and if we find a
million regressions within a day, we can always revert it. I don't see
much point in trying to review a patch that's probably a few MB by now
(or at least several hundreds of kilobytes), nor in making him separate
it all out in different patches. I would offer personal help in cleaning
up regressions if they appear, but my Perl fu is not strong enough to
feel confident in doing so.
Additionally, I suppose, running mxr(-test) has the advantage of having
active people around who know the code. AFAIK, not many people (apart
from timeless himself) have been near lxr for quite some time. But I
might be wrong in that supposition, I don't watch the goings-on in
webtools all that carefully.
Just my few pennies, I suppose.
~ Gijs (Hannibal on IRC)
Well, you could reduce the amount of diskspace by removing all those
non-mozilla roots. I really don't know what (for example) the Eclipse
tree is doing there.
I hope the next is on-topic here, but for that lack of a better place:
But I don't use the mxr-test, because I think it suffers from having too
many features. It is nice that it indexes more items in a source file,
but having most of the file look blue-with-underline doesn't make it any
more readable... (of course, i can fix that with an user stylesheet)
Also, when you search for something, it offers direct links to blame and
change log. That's a lot of visual clutter, making it much harder to
find what I'm actually looking for. (And this can not be solved by an
user stylesheet)
Michiel
IMHO, at the very least a security review should be done before this lands.
/ Jonas
one of the reasons that people are pushing to land it is because of the
known issues w/ lxr, they're fixed in mxr-test.
i've been working on webtools (bugzilla, bonsai, tinderbox, lxr,
despot) for a number of years. and a lot of the time i spend w/ those
tools is giving them bad or nearly malicious input.
when i add features, i add filtering around the inputs that they take.
in some cases this means that some input gets mangled a bit too
aggresively (there's one instance atm), but it does mean that to my
knowledge input is safe for handling.
I'm not opposed to a review. In fact, I want reviews. I don't like
skipping the review process. The problem I have is finding reviewers
and getting time to give them bite sized patches (since most features
involve changes to multiple parts of multiple files - which means I
have to separate out 3-10 different features that have patched the same
block of code).
The main request mvl had is basically something that's on my todo list,
the ident output isn't using templates, and it needs to (especially
because not all installations or are configured to use bonsai).
Unfortunately that will involve moving around 2 blocks of code and
adding 5 new blocks. And it means that more code conflicts will exist
since i'm essentially refactoring code that has already been enhanced
(this is the slippery slope that mxr-test has taken).
FWIW, given how little attention our fork of LXR has received over the
years, I have a hunch that timeless's code is in better condition, and
we'd be better off adopting mxr, even without review, than staying on
our original codebase.
-myk
> What kinds of opinions have we got on this? Should we just land what
> he's got? Should I deploy it as a separate install on the production
> servers and not overwrite the existing stuff in CVS? Or should we just
> ignore it until he can get his patches in order? (It's likely going to
> get kicked off landfill anyway because we need the disk space back)
I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=366155 for this
now.
I'd go for that - are we really sure our current code is better reviewed
than timeless'? I'm not even sure of that. And his for sure is better
maintained, more security-aware and has more functionality.
Actually, I don't know a single really good argument not to take this
code instead of the current one into production.
Robert Kaiser
Draft comments for my cvs commit:
http://viper.haque.net/~timeless/mxr-test.notes (32k)
Items with X's as the leading bullet mark are things that I don't like
and which under normal circumstances would be review- like comments.
The patch (you don't really want to read this):
http://viper.haque.net/~timeless/mxr-test (200k)
-w version:
http://viper.haque.net/~timeless/mxr-test.w (150k)
The final versions of the above three files will be attached to the bug
before I actually do my commit.
wrt credits. If someone feels they were left out, please let me know.
I also expect to add some reference to lists of known bugs resolved
(probably Sunday), that list will probably be derived from
http://mxr-test.landfill.bugzilla.org/patches/ which contains various
bug numbers with patches and descriptions. If I have time, I'll try to
read through all the open bugs and see if I can explain which onces are
resolved and include them in the notes as well. (what's the difference
between a 32k and a 64k checkin comment among friends? :)
Sandbox containing my tree
http://viper.haque.net/~timeless/mxr-test.zip
I've staged the changes on http://mxr.landfill.bugzilla.org/
The original mxr <http://mxr.landfill.bugzilla.org/> was backed up as
<http://mxr-test.landfill.bugzilla.org/patches/mxr-20070117.zip>
There were three patches that I used to convert mxr on landfill:
1. complete replacement of files (excluding lxr.conf, index.html,
root/index.html) with the files that will be committed
http://mxr-test.landfill.bugzilla.org/patches/mxr-to-stage
2. merging the index.html and root/index.html from mxr-test for use on
mxr
http://mxr-test.landfill.bugzilla.org/patches/mxr-to-stage2
3. merging lxr.conf from mxr-test for use on mxr
http://mxr-test.landfill.bugzilla.org/patches/mxr-to-stage3
2/3 would always be something one would manually adapt for one's
purposes, the versions of the files for commit only make sense for
lxr.mozilla.org, and using them anywhere else would result in bad
output :)
Anyway, http://mxr.landfill.bugzilla.org is now running the changes for
commits (minus lxr.conf, index.html and root/index.html).
The differences between mxr and mxr-test will be merged into mxr-test
probably tomorrow, and I'll try to sync the few differences between
mxr(-test) and swift today or tomorrow.
I'm not sure what the scheduled window for the commit and upgrade is. I
was hoping justdave would have staged the commit yesterday, but he was
busy.
phew, long list - I guess the result is actually a different tool :)
> Items with X's as the leading bullet mark are things that I don't like
> and which under normal circumstances would be review- like comments.
Are there bugs filed for those? Probably would be a good idea to have
some record of those rough edges in Bugzilla.
> 2/3 would always be something one would manually adapt for one's
> purposes, the versions of the files for commit only make sense for
> lxr.mozilla.org, and using them anywhere else would result in bad
> output :)
Hmm, could you put a note in the INSTALL file or maybe a README about
how to upgrade from an "old lxr" (i.e. roughly current trunk) to a "new
lxr/mxr" (after your patch)? I guess a simple "cvs up" might not be
enough there...
I have mozilla's bonsai/lxr toolset running for my own projects, I guess
there may be others having the same, and we all would be glad to know
what we need to manually change after an update.
BTW, what would it need or how difficult would it be to add some xref
support for PHP post-your-patch?
Robert Kaiser
there's a reason it's getting rebranded :).
> Are there bugs filed for those? Probably would be a good idea to have
> some record of those rough edges in Bugzilla.
Not yet, I'll deal with that after I import the list of resolved bugs
from mxr-test/patches/ and do some other house cleaning.
I may consider writing bugs listing each feature I've changed so that
there's actually a list of fixed bugs, but I'm not sure how useful that
would be. I've already filed many bugs in bmo, I don't need to
artificially inflate my bug count.
> Hmm, could you put a note in the INSTALL file or maybe a README about
> how to upgrade from an "old lxr" (i.e. roughly current trunk) to a "new
> lxr/mxr" (after your patch)? I guess a simple "cvs up" might not be
> enough there...
A CVS update should actually work, since you'll retain the non symlink
style configuration.
Having walked justdave through a symlink install, the INSTALL file now
has a more detailed explanation of the setup and a note to remind
people that the docroot is different (old: lxr/root/, new: lxr/) if you
use symlinks.
> I have mozilla's bonsai/lxr toolset running for my own projects, I guess
> there may be others having the same, and we all would be glad to know
> what we need to manually change after an update.
I'll add a more detailed migrating section at some point, but basically
the key is:
A. If you want to convert from Apache Alias entries to symlinks:
1. remove your aliases from the Apache section.
2. change the docroot to not include /root
3. setup symlinks to replace the aliases
B. If you are changing the paths for your srcroots in lxr.conf:
* You still need to run update-search/update-xref (just like you would
have with your current LXR).
C. If your trees have different logical source prefixes (say
mozilla-org/ or mozilla/webtools/bugzilla instead of mozilla/)
* You should add sourceprefix: statements for any roots where it makes
sense (but you can do this at a later time).
D. If you're using update-*.sh
* You should change from using update-*.sh to update-*.pl as I will be
killing the .sh files eventually.
Note of course that these files are heavily site specific. I haven't
written update-lxr.pl, but it's a very very trivial script which I'll
probably do Sunday.
> BTW, what would it need or how difficult would it be to add some xref
> support for PHP post-your-patch?
Mostly it's writing a @phpterms list and adding the logic to choose to
use it. At least, that gets you syntax highlighting which is all Perl
had until recently. If you want xrefs to work then you need to write
two functions in genxref: findidentPHP and findusagePHP and the couple
of lines of glue to call them. This requires understanding how to
recognize identifiers in PHP. For me, since I've never written PHP
before in my life is not something I could do instantly, it'd take me
at least an hour... - but I'd want a weekend to start and a month to
test.
(note: i say "you", but that could be me as long as someone explains
PHP to me.)
Oh, and PHP support will not be landing with MXR. I'd like to land MXR
this weekend or by the following weekend. We could shoot for it in
February or March.
My to do list also includes getting diff to work because internally,
"swift" has dozens of trees which only have a week's worth of changes
(since we have weekly releases). The integration point for diff might
be a simple check box next to the select that lets you switch to
another tree for the same file.
FWIW I am updating notes occasionally, I did a spell checking pass
today and made some notes about the Alias => symlink transition last
night. No I'm not version controlling notes, so unfortunately you'll
need to read it again (or you can read it when it's the cvs commit
comment...).
Sure. I think it's more important to have known issues that still exist
filed in Bugzilla than those nice little things that have been fixed (as
long as there's a good checkin comment - which you're doing anyways).
> A CVS update should actually work, since you'll retain the non symlink
> style configuration.
Nice.
And as INSTALL is getting updated as well, I think we're all happy :)
Thanks for your descriptions here, I'll probably need to look them up
again when I'll be updating my install.
> Oh, and PHP support will not be landing with MXR. I'd like to land MXR
> this weekend or by the following weekend. We could shoot for it in
> February or March.
Sure. Actually, if it looks managable to me, I might look into it myself
once MXR is landed and I have updated to it. I think the pointers you
gave here will probably be a good starting point - and if we are able to
handle perl, I think PHP should not be too hard to do :)
Robert Kaiser
There is a sample install of OpenGrok here (thanks Reed!):
http://hera.senecac.on.ca:43080/source/xref/mozilla/
Which has most of the features in lxr/mxr (including parsers for all
sorts of source/bin files) and wicked-fast searches.
Has the secondary advantage of us not being the only folks to
use/support it.
I don't want to hold up obvious improvements to the existing LXR
instance but everyone should take a quick look and see what they think.
There was a quick previous discussion of it here:
Cheers,
Schrep
The parser is not nearly as smart.
Try clicking on window.sizeToContent (line 53) at
http://hera.senecac.on.ca:43080/source/xref/mozilla/browser/base/content/aboutDialog.js
and
http://mxr.mozilla.org/seamonkey/source/browser/base/content/aboutDialog.js
It doesn't really seem to parse things much at all. The searches just
seem to be text searches unless it thinks it's found the real definition
of the method and then it just goes directly to that. Compare
http://hera.senecac.on.ca:43080/source/s?defs=SchemeIs
and
http://mxr.mozilla.org/seamonkey/ident?i=SchemeIs
I guess I'm a bit disappointed to see that mxr didn't pick up schemeIs
hits from .idl and .js. But OpenGrok didn't get that either. mxr finds
the .idl and .js hits if you search for "schemeIs" while OpenGrok thinks
it's found the real definition in some test and winds up at
http://hera.senecac.on.ca:43080/source/xref/mozilla/extensions/java/xpcom/tests/TestQI.java#schemeIs
mxr also has much more advanced search capabilities that come in handy
for a big project. See http://mxr.mozilla.org/seamonkey/search
--
Andrew Schultz
aj...@buffalo.edu
http://www.sens.buffalo.edu/~ajs42/
I think it's much better at some file types than mxr/lxr. For example,
when you click on a type in a C++ file, you go directly to the correct
definition, not a list of guesses.
We probably need JS and CSS analyzers to make OpenGrok really work for
us. For a list of the analyzers they already have, see
<http://src.opensolaris.org/source/xref/opengrok/trunk/src/org/opensolaris/opengrok/analysis/>
-Rob
Sometimes. See the {s,S}chemeIs links I had. And it guesses which one
is correct and doesn't always get it right. And sometimes you might
want all the callers.
But it /is/ really fast.
--
Andrew Schultz
ajsc...@verizon.net
http://www.sens.buffalo.edu/~ajs42/
mxr.landfill.bugzilla.org (and of course lxr.mozilla.org) know it though:
http://mxr.landfill.bugzilla.org/seamonkey/find?string=datetimepicker.xml
http://lxr.mozilla.org/seamonkey/find?string=datetimepicker.xml
Steffen