search

brimdata / zeek

Zeek is a powerful network analysis framework that is much different from the typical IDS you may know.
https://www.zeek.org
Other
2 stars 0 forks source link

Zeek v3.2.0-dev-551-gfda8b98ac for macOS, Linux, and Windows #26

Closed nwt closed 4 years ago

nwt commented 4 years ago
  1. Update Zeek to v3.2.0-dev-551-gfda8b98ac.
  2. Make it build with MinGW tools. The resulting Windows executable is missing a lot of functionality but can process pcaps.
  3. Update zeekrunner.go for MingGW Zeek.
  4. Add a Windows artifact to Brim releases.
  5. Advance the macOS deployment target to 10.14 (from 10.10) for the sake of a C++ feature I can't recall.
  6. Run the Brim GitHub workflow (renamed to brim.yml from brim-release.yml) for PRs, master, and release tags, and upload the artifacts.

This should close #10, #14, and #24.

henridf commented 4 years ago

Update Zeek to v3.2.0-dev-551-gfda8b98ac.

Is it possible to add these commits on a separate PR? Also, what does v3.2.0-dev-551-gfda8b98ac refer to? I see the v3.2.0-dev branch in upstream, but no commit gfda8b98ac or reference to "551".

nwt commented 4 years ago

@henridf: v3.2.0-dev-551-gfda8b98ac is "git describe" output for fda8b98ac, which is 551 commits past tag v3.2.0-dev. (That was zeek/zeek@master at the time I started merging this branch into brimsec/zeek@master).

Should I simply push the merge commit (c62b41862103eab11d4ec9bd12c1255bb7f54d85) to master so this PR shrinks to the last six commits? Or would you like to see a PR with the merge commit plus minimal necessary changes to brim/ and .github/workflows/brim-release.yml (which I don't have on hand but could prepare)? Or something else?

henridf commented 4 years ago

Should I simply push the merge commit (c62b418) to master so this PR shrinks to the last six commits?

Hmm. I guess that (assuming this PR isn't squashed upon merging), then the resulting history in master would be the same if you push that merge commit vs doing everything here.

Or would you like to see a PR with the merge commit plus minimal necessary changes to brim/ and .github/workflows/brim-release.yml (which I don't have on hand but could prepare)?

This doesn't seem indispensable to me, and it's extra work, so I'd say not worth it.

Overall my concern was just about future readability and maintenance, but after some more thought I think this approach (with an unsquashed merge) is good.

(Also I was anticpating the "how do we maintain these changes on top of zeek over time", but given that the goal is for your changes to be upstreamed, this shouldn't be an issue).

nwt commented 4 years ago

Overall my concern was just about future readability and maintenance, but after some more thought I think this approach (with an unsquashed merge) is good.

👍 I was planning to merge this by adding a merge commit.

nwt commented 4 years ago

We don't need this any longer, so I removed it from .github/workflows/brim-release.yml in 04df34d73039313c781c6664df852c4fa746422b.

      # Remove this step when the hosted runners have
      # https://github.com/actions/virtual-environments/pull/632.
      - if: startsWith(matrix.os, 'windows-')
        name: Windows MSYS2 installation
        shell: pwsh
        run: |
          Set-PSDebug -Trace 1
          if (Test-Path C:/msys64) { exit 0 }
          $url = 'http://repo.msys2.org/distrib/x86_64/msys2-base-x86_64-20190524.tar.xz'
          bash -c "curl -LSs $url | tar -xJf - -C /c"
          $env:Path = "C:\msys64\usr\bin"
          # packman-key is a script, so PowerShell can't run it directly.
          bash -c 'pacman-key --init'
          bash -c 'pacman-key --populate msys2'
          # pacman-key starts gpg-agent, causing breakage if pacman
          # upgrades msys-2.0.dll.
          Stop-Process -Name gpg-agent
          # First update pacman to avoid
          # https://github.com/msys2/MSYS2-packages/issues/1960.
          pacman -Sy --noconfirm pacman
          pacman -Syuu --noconfirm
          pacman -Syuu --noconfirm