Skip to content
This repository has been archived by the owner on Apr 1, 2024. It is now read-only.

Windows build failure due to missing include #61

Closed
philrz opened this issue Apr 18, 2022 · 2 comments
Closed

Windows build failure due to missing include #61

philrz opened this issue Apr 18, 2022 · 2 comments

Comments

@philrz
Copy link

philrz commented Apr 18, 2022

As identified in #59, Brim's Zeek port now experiences build failures on Windows. Through some digging I've confirmed that this fix addresses the issue. Since the code for Brim's Zeek port and the hash-based pointers to underlying submodule dependencies have not changed, it's a bit of a mystery to me why it started failing. The fix linked above unfortunately seems to lack any PR or other context about when they stumbled across the problem. My best guess is that maybe the tooling on the GitHub Actions Windows 2019 hosted runners may have undergone changes that caused this failure during the long period since Brim's Zeek was last built.

In a world of infinite resources, perhaps the ideal solution would be to begin using the much-newer C++ Actor Framework (CAF) that includes the fix as well as the much-newer Zeek Broker and Zeek itself that depend on it. However, assuming Brim Dev the team lacks the time or patience to revisit the complex porting work involved, I've instead investigated the minimal steps necessary to only cherry-pick the CAF fix and then wire up the necessary submodule pointers to leverage it. Because of how Zeek uses multiple levels of submodules, this was a bit of a pain to wire up, but now that I've gone through it and seen it build successfully using personal forks of the necessary repos, here I can lay out the steps so someone on the Brim team with necessary GitHub permissions can carry out the steps with forks in the Brimdata GitHub org.

The steps:

  1. In the GitHub UI, do a "fetch upstream" of https://github.com/brimdata/broker. As you can see here, while this Broker fork has existed for some time, the submodule dependency in Brim's Zeek port hasn't been pointing at it. However, we'll need to start pointing at a new Broker commit that'll point to a CAF fork that contains the ultimate fix, and Brim's Broker fork is currently way behind the Broker submodule pointer to commit 5edfc77 shown here, so the fork needs to be brought current as a prerequisite.

  2. In the GitHub UI, delete the fork https://github.com/brimdata/actor-framework and replace it with a fork of https://github.com/zeek/actor-framework. We established in the last step that Brim's Zeek port has been pointing at a commit in the Zeek project's Broker repo this whole time, and as shown here, the Zeek Broker at that commit has been pointing to Zeek's CAF fork at commit f7d4fc7. Therefore, we'll want to start from having a current Zeek CAF fork onto which we can cherry-pick the fix.

  3. Clone the Brim CAF fork at the commit that was used to build successfully in the past (which we can do via a tag), cherry-pick the fix atop that, and commit to a branch.

export ORG="brimdata"
cd $TMPDIR
git clone -b 0.17.6 https://github.com/$ORG/actor-framework
cd actor-framework
git checkout -b inc-limits-fix
git cherry-pick a080891412b028bf89801cd8db959f3478352e89
git push origin inc-limits-fix
  1. Clone the Brim Broker fork at the commit that was used to build successfully in the past (which we can do via a tag), change the CAF submodule pointer to the CAF branch with the fix, and commit to a branch.
export ORG="brimdata"
cd $TMPDIR
git clone -b v1.4.0 https://github.com/$ORG/broker.git
cd broker
git checkout -b point-at-fixed-caf
git config --file=.gitmodules submodule.caf.url https://github.com/$ORG/actor-framework.git
git config --file=.gitmodules submodule.caf.branch inc-limits-fix
git submodule update --init --remote caf
git add .gitmodules caf
git commit -m "Point at caf fork/branch with limit fix"
git push origin point-at-fixed-caf
  1. Clone the Brim Zeek fork at master and change the Broker submodule pointer to the Broker branch that points to the CAF branch with the fix. The change from Clone repo over https #59 is also included.
export ORG="brimdata"
cd $TMPDIR
git clone https://github.com/$ORG/zeek.git
cd zeek
git checkout -b point-at-own-broker
perl -pi -e 's/git:/https:/' .github/workflows/brim.yml
git config --file=.gitmodules submodule.auxil/broker.url https://github.com/$ORG/broker.git
git config --file=.gitmodules submodule.auxil/broker.branch point-at-fixed-caf
git submodule update --init --remote auxil/broker
git add .gitmodules auxil/broker .github/workflows/brim.yml
git commit -m "Point at broker fork/branch that points to caf fork/branch with limits fix"
git push origin point-at-own-broker

At this point a PR can be made for the Brim Zeek point-at-own-broker branch and it should build successfully in Actions. You can see an example of this working in my personal forks (i.e., the above commands run with export ORG="philrz") in https://github.com/philrz/zeek/pull/1 and its accompanying successful Actions Run here.

@philrz
Copy link
Author

philrz commented May 30, 2022

Note that a community-requested enhancement described in brimdata/geoip-conn#39 is blocked by the lack of a working Zeek build system.

@philrz philrz self-assigned this Oct 5, 2022
@philrz philrz removed their assignment Nov 20, 2023
@philrz
Copy link
Author

philrz commented Apr 1, 2024

Thanks to the porting work from Microsoft and the Zeek dev team over the past couple years, we've been able to "catch up" with current Zeek such that we no longer have the build problems previously tracked in this issue. The newer repo https://github.com/brimdata/build-zeek is now being used to build current Zeek releases for bundling with Brimcap as they come out. Therefore this issue can be closed.

@philrz philrz closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant