Skip to content

Dependency-related fixes - #3

Open
jsiwek wants to merge 3 commits into
ncsa:masterfrom
jsiwek:master
Open

Dependency-related fixes#3
jsiwek wants to merge 3 commits into
ncsa:masterfrom
jsiwek:master

Conversation

@jsiwek

@jsiwek jsiwek commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

i.e. related to the following issues:
https://github.com/bro/package-manager/issues/14
https://github.com/bro/package-manager/issues/15
#1

I think the changes in this PR are the way dependencies should be handled by this package. At least for the current state of Bro's master vs. 2.5.1.

@JustinAzoff

Copy link
Copy Markdown
Contributor

Hmm.. I'm not sure if that's right.. the issue is that the script needs

commit 7c107f9f02abebffbb625a4c76ea62eed8363dca
Merge: ff2c5b934 ff998dfa4
Author: Seth Hall <seth@icir.org>
Date:   Fri May 12 15:31:32 2017 -0400

    Merge remote-tracking branch 'origin/topic/johanna/notice-suppression'

    * origin/topic/johanna/notice-suppression:
      Lessen cluster node of notice suppression.

Which was added in 2.5-140

if I depend on bro >= 2.5-140 I think it will complain that bro 2.5.1 is not new enough, but if I depend on 2.5.1, it won't allow master.

@jsiwek

jsiwek commented Sep 20, 2017

Copy link
Copy Markdown
Contributor Author

Did you see I had !=2.5.1 in the version requirement? i.e. If you choose to merge the commit that removes the 'packages' prefix from the @load, then we can never use 2.5.1 anyway, it doesn't have the necessary fix in broctl.

if I depend on bro >= 2.5-140 I think it will complain that bro 2.5.1 is not new enough

But hypothetically, I think that's incorrect. 2.5.1 should satisfy that spec:

$ python
Python 3.6.2 (default, Jul 17 2017, 16:44:45) 
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from semantic_version import *
>>> spec = Spec(">=2.5-140")
>>> version = Version.coerce("2.5.1")
>>> version in spec
True

but if I depend on 2.5.1, it won't allow master.

Yeah, I just posted more thoughts about the state of Bro's versioning in https://github.com/bro/package-manager/issues/15

I think probably in the future it should be rare to depend on patch-level releases, and certainly for 2.5.1, the state of things is messy enough to be not advisable.

@JustinAzoff

Copy link
Copy Markdown
Contributor

Did you see I had !=2.5.1 in the version requirement? i.e. If you choose to merge the commit that removes the 'packages' prefix from the @load, then we can never use 2.5.1 anyway, it doesn't have the necessary fix in broctl.

Ah, that makes sense.

I guess there's no great solution at the moment... either way breaks things for some people.

I think I'll leave this un-merged then for now, so that at least everything works as good as it can on a bro 2.5.1 install. Once a new release is out including the broctl fixes I'll update the requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants