Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zeek renaming: update executable names #239

Closed
jsiwek opened this issue Jan 11, 2019 · 12 comments
Closed

Zeek renaming: update executable names #239

jsiwek opened this issue Jan 11, 2019 · 12 comments
Assignees
Labels
good first issue A good place to get started working with Zeek. Type: Maintenance
Milestone

Comments

@jsiwek
Copy link
Contributor

jsiwek commented Jan 11, 2019

Things like the bro binary and other tools/scripts like bro-config or bro-cut should be zeekified (I just listed those from memory, should search for others, too).

Seems they can either be symlinks or else a separate script that first emits a deprecation warning before calling out to the actual executable. Depends on what plans are to support the old names... maybe something to consider is that there's may be external scripts/tools that are actually depending on a process name being 'bro'.

@jsiwek jsiwek added good first issue A good place to get started working with Zeek. Priority: Low Type: Maintenance labels Jan 11, 2019
@jsiwek jsiwek added this to the 2.7 milestone Jan 11, 2019
@rsmmr rsmmr self-assigned this Mar 28, 2019
@rsmmr
Copy link
Member

rsmmr commented Apr 2, 2019

I'm planing to do the following:

  1. Rename executables as proposed below.
  2. For each, install a link under the old name pointing to a wrapper script that forwards to the new executable.
  3. The wrapper script will also print a deprecation warning if (1) it's running from a tty, and (2) the environment variable ZEEK_IS_BRO remains unset.

I'd ignore any concerns about the process name, that seems hard to anticipate and address.

Does that all sound right? Anything missing from the list below?

bro -> zeek
bro-config -> zeek-config
broctl -> zeekctl
bro-cut -> zeek-cut

No change:
    bifcl
    binpac
    btest
    capstats
    trace-summary
    Anything Broker
    Anything Broccoli (which we should remove for 3.0)

@jsiwek
Copy link
Contributor Author

jsiwek commented Apr 2, 2019

Yes, the wrapper script approach was what I was thinking also, but a couple thoughts:

  • I'm expecting you maybe have to at least care about fixing BroControl to be aware of the new 'zeek' process name, thought I remember some commands specifically looking for that.

  • Renaming broctl -> zeekctl may be awkward if we wanted to later use zeekctl to mean something in the new Supervisor ecosystem, but I guess it's maybe fine and we can instead go with zctl or zktl if later on we need/want something where that kind of name still makes sense.

@sethhall
Copy link
Member

sethhall commented Apr 3, 2019 via email

@rsmmr
Copy link
Member

rsmmr commented Apr 3, 2019

@jsiwek Yeah, BroControl will need some additional care. For its new name, I'd prefer to stick to a predictable renaming scheme for all executables, i.e., sed 's/^bro/zeek/'. And it will probably make sense to find a different name for any new tool anyways.

@sethhall we'll need to do this for the other tools as well, and a single wrapper script seems easier. Any problem with "bro" being a wrapper?

@sethhall
Copy link
Member

sethhall commented Apr 3, 2019

Not really. At least with the wrapper script, it won't be new code and can be easily removed once the "bro" name is completely deprecated.

rsmmr added a commit that referenced this issue Apr 24, 2019
…ev to zeek-path-dev.

This also installs symlinks from "zeek" and "bro-config" to a wrapper
script that prints a deprecation warning.

The btests pass, but this is still WIP. broctl renaming is still
missing.

#239
rsmmr added a commit that referenced this issue May 2, 2019
…ev to zeek-path-dev.

This also installs symlinks from "zeek" and "bro-config" to a wrapper
script that prints a deprecation warning.

The btests pass, but this is still WIP. broctl renaming is still
missing.

#239
@rsmmr
Copy link
Member

rsmmr commented May 6, 2019

First stab of this is now in topic/robin/gh-239 in repos zeek, bro-aux, broctl, and zeek-docs. It's probably not perfect yet, but ready for some testing.

For BroControl I went with a pretty comprehensive search-and-replace as it turned out difficult to tease it apart otherwise. I put wrappers for legacy names in place, including for the plugin API. The tests all pass (one exception: command.update, but that's failing on master already for me). I could use some more help testing zeekctl from people more familiar with it. @justin or @dnthayer, could you give it a try and see if you spot anything not working?

@jsiwek
Copy link
Contributor Author

jsiwek commented May 7, 2019

@rsmmr nice (and thanks, this looks like maybe the most tedious rename yet). I did a quick interactive test of zeekctl (both standalone and cluster) and didn't notice any issues.

Also did a quick skimming review of the changes:

  • zeek

    • wrapper script was missing arg forwarding ?
  • zeek-aux

    • does the configure skeleton need to fallback on looking for a bro-config if there's no zeek-config, since some may want to still write plugins that are 2.6 compatible ?
    • similarly, need a fallback on looking for bro-path-dev.in ?
  • zeekctl

    • some "broke" strings got changed to "zeekke"
    • we'll remove the update command soon, so seems fine if that test fails
    • change broctl repo name to zeekctl (and submodule/dir name within zeek repo) ? We can do this whenever, like on or after merge, just confirming we want that.

@dnthayer
Copy link
Contributor

dnthayer commented May 8, 2019

I found some typos in the changes made in the broctl repo. Should I just push a commit in branch topic/robin/gh-239 in the broctl repo to fix those typos?

Also, not sure if the change "Big Brother" --> "Big Zeekther" was intentional or not (in the MailFrom option).

@rsmmr
Copy link
Member

rsmmr commented May 9, 2019 via email

@rsmmr
Copy link
Member

rsmmr commented May 12, 2019

@dnthayer Thanks for the fixes and improvements.

@jsiwek Fallback on bro-config and bro-path-dev.in for the plugin configure is a good idea. I'll add that. I didn't change the repo name yet to not break stuff, can I leave that to you on merge?

@rsmmr
Copy link
Member

rsmmr commented May 12, 2019

Done, I believe this is ready to merge now. Going file PR.

jsiwek added a commit that referenced this issue May 14, 2019
* origin/topic/robin/gh-239:
  Undo a change to btest.cfg from a recent commit
  Updating submodule.
  Fix zeek-wrapper
  Update for renaming BroControl to ZeekControl.
  Updating submodule.
  GH-239: Rename bro to zeek, bro-config to zeek-config, and bro-path-dev to zeek-path-dev.
@jsiwek
Copy link
Contributor Author

jsiwek commented May 14, 2019

This is completed via #363, thanks Robin.

@jsiwek jsiwek closed this as completed May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good place to get started working with Zeek. Type: Maintenance
Projects
None yet
Development

No branches or pull requests

4 participants