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

Fix compilation error with GCC 14 due to missing header #413

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

ckreibich
Copy link
Member

@ckreibich ckreibich commented Jun 3, 2024

Zeek 6.2 no longer compiles with GCC 14.1.1 (the default on Fedora 40):

/home/christian/devel/zeek/broker/tests/benchmark/broker-benchmark.cc: In function ‘broker::list_builder {anonymous}::createEventArgs()’:
/home/christian/devel/zeek/broker/tests/benchmark/broker-benchmark.cc:138:12: error: ‘sort’ is not a member of ‘std’; did you mean ‘qsort’?
  138 |       std::sort(keys.begin(), keys.end());
      |            ^~~~
      |            qsort
/home/christian/devel/zeek/broker/tests/benchmark/broker-benchmark.cc:144:14: error: ‘sort’ is not a member of ‘std’; did you mean ‘qsort’?
  144 |         std::sort(vals.begin(), vals.end());
      |              ^~~~
      |              qsort
make[3]: *** [tests/CMakeFiles/broker-benchmark.dir/build.make:76: tests/CMakeFiles/broker-benchmark.dir/benchmark/broker-benchmark.cc.o] Error 1

I'm going to mark this down on the list of issues for upcoming Zeek patch releases. Zeek 6.0 is not affected.

Apparently this now needs <algorithm>.
@ckreibich
Copy link
Member Author

ckreibich commented Jun 3, 2024

I don't get why clang-tidy is blowing up here. Did we miss the fact that it did so in the past? I've not gone near the things it's complaining about. @timwoj, @Neverlord, I'd be grateful, thx...

@timwoj
Copy link
Member

timwoj commented Jun 3, 2024

Probably #365

@Neverlord
Copy link
Member

Neverlord commented Jun 13, 2024

I don't get why clang-tidy is blowing up here. Did we miss the fact that it did so in the past? I've not gone near the things it's complaining about. @timwoj, @Neverlord, I'd be grateful, thx...

The noexcept: clang-tidy is just right here. std::map::find isn't noexcept, so we can't (shouldn't) make functions that call that noexcept. The other warning I just suppressed for now, since it's not something we care about.

/edit: seems there's more. I'll look into it.

@Neverlord Neverlord force-pushed the topic/christian/fix-gcc-14-fail branch 4 times, most recently from fc14b03 to 6a702cb Compare June 13, 2024 19:23
@Neverlord
Copy link
Member

@timwoj can I pull this into master or is this going to be merged into a separate branch for Zeek 6.2?

@awelzel
Copy link
Contributor

awelzel commented Jun 17, 2024

@timwoj can I pull this into master or is this going to be merged into a separate branch for Zeek 6.2?

This PR is marked/labeled for release/zeek-6.2 which is used for Zeek 6.2 (and IIUC part of the fixes are already in master).

So maybe open a new PR with just the clang-tidy fixes targeted for master ?

@awelzel
Copy link
Contributor

awelzel commented Jun 18, 2024

@Neverlord and me chatted - not 100% sure what the plan is/was but seems

  • we don't necessarily need the clang-tidy fixes for 6.2.1 6.2.2
  • the header include fix we need though

So he'll force-push remove the clang-tidy fixes (and re-spin in a separate PR) and the rest of the fixes would be merged into release/zeek-6.2 for 6.2.2.

@Neverlord Neverlord force-pushed the topic/christian/fix-gcc-14-fail branch from 6a702cb to 3beedb5 Compare June 18, 2024 16:07
@awelzel awelzel merged commit 8c8c473 into release/zeek-6.2 Jun 24, 2024
22 of 23 checks passed
@awelzel awelzel deleted the topic/christian/fix-gcc-14-fail branch June 24, 2024 09:48
@timwoj
Copy link
Member

timwoj commented Jul 8, 2024

As a heads up, we won't be doing any more 6.0/6.2 releases once 7.0 is released.

@awelzel
Copy link
Contributor

awelzel commented Jul 9, 2024

As a heads up, we won't be doing any more 6.0/6.2 releases once 7.0 is released.

It might be that for 6.0 there's nothing interesting right now? But there's minimally the following in Release Cadence:

Once a new LTS release comes out, we will stop maintaining the previous one after one further feature release on top of that new LTS.

This PR concerns only 6.2 though IIUC.

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.

4 participants