Skip to content

ci(codeql): add binutils-dev back for analyzesigs.cc bfd.h include#74

Merged
CryptoJones merged 1 commit into
masterfrom
ci/codeql-c-cpp-binutils-dev-fix
May 26, 2026
Merged

ci(codeql): add binutils-dev back for analyzesigs.cc bfd.h include#74
CryptoJones merged 1 commit into
masterfrom
ci/codeql-c-cpp-binutils-dev-fix

Conversation

@CryptoJones
Copy link
Copy Markdown
Owner

Salvages the part of #50 that didn't land. PR #51's squash-merge accidentally included PR #50's first commit (the wrong-binutils-dev one) because PR #51's branch was inadvertently based on PR #50's branch, not master. So master ended up with the broken CodeQL config that PR #50 was trying to fix-forward; PR #50 then can't merge because its first commit is already on master.

This PR cherry-picks only PR #50's second commit (65eee71c) onto master, with no replay of the broken first commit.

The fix (verified to make Analyze (c-cpp) pass on PR #50 — last green run was 13m12s):

# bison + flex are needed to regenerate xml.cc / slghparse.cc /
# slghscan.cc from their .y / .l sources during the manual
# build below. binutils-dev + libiberty-dev provide bfd.h —
# we don't *link* against libbfd (libdecomp_dbg.a is a static
# archive, no link step), but `analyzesigs.cc` and
# `loadimage_bfd.cc` `#include <bfd.h>` and therefore need
# the header at compile time.
run: |
  sudo apt-get update
  sudo apt-get install -y bison flex g++ make binutils-dev libiberty-dev

After this lands, the Analyze (c-cpp) CodeQL job should pass on master and on subsequent PRs, instead of the preexisting cpp/autobuilder: No supported build system detected failure that was hitting every PR (including doc-only ones).

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

PR #50 first attempt removed `binutils-dev libiberty-dev` from the
apt-get install on the theory that libdecomp_dbg.a is a static
archive with no link step. That was wrong:

  In file included from analyzesigs.cc:17:
  loadimage_bfd.hh:37:10: fatal error: bfd.h: No such file or directory
     37 | #include <bfd.h>
        |          ^~~~~~~

`analyzesigs.cc` and `loadimage_bfd.cc` `#include <bfd.h>` at the
top of the file, so the header is needed at compile time even when
we never link against `-lbfd`. Restoring the original deps install.

The autobuild replacement (`make libdecomp_dbg.a`) stays as the
right approach — the failure was a stale assumption about which
headers the archive's component .o files need, not about the
overall strategy.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CryptoJones added a commit that referenced this pull request May 26, 2026
PR #51's branch was inadvertently based on the in-flight PR #50
CodeQL-fix branch (not master), so PR #51's squash-merge included
PR #50's broken first commit alongside the intended lvalue RAII
change. Master at f41d8fc ended up with the broken CodeQL
config; PR #50 couldn't merge as-is; PR #52 (xml global_scan,
stacked on PR #51) also auto-closed when its base disappeared.

Mitigation:
  - PR #74 cherry-picks PR #50's second commit (binutils-dev
    fix) onto current master cleanly.
  - PR #73 cherry-picks PR #52's global_scan commit onto current
    master cleanly.

Adds an entry to Apologies.md at the top (per the log policy)
recording cause + downstream damage + mitigation.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CryptoJones CryptoJones merged commit 8d41b56 into master May 26, 2026
14 checks passed
@CryptoJones CryptoJones deleted the ci/codeql-c-cpp-binutils-dev-fix branch May 26, 2026 10:03
CryptoJones added a commit that referenced this pull request May 26, 2026
…leased] (#76)

The catch-up changelog PR (#72) listed #50, #51, #52 as in-flight.
Now resolved:

  - #51 (lvalue) — merged (was the lone in-flight item that landed
    cleanly).
  - #50 (CodeQL fix) — superseded by #74 after the stacking mistake.
    #74 landed and Analyze (c-cpp) now passes on master.
  - #52 (global_scan) — superseded by #73 after the same stacking
    mistake. #73 landed.
  - #75 (Apologies) — landed alongside, recording the chain.

Removes the "in flight" footnote and replaces with a paragraph
explaining the chain of events so readers understand why #50 / #52
are absent from the merged ledger and #73 / #74 are present
covering the same scope.

Aaron's per-PR changelog feedback (feedback_changelog_per_pr.md)
applied: this PR ships its own changelog touch alongside the actual
state change, not as a catch-up.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant