380118 - Tinderbox doesn't know how to deal with multiple bonsai installations

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, enhancement)

Description

[Dave Miller [:justdave]]

tinderbox.mozilla.org has a number of trees reporting into it that build off the l10n repo.  These trees report their cvs paths as being in the /l10n repo.  We have a separate bonsai for that repo, and I get error mail from cron every time one of those trees reports in complaining about an invalid tree name in buildwho.pl.  Originally bonsai was complaining about /l10n not existing, so I symlinked that in the cvs bonsai to the appropriate data directory in the l10n bonsai.  That got the batch load working, but now buildwho complains that the tree doesn't exist.  Not sure what to do here.

"buildwho.pl returned an error for tree Mozilla1.8.0-l10n-ca" etc.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Sounds like this blocks bug 281910?

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Oh, ignore comment #1, I misread.

Comment 3

[cls]

Tinderbox has had support for multiple bonsai installations for years.  $bonsai_tree, $cvsroot, $cvs_module & $cvs_branch are tree specific variables that are set in treedata.pl .  Try setting $F_DEBUG=1 in buildwho.pl and see what it prints out.

Comment 4

[cls]

It's going to be one of those mornings.  Tinderbox currently supports multiple bonsai trees via treedata.pl but BONSAI_DIR is set in the Makefile so tinderbox only supports a single bonsai installation.

Comment 6

[Reed Loden [:reed]]

So, this caused bug 380118. I added $bonsai_dir and $bonsai_url for two of the l10n trees to point to bonsai-l10n, but the variables were leaked into other trees causing links to bonsai to use the l10n-specific values. :/

So, will I need to add $bonsai_dir and $bonsai_url to each treedata.pl or what? I didn't see any changes when I added it to the l10n trees, so it didn't seem like it helped any.

cls: Any ideas?

Comment 7

[cls]

I assume you mean bug 386011.  

Essentially, the bonsai variables from the Makefile will need to be moved into each tree's treedata.pl.  Without digging into it, that means:
* updating %::default_treedata so that trees are created correctly
* replacing references to $::bonsai_dir & $::bonsai_url with $::global_treedata->{$tree}->{bonsai_dir}
* some how fix checksetup.pl so that it can automatically add the missing bonsai_* variables to the existing treedata.pls

Comment 8

[cls]

Attachment: v1.0

Here's a first cut.  It needs to be tested by someone with a working bonsai installation.

This patch moves the bonsai_dir, bonsai_url & registry_url variables from being set as globals in Makefile/tbglobals.pl to being members of the global treedata hash.

There are 2 incidental behavior changes:
1) In treedata.pl, use_bonsai and use_viewvc will just be set to 1 instead of the number of bonsai/viewvc variables that are set.
2) We no longer fall back to $bonsai_dir/data/banner.html to get the banner that shows up at the top of the pages.  Installations that want to use that file will need to symlink it to $(TINDERBOX_DIR)/data/banner.html .

A hint for upgrading the treedata.pls: 
You can avoid needing to manually update every treedata.pl by setting bonsai_dir, bonsai_url & registry_url to your desired values in tbglobals.pl's default_treedata hash before running ./checksetup.pl .  Just remember to restore them to the default empty value once you're done upgrading.

Comment 9

[Mike Taylor [:bear]]

wow - this is going to be a fun one to review :)

I like the changes you are suggesting.

I'll update the landfill bonsai and also my two home setups to give it a workout either tonight or tomorrow.

Comment 10

[Reed Loden [:reed]]

bear, any update on this patch?

Comment 11

[Reed Loden [:reed]]

bear: ping again? Can you review this patch?

Comment 12

[Mike Taylor [:bear]]

Comment on attachment 274817 [details] [diff] [review]
v1.0

I've made one pass thru the code a couple weeks ago, didn't see anything - another tonight.  The only concern I have is anything that may have been missed but it needs to be put into a test area with more activity than I generate at home.

Comment 13

[Mike Taylor [:bear]]

Checking in Makefile;
/cvsroot/mozilla/webtools/tinderbox/Makefile,v  <--  Makefile
new revision: 1.19; previous revision: 1.18
done
Checking in admintree.cgi;
/cvsroot/mozilla/webtools/tinderbox/admintree.cgi,v  <--  admintree.cgi
new revision: 1.35; previous revision: 1.34
done
Checking in buildwho.pl;
/cvsroot/mozilla/webtools/tinderbox/buildwho.pl,v  <--  buildwho.pl
new revision: 1.23; previous revision: 1.22
done
Checking in doadmin.cgi;
/cvsroot/mozilla/webtools/tinderbox/doadmin.cgi,v  <--  doadmin.cgi
new revision: 1.30; previous revision: 1.29
done
Checking in header.pl;
/cvsroot/mozilla/webtools/tinderbox/header.pl,v  <--  header.pl
new revision: 1.5; previous revision: 1.4
done
Checking in showbuilds.pl;
/cvsroot/mozilla/webtools/tinderbox/showbuilds.pl,v  <--  showbuilds.pl
new revision: 1.26; previous revision: 1.25
done
Checking in showlog.cgi;
/cvsroot/mozilla/webtools/tinderbox/showlog.cgi,v  <--  showlog.cgi
new revision: 1.27; previous revision: 1.26
done
Checking in tbglobals.pl;
/cvsroot/mozilla/webtools/tinderbox/tbglobals.pl,v  <--  tbglobals.pl
new revision: 1.59; previous revision: 1.58
done
Checking in warnings.pl;
/cvsroot/mozilla/webtools/tinderbox/warnings.pl,v  <--  warnings.pl
new revision: 1.104; previous revision: 1.103
done

Comment 14

[Andrew Schultz]

Comment on attachment 274817 [details] [diff] [review]
v1.0

>+BEGIN { $bonsai_dir = $::global_treedata->{$tree}->{bonsai_dir} || '@TINDERBOX_DIR@'; }

Perl executes this before the rest of the file, so $tree and $::global_treedata are both undefined at this point.

But without the BEGIN, |use lib "$bonsai_dir";| does not work.  If I hard-code |$bonsai_dir="/var/www/bonsai";| in the BEGIN block, then the "use lib" bit does work.

I'm running perl 5.8.5

Comment 15

[Mike Taylor [:bear]]

re-opening to fix regression

Comment 16

[Mike Taylor [:bear]]

cls - the issue is that the require 'tbglobals.pl' needs to be relative to the bonsai dir but the true bonsai dir cannot be determined during compile time eval

we need to find a way to pull what is needed from tbglobals without using a require.

Comment 17

[Mike Taylor [:bear]]

Attachment: attempt to fix buildwho.pl regression

Comment 18

[cls]

Comment on attachment 278439 [details] [diff] [review]
attempt to fix buildwho.pl regression

We need 'use lib $bonsai_dir' to find cvsquery.pl from the bonsai tree.  I'm sorely tempted to just copy that file into the tinderbox tree as bonsai.pl and be done with it.  Otherwise, we'll need to expand the BEGIN block to set $tree from @ARGV and call tb_load_treedata().  FYI, warnings.pl has the same issue but uses cvsblame.pl.

Comment 19

[Mike Taylor [:bear]]

copy the file and lets file a bug to refactor to solve this cleanly.

Comment 20

[cls]

Attachment: v2.0

Unfortunately, simply copying cvsquery.pl isn't really an option due to the tangled code in bonsai.  However, viewvc.pl is a stripped down copy of cvsquery.pl so it was fairly easy to add back the bits needed to populate the guilty column when using bonsai.

A bit harder to swallow may be that this fix requires putting the bonsai db info into each treedata.pl.  Not a problem for small installations but could be fairly inconvenient for m.o. 

FYI, cvsblame.pl has no external dependencies and could probably be copied into tinderbox wholesale but we'd still need $bonsai_dir around to read the batch files for the open/closed state so there isn't much point in copying over cvsblame.pl at this time.

Comment 21

[cls]

Checking in webtools/tinderbox/admintree.cgi;
/cvsroot/mozilla/webtools/tinderbox/admintree.cgi,v  <--  admintree.cgi
new revision: 1.36; previous revision: 1.35
done
Checking in webtools/tinderbox/buildwho.pl;
/cvsroot/mozilla/webtools/tinderbox/buildwho.pl,v  <--  buildwho.pl
new revision: 1.24; previous revision: 1.23
done
Checking in webtools/tinderbox/doadmin.cgi;
/cvsroot/mozilla/webtools/tinderbox/doadmin.cgi,v  <--  doadmin.cgi
new revision: 1.31; previous revision: 1.30
done
Checking in webtools/tinderbox/tbglobals.pl;
/cvsroot/mozilla/webtools/tinderbox/tbglobals.pl,v  <--  tbglobals.pl
new revision: 1.60; previous revision: 1.59
done
Checking in webtools/tinderbox/viewvc.pl;
/cvsroot/mozilla/webtools/tinderbox/viewvc.pl,v  <--  viewvc.pl
new revision: 1.6; previous revision: 1.5
done
Checking in webtools/tinderbox/warnings.pl;
/cvsroot/mozilla/webtools/tinderbox/warnings.pl,v  <--  warnings.pl
new revision: 1.105; previous revision: 1.104
done

Comment 22

[Dave Miller [:justdave]]

now that you've already gone and done it...

instead of all that mess, how about just chdir into $bonsai_dir before doing the require on cvsquery.pl ?

. is already in @INC because you're not running in taint mode, so that should Just Work.

Comment 23

[cls]

I think we are running in taint mode since buildwho.pl is called by processbuild.pl which is setuid.  There are trick_taint() calls in buildwho.pl so it was running in taint mode at some point.

I don't like the way we stumbled onto the solution but I think the current setup is better for long term maintenance of the tinderbox code base than hacking around the dynamic 'use lib' issues with chdir.  We already need to maintain the query checkin code for viewvc support so why not use it for bonsai as well?  

If bonsai had an exposed API that allowed us to do the minimum needed to query the db for the who data, then it'd be a different story.  As it is, I believe that we're now one step from being completely decoupled from the mess that is bonsai and that's figuring out how to determine the tree state without needing direct access to $bonsai_dir .

Comment 24

[Mike Taylor [:bear]]

during production upgrade justdave and reed spotted two issues that I fixed with this commit - adding patch in a sec:

Checking in showbuilds.pl;
/cvsroot/mozilla/webtools/tinderbox/showbuilds.pl,v  <--  showbuilds.pl
new revision: 1.28; previous revision: 1.27
done

Comment 25

[Mike Taylor [:bear]]

(In reply to comment #24)
> during production upgrade justdave and reed spotted two issues that I fixed
> with this commit - adding patch in a sec:
> 
> Checking in showbuilds.pl;
> /cvsroot/mozilla/webtools/tinderbox/showbuilds.pl,v  <--  showbuilds.pl
> new revision: 1.28; previous revision: 1.27
> done

wrong bug - damn

Comment 26

[cls]

Attachment: v3.0

Copy cvsquery.pl & get_line.pl from bonsai to tinderbox and strip out all of the bits that require bonsai globals.pl.  Then merge in viewvc.pl.

Comment 27

[cls]

I believe that this has been live for awhile.