[Bro-Dev] #545: More script reorg fallout

[Bro Tracker]

#545: More script reorg fallout
----------------------+------------------------
  Reporter:  jsiwek   |      Owner:
      Type:  Problem  |     Status:  new
  Priority:  Normal   |  Milestone:  Bro1.6
 Component:  Bro      |    Version:  git/master
Resolution:           |   Keywords:
----------------------+------------------------

Comment (by jsiwek):

 Replying to [comment:0 seth]:
 > > default.  Scripts should always explicitly `@load` all dependencies as
 > > that will be more pleasant for a user trying to run with `--bare-mode`
 > > enabled.
 >
 > Yes, I left that out with an eye toward discussing it a bit more.  Do
 you think scripts within the base/ directory should do relative loading or
 they should be doing full path loading?

 Here's a possible rule to follow: if you need to @load across "package"
 boundaries use the full path, but if you need to @load something that's
 intra-package you can use a path relative to the current script.  And I'm
 defining "package" to be any directory of related scripts one level below
 `base/`, `policy/`, etc. directories.  I forget why exactly we implemented
 relative path loading in the first place (!), but doing it like this
 should mean you can move packages around as a whole without having to
 rename dependencies.  e.g. say you want to promote a package in `policy/`
 to now live in `base/`, that should just involve a move of the directory.

 Replying to [comment:1 robin]:
 > - core.load-prefixes now fails and I don't immediately see why.

 It's fixed, pending merge request in #543.

 > - I was trying to load listen-{clear,ssl) in testing-all.bro, but that
 >   failed complaining that Communication::default_port_* aren't
 >   defined.  But it seem they should be available because the comm
 >   framework is pulled in as well as part of the base. Any idea?

 Didn't verify, but I was looking yesterday at similar errors and there's a
 TODO in `base/frameworks/communication/__load__.bro` that was waiting on
 the enable_communication BIF stuff; can probably be fixed now.

 > - I think it should work to run "bro -b" and then load files from
 >   policy/* without having to manually pull in dependencies from
 >   base/*.

 Yeah, but I'd go further than that (one of my points above): for any given
 script anywhere in the `scripts/` tree you should be able to load it by
 itself with `bro -b` without having to figure out and manually specify
 dependencies.

 >     bro -b test-all-policy
 >     /home/robin/bro/master/scripts/policy/frameworks/dpd/detect-
 protocols.bro,
 >     line 6: unknown identifier (Notice::Type)
 >     bro: parse.y:717: int yyparse(): Assertion `cur_enum_type' failed.
 >
 >   I've added a test for it (which currently fails of course)

 I don't think relying on the `test-all-policy.bro` script for this
 coverage test is good enough because 1) we also need to check stuff in
 `base/` can be loaded individually 2) scripts that are loaded further down
 in the file might succeed when they otherwise wouldn't just because the
 right dependency happened to be loaded earlier on

 > - *.bif.bro install into the top-level share/bro directory. I think they
 should go into base now.

 I was wondering that, too.

-- 
Ticket URL: <http://tracker.bro-ids.org/bro/ticket/545#comment:2>
Bro Tracker <http://tracker.bro-ids.org/bro>
Bro Issue Tracker