Skip to content

tools: copy hdr_histogram_internal.h in update-histogram.sh - #66528

Open
fcostaoliveira wants to merge 1 commit into
nodejs:mainfrom
fcostaoliveira:tools-histogram-updater-header
Open

fcostaoliveira wants to merge 1 commit into
nodejs:mainfrom
fcostaoliveira:tools-histogram-updater-header

Conversation

@fcostaoliveira

@fcostaoliveira fcostaoliveira commented Oct 5, 2026 •

Copy link
Copy Markdown

tools/dep_updaters/update-histogram.sh copies a fixed list of files from the HdrHistogram_c release into deps/histogram.

HdrHistogram_c 0.12.0 (released 2026-10-02, https://github.com/HdrHistogram/HdrHistogram_c/releases/tag/0.12.0) made src/hdr_tests.h include a new private header, src/hdr_histogram_internal.h, and that header is not on the list.

As a result the automated update, #66494, fails to build in CI:

deps/histogram/src/hdr_tests.h:12:10: fatal error:
'hdr_histogram_internal.h' file not found

This adds the header to the cp line. Nothing else changes: histogram.gyp, BUILD.gn and unofficial.gni list only src/hdr_histogram.c and the public header, and the other files 0.12.0 needs (hdr_atomic.h, hdr_malloc.h, hdr_tests.h) are already copied.

The 0.12.0 update itself is not part of this PR; it is #66494, which still needs the header file added (see my comment there).

How I checked it:

  • Resolved the project-local #includes of src/hdr_histogram.c at the 0.12.0 tag, transitively: it needs hdr_histogram.h, hdr_atomic.h, hdr_tests.h, hdr_histogram_internal.h and itself. Before this change only hdr_histogram_internal.h is missing from the copy list.
  • All five failing jobs of deps: update histogram to 0.12.0 #66494 (Linux x64 and arm64, macOS, the tarball build and Windows coverage) fail with exactly this error and no other.
  • Ran tools/dep_updaters/update-histogram.sh in a checkout of main (c56cb094), which fetched 0.12.0: deps/histogram/src/hdr_histogram_internal.h appears as a new file with this change. Syntax-checked with cc -fsyntax-only -Ideps/histogram/src -Ideps/histogram/include deps/histogram/src/hdr_histogram.c, which succeeds. I did not run a full Node build. I then discarded the update, so this PR contains only the one-line script change.

AI use disclosure: I used an AI coding assistant (Claude Code) to analyse the updater script against the 0.12.0 tag and to draft this one-line patch and this description. I reviewed the change and the include analysis myself and I will answer review comments myself. I am a maintainer of HdrHistogram_c.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

HdrHistogram_c 0.12.0 added a private header,
src/hdr_histogram_internal.h, which src/hdr_tests.h now includes.
update-histogram.sh copies a fixed list of files, so the automated
update to 0.12.0 would leave deps/histogram without it and fail to
compile. Add the header to the list.

Signed-off-by: fcostaoliveira <filipe@redis.com>
@fcostaoliveira
fcostaoliveira force-pushed the tools-histogram-updater-header branch from 981399c to 532f6c7 Compare October 5, 2026 10:02
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. labels Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants