Commit Graph
100 Commits
Author SHA1 Message Date
Scot Breitenfeld 46323c29b8 review-checklist: ping assignee(s) and label PRs once fully signed off (#6672)
* review-checklist: ping GitHub assignee(s) once the checklist is fully signed off

Adds computeAssigneePing(), a pure function that posts a ready-to-merge
comment mentioning the PR's assignees the moment every CODEOWNERS area
transitions from pending to signed-off. Gated on the false->true
transition so it fires once, not on every later workflow run while the
PR stays fully approved, and posted as its own comment since editing
an existing comment to add a mention isn't a reliable notification.

* review-checklist: sync a checklist-complete label with the all-done state

Toggles the "checklist-complete" label (pre-created in the repo, green
#0E8A16) on and off to track whether every CODEOWNERS area is currently
signed off, so it's visible as a badge on the /pulls list without
opening each PR. Unlike the assignee ping, this is level-triggered: the
label comes off again if a later change request breaks the sign-off,
and reattaches once the PR is fully approved again.
2026-09-15 14:33:17 -05:00
Scot Breitenfeld 31d9d3c322 Fix release-progress badges to read native issue-level Priority field (#6668)
* Fix release-progress badges to read native issue-level Priority field

The Project Priority board field was consolidated into a Priority field
managed at the issue level (GitHub's org-wide custom issue fields), which
the GraphQL API mirrors into projects as ProjectV2ItemIssueFieldValue
instead of ProjectV2ItemFieldSingleSelectValue. The badge script's query
had no fragment for that type, so it silently found zero priority values
and would raise ProjectFieldMissingError on every run.

* Show TBD on the Next Release badge when no milestone due date is set

Previously the badge silently omitted any due-date annotation when the
milestone had none, which read as if the badge just hadn't picked one up.

* Update release-schedule image and plantuml source from add-next-release-badge branch

* Don't flag CODEOWNERS auto-assignment as a manual reviewer add on a PR's first pass

GitHub's own CODEOWNERS engine fires an identical review_requested webhook
(sender type User, not Bot) when it auto-assigns owners at PR-open time. If
one of those survives the cancel-in-progress race as the run that actually
executes, it was mistaken for a human deliberately requesting that reviewer,
permanently flagging them "manually added" and requiring separate approval —
e.g. a catch-all "*" owner also named on a touched area's CODEOWNERS line got
flagged on every PR touching that area, even though no human ever picked them.

Gate the manual-add detection and the sticky per-area assignment on
!isFirstCoordinationPass (no checklist comment posted yet), since that's
exactly the window where GitHub's own auto-assignment is indistinguishable
from a real human pick.
2026-09-15 14:32:55 -05:00
Scot Breitenfeld 7489f285be Fix clang-format-fix.yml: give EndBug/add-and-commit its own push token (#6669)
Checkout deliberately uses persist-credentials: false so the workflow
token never lands in git config, but the add-and-commit step never got
a token of its own to push with -- it silently relied on credentials
checkout no longer leaves behind. Every push to any branch failed with
"could not read Username for 'https://github.com'" once clang-format
found something to fix, regardless of the repo's Actions write-permission
setting.
2026-09-12 08:46:25 -05:00
Scot Breitenfeld 9515219a32 Review automation: fix comment-only reviewer drop and draft-stale thrash (#6658)
* review-checklist: don't lose a reviewer who only left comments

GitHub un-requests a reviewer the instant they submit any review,
including a comment-only one from batching several inline comments into
a single submission — not just Approve/Request-changes. Unlike a stale
approval, this never produces a DISMISSED transition, so the checklist
had no way to tell "abandoned the area" apart from "still reviewing,
just hasn't finished yet."

This silently dropped the reviewer's mention from the checklist display
the moment they left comments, and risked the next push's additive-fill
picker handing their area to a completely different load-balanced
reviewer (observed on PR #6645: jhendersonHDF's batched review comments
repeatedly vanished him from src/test's rows mid-review).

Track a sticky area assignee as still engaged whenever they have no
APPROVED/CHANGES_REQUESTED/DISMISSED review on record, regardless of
whether GitHub currently lists them as requested, and use that in the
read-only display path, resolveAreaPicks, and the additive-fill picker.

* draft-pr-policy: a checked keepalive checkbox is real activity too

lastRealActivityAt() deliberately ignores bot comments so a metadata-only
bump can't dodge the staleness check forever. But checking the "Still
working on this" checkbox is an edit to the bot's own keepalive comment,
not a new comment of the human's own — its author stays
github-actions[bot], so the edit was invisible to the activity check too.

Confirming via the checkbox removed the label and posted "Thanks for
confirming" without ever moving the underlying 60-day clock, so the very
next scheduled run saw the same stale last-activity timestamp and
immediately re-flagged it — contradicting the checkbox's own promise
that checking it "resets this".

Observed on #6326: once its true last activity fell behind the 60-day
window, and only the checkbox (never a new commit or comment) was used
to confirm it, the label thrashed on and off on a roughly 1-2 day loop.

Now a checked keepalive checkbox counts via the comment's updated_at
(when it was toggled), same as any other real activity signal.
2026-09-04 08:55:42 -05:00
Scot Breitenfeld 780f6e40de Skip runner spin-up for draft-pr-policy comments unless already stale (#6631)
The issue_comment trigger let every PR comment boot a full runner just
to make one API call checking draft status, even though the only
comments that matter are ones on PRs already labeled draft-stale. That
label snapshot is already present in the webhook payload, so gate the
job on it directly and drop the now-redundant draft-status API step.
2026-09-01 15:14:25 -05:00
Scot Breitenfeld 9b77f784f7 Add parallel example for HDF5 on an MPI_Comm_split sub-communicator (#6646)
Adds HDF5Examples/C/H5PAR/ph5_comm_split.c, showing parallel HDF5 use on a
communicator produced by MPI_Comm_split rather than on MPI_COMM_WORLD.

The example splits MPI_COMM_WORLD by rank parity. The even ranks form a
writer group that collectively creates a dataset with one row per writer
rank, then reopens the file read-only on the same sub-communicator and
verifies what it wrote. The odd ranks stand in for a group with other
responsibilities and make no HDF5 calls at all.

The comments cover the two rules this pattern depends on: MPI_Comm_split is
collective over the parent communicator, so every rank must call it even if
it never opens the file; and once a file is opened with a FAPL carrying a
sub-communicator, every collective HDF5 call on that file is collective over
that sub-communicator alone.

The example runs with any number of MPI ranks.
2026-09-01 15:13:31 -05:00
Scot Breitenfeld 109e670e73 Fix h5open_f failing to re-initialize the Fortran interface (#6642) (#6649)
h5close_f reset its count of the objects created by h5open_f with

  CALL h5fget_obj_count_f(INT(H5F_OBJ_ALL_F,HID_T), H5F_OBJ_ALL_F, &
                          H5OPEN_NUM_OBJ, error)

passing the H5F module variable H5OPEN_NUM_OBJ as the actual argument for
the INTENT(OUT) obj_count dummy, while h5fget_obj_count_f also reads
H5OPEN_NUM_OBJ by use association. F2018 15.5.2.13 prohibits referencing a
variable through use association once it has been redefined through a dummy
argument in the same call, so the result depended on how the compiler
implemented argument association. Where the actual argument was passed by
reference the subtraction collapsed to 0 - 0 and produced the intended zero;
where the compiler used copy-in/copy-out it evaluated 0 - H5OPEN_NUM_OBJ and
left the count negative.

A negative count then defeats the guard at the top of h5open_f, which returns
early when H5OPEN_NUM_OBJ is non-zero. h5open_f reported success without
calling h5init_types_c, leaving H5T_NATIVE_INTEGER and the other predefined
types holding identifiers that h5close_f had released.

h5close_f now assigns the count directly, which is what the comment there has
always described. h5fget_obj_count_f computes into a local variable so that no
caller can reintroduce the aliasing; note that this change alone would make
the old h5close_f call site produce the negative count on every compiler
rather than only on some, so the two belong together. H5OPEN_NUM_OBJ is also
given an initial value, since h5open_f tests it before anything assigns to it.

h5fget_obj_count_f subtracted every object created by h5open_f from a count of
a single object type, so with the interface open a query such as

  CALL h5fget_obj_count_f(INT(H5F_OBJ_ALL_F,HID_T), H5F_OBJ_FILE_F, n, error)

returned a negative n and hdferr of 0. h5open_f now records what it leaves open
per object type, and a count is adjusted by the recorded value for the types
being counted, so the adjustment does not depend on which types the
initialization creates. The check for a negative count runs both on the value
returned by H5Fget_obj_count and after the adjustment.

h5fget_obj_ids_f applied no such adjustment, so it returned the identifiers
h5open_f opened alongside the application's own and disagreed with
h5fget_obj_count_f about the same query: with only a file and a group open,
H5F_OBJ_ALL_F counted 2 objects but listed 62. The C API reports 2 and 2. An
application walking the list found datatypes it never opened, and closing them
breaks the Fortran interface. h5fget_obj_ids_f now excludes those identifiers,
requesting enough from H5Fget_obj_ids that max_objs of the application's own
can still be returned when the two are interleaved.

The h5open/h5close test verified its object counts by calling
h5fget_obj_count_f after h5close_f, when h5open_f is the only call the Fortran
interface permits. Those checks move to after the interface is reopened, where
they additionally confirm that the predefined types are valid again, that the
preceding h5close_f released the previous h5open_f's types, and that
h5fget_obj_ids_f agrees with h5fget_obj_count_f.

The Fortran tests also aborted unrecoverable failures with STOP, which exits
with a success status whether the stop code is absent or is a string, so a run
that died part way through reported no failure to CTest. They now exit through
h5_exit_f(1). The STOPs that end a run normally, in fflush1 and in the async
test's skip path for a build without MPI_THREAD_MULTIPLE, are unchanged.

Fixes #6642
Fixes #6648

Reported and diagnosed by Dom Heinzeller.
2026-09-01 15:12:55 -05:00
Scot Breitenfeld d8ec63966c Fix segfault in ph5_f90_filtered_writes_no_sel example: swap inverted mem_space_id (#6633)
The Fortran example set sel_type = H5S_BLOCK_F on the branch where a rank
calls H5Sselect_none (no data to contribute) and H5S_ALL_F on the branch
where a rank sets a real hyperslab, which is backwards. The equivalent C
example (ph5_filtered_writes_no_sel.c) does the opposite:

    H5Dwrite(..., no_selection ? H5S_ALL : H5S_BLOCK, ...);

i.e. H5S_ALL when there is no selection (mirrors the file dataspace's own
empty selection) and H5S_BLOCK when a hyperslab was set (memory buffer
matches the block shape of the file selection).

With the inversion, H5Dwrite_f feeds a bogus selection into the collective
filtered I/O path, corrupting the element/byte-length accounting in
H5D__mpio_collective_filtered_chunk_update, which segfaults inside
H5D_select_io_mem -> H5VM_memcpyvv on 12 MPI ranks. Reproduces on current
develop; the bug has been present since the example was added in #3916.
2026-08-25 12:07:04 -05:00
Scot BreitenfeldandH. Joe Lee 7b37aac674 review-checklist: make avalanche pruning prefer the already-settled reviewer (#6596)
Every CODEOWNERS "avalanche" (multiple owners of one area simultaneously
requested — on PR creation, on ready_for_review, or when a push first
touches an already-covered area) was resolved by re-running the
load-balancer from scratch, with no memory of who was already the
settled reviewer for that area. Since open-PR review load drifts over
time, this made the pick non-deterministic across repeated events and
caused three related symptoms:

- marking a draft ready for review could swap out reviewers who were
  already assigned (manually or from an earlier pass) for different
  people
- a routine follow-up push could re-avalanche an area and bump the
  already-engaged reviewer for whoever currently has a lighter queue
- a manually re-requested reviewer survived the triggering event (the
  existing forced-pick handling already covers that) but got silently
  removed again on a later event, since two now-requested owners look
  identical to an unpruned avalanche without that memory

Add a persisted per-area "sticky assignment" record (ASSIGNED_PREFIX,
alongside the existing exclusion and manually-added markers) and a
resolveAreaPicks() helper that prefers a valid sticky pick, then a lone
already-requested owner, before ever falling back to a fresh
load-balanced pick. Manual review_requested actions now write to this
record too, so a deliberate reviewer choice holds up across future
runs, not just the run it was made on.

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-08-05 22:02:52 -05:00
Scot Breitenfeld aa9df72cc2 Add Fortran ABI check and auto-detect the release baseline (#6550)
* Add Fortran ABI compatibility check to abi-report workflow

Mirrors the existing C/HL/C++ abi-dumper + abi-compliance-checker
steps for libhdf5_fortran.so, plus a supplementary nm-based exported
symbol diff since gfortran's mangled names don't encode argument
lists and DWARF-based diffing has not been validated against
Fortran-specific constructs (array descriptors, derived types).

* Auto-detect ABI reference release instead of hardcoding it

file_ref was pinned to '2.0.0' in both daily-build.yml and release.yml
and had gone stale across two releases (2.1.0, 2.1.1) without being
bumped, silently comparing against a two-versions-old baseline. Have
abi-report.yml query the GitHub API for the latest published
HDFGroup/hdf5 release when file_ref isn't explicitly given, so callers
no longer need to remember to update a pinned tag after every release.
Verified the resolution command returns 2.1.1 against the live API.

Also documents the expected false-positive pattern in the new Fortran
ABI check: abi-dumper misreads gfortran's DWARF encoding of
assumed-shape array descriptors as fixed array bounds, producing bulk
Low-severity noise on the KIND/RANK-generated H5_gen.F90 procedures
that isn't a real interface change.
2026-07-27 08:28:18 -05:00
Scot Breitenfeldandgithub-actions 400de1dc95 pre-size chunk buffer to avoid realloc fragmentation; fix nbit/scaleoffset no-op return values (#6389)
* fix: pre-size filter output buffer to avoid realloc fragmentation on Windows

When reading a filtered (compressed) chunk, filters like deflate initialize
their output buffer using *buf_size (the pipeline's buffer capacity hint).
With *buf_size set to chunk_disk_size (the compressed size), deflate grows
its output buffer via repeated realloc() doubling. On Windows this fragments
the heap and causes read times to increase steadily across iterations.

Set buf_alloc to MAX(chunk_disk_size, chunk_size) immediately before calling
H5Z_pipeline so filters pre-allocate their output at the full uncompressed
size and avoid realloc. The inbuf is still allocated at chunk_disk_size —
only the hint passed to the pipeline is enlarged — so there is no
double-large-allocation overhead and peak memory stays at
chunk_disk_size + chunk_size rather than 2 * chunk_size.
For incompressible chunks where chunk_disk_size > chunk_size the hint
remains at chunk_disk_size, satisfying H5Z_pipeline's post-filter size
validation.

Fixes #4481 and #4513.

* fix: return nbytes instead of *buf_size in nbit and scaleoffset no-op paths

Per the HDF5 filter API, nbytes is the count of valid data bytes in the
buffer, while *buf_size is the allocated buffer capacity. These two values
were historically always equal on the read path, so either worked in
practice. The pre-sizing change (which sets buf_alloc = chunk_size before
the filter pipeline) makes *buf_size > nbytes for compressed reads,
exposing the latent bug.

In H5Znbit.c: the no-compress pass-through (cd_values[1] == 1) was
returning *buf_size, causing the next filter (e.g. fletcher32) to treat
the full pre-sized buffer as valid data and compute a checksum over
uninitialised bytes.

In H5Zscaleoffset.c: the no-process path had the same pattern. No current
test exercises this path through a multi-filter pipeline where the size
mismatch would be observable, but the fix is correct by the same API
reasoning.

* experiment: deflate decompress reuses inbuf via H5resize_memory

Instead of allocating a fresh output buffer (H5MM_malloc), copy the
compressed input to a small temp buffer and resize *buf in-place via
H5resize_memory. On Windows, HeapReAlloc can often extend an existing
heap block without moving it, avoiding the cost of finding and committing
a fresh large allocation for every chunk read.

This is combined with the pre-sizing hint in H5Dchunk.c that sets
buf_alloc = chunk_size before the pipeline call, so the resize goes
directly to chunk_size in one step with no realloc loop.

* fix: pre-resize chunk buf in H5Dchunk.c so all filters see correct capacity

Replace the hint-only buf_alloc enlargement with an actual
H5D__chunk_mem_realloc() before calling H5Z_pipeline.  Every filter now
sees *buf_size equal to the real buffer capacity, not just an advisory
hint.  This fixes a bounds-check regression in H5Zscaleoffset where the
read path uses *buf_size as the end-of-buffer sentinel in
H5_IS_BUFFER_OVERFLOW and as the input-size argument to
H5Z__scaleoffset_decompress; an inflated hint caused false-negative
overflow checks and potential over-reads when scaleoffset is the on-disk
filter.

H5Zdeflate is simplified accordingly: the up-front H5resize_memory(*buf,
nalloc) is removed since the buffer is already at nalloc on entry.  The
inbuf copy is retained (still needed to read compressed input while
writing uncompressed output into the same buffer), as is the
realloc-doubling loop for the uncommon case where output exceeds
chunk_size.

Also add test/chunk_deflate_perf.c, a standalone benchmark that times
per-chunk deflate reads over multiple passes to detect the steady read-time
increase caused by heap fragmentation on Windows (issues #4481 / #4513).

* Committing clang-format changes

* test: add file path argument to chunk_deflate_perf benchmark

Add optional 4th argument to specify the output HDF5 file path
(default: chunk_deflate_perf.h5 in CWD).  Allows running develop
and fix builds against separate files so pass 1 is cold-cache for
both and the two runs don't share page-cache state.

* test: remove chunk_deflate_perf benchmark

Not suitable for the test suite; intended for manual Windows validation
only.  Keep locally if needed.

* fix: pre-resize chunk buf in H5Dchunk.c so all filters see correct capacity

Allocate the chunk read buffer at MAX(chunk_disk_size, chunk_size) from
the start rather than allocating at chunk_disk_size and immediately
reallocating.  The read only fills chunk_disk_size bytes regardless of
buffer size, so there is no cost to the larger initial allocation and the
separate realloc step is eliminated.

Every filter now receives *buf_size equal to the actual buffer capacity
with no additional allocation needed.  On Windows a single HeapAlloc at
the correct size avoids the repeated realloc-doubling in the deflate
filter that fragments the heap and causes read times to increase over
successive iterations (issues #4481 / #4513).  Also fixes the
scaleoffset bounds-check regression where *buf_size was used as an
end-of-buffer sentinel.

* revert: restore H5Dchunk.c and H5Zdeflate.c to pre-experiment state

Reverts the deflate in-place decompression experiment and the H5Dchunk.c
pre-sizing changes back to the state at 9ab5e191d7, keeping the nbit
and scaleoffset no-op path fixes.

* fix: allocate chunk buf at MAX(disk_size, chunk_size) before filter pipeline

The previous approach allocated the buffer at chunk_disk_size and then
bumped buf_alloc to chunk_size as a hint to the pipeline, causing *buf_size
to misrepresent the actual allocation.  Any filter that writes up to
*buf_size bytes into *buf would overflow.

Allocate at MAX(chunk_disk_size, chunk_size) upfront so the buffer and
the hint given to filters are always consistent.  For incompressible chunks
where chunk_disk_size >= chunk_size the allocation is unchanged.

* Use HGOTO_DONE(nbytes) for the scaleoffset no-op passthrough

Matches the macro convention used for the other early-return in this
function per bmribler's review comment on PR #6389.

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
2026-07-21 12:27:52 -06:00
Scot Breitenfeld 5bcbe6c6d8 Add release-progress badges: Next/Latest Release, Medium/Low priority (#6537)
* Add Next Release badge and Medium/Low priority progress badges

Adds a "Next Release" badge (in-development version from H5public.h)
and extends the existing Critical/High priority progress badges with
Medium (P2) and Low (P3) priority tracking, all fed from the same
GitHub Project #39 gist-based badge pipeline.

* Remove in-progress 2.0 entry from release schedule diagram

The Release Schedule chart is meant to show only past releases that
have reached end of life; the in-development series is now tracked
separately by the Release Progress badges. Regenerate the PNG from
the updated PlantUML source and clarify the README wording.

* Add Latest Release badge and milestone target date to Next Release

Fetches the most recent published release in the current major
version series from the GitHub Releases API (e.g. "2.1.1
(2026-03-23)") and exposes it as a new "Latest Release" badge.
Also looks up the target due date of the matching GitHub milestone
(e.g. "HDF5 2.2.0") and appends it to the existing Next Release
badge when one is set. Both lookups are independent of the
project-board query and degrade gracefully to "N/A" on failure.
2026-07-20 22:10:41 -06:00
Scot Breitenfeld 1fc781961d Add change-request and manually-added-CODEOWNER lines to review checklist (#6528)
* Add change-request and manually-added-CODEOWNER lines to review checklist

Reviewers who request changes now get their own line under each area
their inline comments touch, whether or not they're a CODEOWNER or a
requested reviewer. CODEOWNERS who are review-requested directly by a
human (rather than auto-picked) get a separate required-approval line,
and their approval is now required for that area's sign-off in addition
to the usual reviewer's.

* Prune reviewers whose area dropped out of scope

A CODEOWNER requested for an area the PR used to touch, before a later
push narrowed the diff, was never removed: touchedAreaOwners only
reflects areas touched right now, so none of the existing avalanche
pruning ever considered them. Reproduced live on PR #6528, where the
branch's diff shrank to just .github/ but gheber/fortnern/mattjala
(owners of unrelated areas) stayed as requested reviewers.

Manually-added CODEOWNERS are exempt, since a human's deliberate
request shouldn't be silently undone by a later push.

* review-checklist: fix avalanche exemption regressing genuine avalanches

#6524 exempted a review_requested login's whole area from avalanche
detection to stop a manual re-request from being undone. But a genuine
CODEOWNERS avalanche's own review_requested sub-events look identical
to that case, and one of them can survive the ready_for_review vs.
review_requested concurrency race (#6530: draft PR marked ready for
review left all 3 CODEOWNERS owners requested instead of pruning to
one, because the surviving run's action was one of the avalanche's own
review_requested events, exempting the whole area).

Replace the exemption with a forced pick: the directly-requested login
still always survives, but now by replacing the rest of that area's
avalanche instead of exempting the area from pruning — so a genuine
avalanche still collapses to one person. Also gate justRequestedLogin
on !isBotSender, matching updatedManuallyAdded's existing reasoning
just above it: the bot's own requestReviewers calls fire this same
event and aren't a human decision.
2026-07-16 11:23:26 -05:00
Scot Breitenfeld 50465b5f6a review-checklist: exempt a just-requested reviewer from avalanche pruning (#6524)
A direct review_requested (e.g. manually re-requesting a reviewer via
the GitHub UI) adds a second currently-requested owner to that
reviewer's area — indistinguishable from an unpruned CODEOWNERS
avalanche, so the avalanche-detection pass immediately removed them
again on the same run. Carve out the login this run's review_requested
action names before running avalanche detection.
2026-07-13 17:21:15 -05:00
Scot Breitenfeld d9d1c88e65 CI: skip draft-pr-policy checkout on issue-only comments (#6519)
* ci: skip draft-pr-policy checkout on issue-only comments

The issue_comment event fires for comments on both issues and PRs;
draft-pr-policy only cares about PR draft-staleness, so short-circuit
before checkout when the comment is on a plain issue. Also switch all
three jobs to a sparse, shallow checkout since they only need
.github/scripts to load the github-script payload.

* ci: also skip draft-pr-policy on bot-authored PR comments

draft-pr-policy.js already excludes Bot comments when deciding whether
a draft was revived, so a bot comment (e.g. review-checklist posting
its checklist) can never do anything meaningful here — it still ran
checkout + API calls only to no-op. Filter it out at the same if:
using the free github.event.comment.user.type field.

* ci: skip checkout for draft-pr-policy on non-draft PR comments

Even after filtering out issue-only and bot comments, a human comment
on any non-draft PR still paid for a full checkout just to have the
script itself discover pr.draft is false and return. Add a cheap
pulls.get-only pre-check step (no checkout required) and gate the real
checkout + script load on its result.
2026-07-13 08:58:18 -05:00
Scot Breitenfeld c081ea17a7 CI: restrict review-checklist workflows to HDFGroup/hdf5 only (#6515)
These workflows use pull_request_target / workflow_run with elevated
permissions (pull-requests: write, issues: write). Because GitHub
copies workflow files into forks, any fork with Actions enabled was
independently running these privileged workflows against its own
PRs, e.g. https://github.com/sp26-hdfgroup/hdf5-sandbox/pull/2.

Add the same github.repository == 'HDFGroup/hdf5' guard already used
in review-checklist-test.yml so the job-level if short-circuits
before doing anything in a fork's copy of the workflow.
2026-07-09 13:10:37 -05:00
Scot BreitenfeldandH. Joe Lee 464824974d CI: allow safe fork-PR checkout in review-checklist workflow (#6512)
actions/checkout v7.0.0 (bumped in #6500) added a blanket check that
blocks checkout on pull_request_target/workflow_run events, even when
no fork ref is checked out. This workflow only ever checks out the
base branch (develop), so opt in via allow-unsafe-pr-checkout.

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-07-08 21:52:03 -05:00
Scot BreitenfeldandH. Joe Lee cab98811c6 h5repack: fix silent loss of a declared cd_nelmts for UD filters (#6466)
A declared cd_nelmts with no values following it (e.g. UD=<filtn>,<flag>,1)
was never committed to filt->cd_nelmts: the trailing token (no comma after
it) was always treated as a value, never checked against the l/f/p pending
flags the way the comma-triggered branch does. h5repack's own validation
then silently passed since cd_nelmts stayed at its default 0, and the
mismatch was only ever caught by coincidence when the underlying filter
plugin did its own internal cd_values check (issue #6462).

Commit the trailing token to whichever UD field is still pending, mirroring
the comma-triggered branch exactly.

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-07-01 16:45:16 -05:00
Scot Breitenfeld 8f2c548743 Fix runExecute.cmake: restore ERROR_APPEND and fix zero-or-more mask patterns (#6484)
* Fix runExecute.cmake: restore ERROR_APPEND and fix zero-or-more mask patterns

Three bugs were introduced when runTest.cmake was refactored into runExecute.cmake:

1. ERROR_APPEND=1 support was dropped. Tests that redirect their error-stack
   output to stderr and rely on ERROR_APPEND to combine it with stdout before
   comparison (e.g. H5TEST-err_compat, H5TEST-error_test) fail silently because
   the reference file never matches the truncated actual output.

2. " line [0-9]*" uses zero-or-more digits, so it matches " line " (with zero
   digits) in already-masked reference text like " line (number)", prepending
   another "(number)" and producing " line (number)(number)".

3. "HDF5 [1-9]*[.][0-9]*..." has the "HDF5 " prefix and uses [1-9]* (zero-or-
   more), so it does not match "2.2.0" in actual output (blocked by the leading
   "("), and double-masks "version (number)" in the reference.
   "H5Eget_auto[1-2]*" / "H5Eset_auto[1-2]*" have the same zero-or-more issue,
   double-masking "(1 or 2)" already present in reference files.

Fix: restore ERROR_APPEND handling; change * to + for all digit/char class
quantifiers that can match zero occurrences in already-masked reference text;
remove the "HDF5 " prefix from the version pattern. Same fix applied to
HDF5Examples/config/cmake/runExecute.cmake which has identical bugs.

* Fix AOCC CI: source setenv_AOCC.sh from its actual install location

install.sh generates setenv_AOCC.sh inside the aocc-compiler-VERSION/
directory (where it runs from), not in the repo root one level up.

* Fix AOCC CI: replace hardcoded runner paths with GITHUB_WORKSPACE

All /home/runner/work/hdf5/hdf5/ paths were hardcoded to the HDFGroup/hdf5
repo name. Running the workflow in any fork (e.g. brtnfld/hdf5_swmr_2)
puts the checkout under a different path, breaking AOCC/OpenMPI install,
configure, and build steps. Use $GITHUB_WORKSPACE / ${{ github.workspace }}
so the paths resolve correctly regardless of repo name.

* fix(aocc): source setenv_AOCC.sh from workspace root, not compiler subdir

install.sh places setenv_AOCC.sh in the workspace root, not inside the
aocc-compiler-X.X.X/ directory.

* Fix runExecute.cmake: anchor HDF5 version regex to full token

Replace the over-broad version pattern with one that matches the complete
'HDF5 (x.y.z)' token, preventing over-matching of IP addresses and other
version-like strings, and avoiding nested substitution artifacts in .err
reference comparisons. Applied identically to both copies of the file.
2026-06-30 14:05:00 -05:00
Scot Breitenfeld 7858034164 ci(review-checklist): fix three CODEOWNERS reviewer-avalanche bugs (#6485)
Three distinct but related bugs caused the review-checklist bot to
@ mention more reviewers than intended.

**Bug 1 — bot-sender guard (sticky exclusion)**
The bot's own removeRequestedReviewers API calls fire
review_request_removed events that self-trigger the workflow. Without a
guard, that self-triggered run treated its own bookkeeping removal as a
deliberate human decision and added the login to the persisted
exclusion set permanently. Fix: check sender.type === 'Bot' and skip
the exclusion update for bot-originated removals.

**Bug 2 — cancel-in-progress race on opened/ready_for_review**
GitHub fires one review_requested event per CODEOWNERS auto-assigned
owner; each triggers a workflow run. With cancel-in-progress, the
surviving run may be a review_requested rather than the opened event,
bypassing the avalanche-prune branch entirely and leaving all CODEOWNERS
requested. Fix: track hasExistingComment as a proxy for "first
coordination pass" — if no checklist comment exists yet, prune regardless
of which action survives the race.

**Bug 3 — per-area avalanche on synchronize (PR #6484)**
GitHub's CODEOWNERS engine re-fires when a commit first touches a new
CODEOWNERS-covered area, not only on PR open but also on synchronize.
The surviving run fell through to additive fill, saw the area as
"already has owners, skip", and listed all auto-assigned CODEOWNERS.
Fix: before the synchronize-swap and additive-fill paths, detect any
area whose owner-list ∩ existingRequested > 1 (per-area avalanche) and
prune that area to the single load-balanced pick.

84 tests (was 82).
2026-06-29 11:26:21 -05:00
Scot Breitenfeld 884ce02101 ci: replace actions/stale with bot-aware mark-stale script (#6477)
* ci: replace actions/stale with bot-aware mark-stale script

actions/stale uses updatedAt to measure inactivity, so any bot event
(e.g. the /remove-reviewer acknowledgment comment) resets the stale
countdown even when there has been no meaningful human activity for
months.  PR #6332 was last touched by a human on 2026-04-02 but was
not flagged because a reviewer-removal on 2026-06-11 refreshed the
timestamp.

Replace the actions/stale step with a custom mark-stale.js script
(same pattern as alert-stale.js) that only counts non-bot comments,
non-bot review submissions, and commits as meaningful activity.  The
script also removes the stale label if such activity occurs after the
label was applied.

* ci: fix draft-stale keepalive for external contributors

Two bugs with the keepalive checkbox on draft-stale PRs:

1. External contributors (fork authors) lack write access to edit the
   bot's comment, so clicking the checkbox silently fails for them.
   Fix: also treat a new non-bot comment posted after the keepalive
   comment as a sufficient keepalive signal.

2. The stale label could take up to 24 hours to be removed (daily cron
   only). Fix: add an issue_comment.created trigger so draft-pr-policy
   fires immediately when someone comments on a stale draft PR.
   mark-stale and alert-stale are guarded to only run on
   schedule/workflow_dispatch, not on every comment.

Also fix lastRealActivityAt to exclude bot comments (matching
mark-stale.js), so the keepalive and "Thanks for confirming" bot
comments don't count as real activity when measuring staleness.
2026-06-23 14:03:12 -05:00
Scot BreitenfeldandH. Joe Lee 0f1dfcc08e fix(review-checklist): re-request dismissed reviewer on fixup push (#6479)
* fix(review-checklist): re-request dismissed reviewer on fixup push

When a new commit dismisses a prior reviewer's approval, GitHub's
CODEOWNERS engine auto-assigns a fresh (possibly different) owner for
the changed area. The synchronize handler now detects dismissed
area-owners and swaps them back in, removing the fresh CODEOWNERS pick.

Adds planSynchronizeSwaps() pure helper (exported) with 7 unit tests
covering the PR 6475 scenario and edge cases.

* fix(review-checklist): don't remove a fresh pick still needed by another area

planSynchronizeSwaps could remove a fresh CODEOWNERS pick to restore a
dismissed reviewer even when that pick also covered a different,
unrelated touched area — removeRequestedReviewers strips them from the
whole PR, silently uncovering the other area. Now skips removal when
the candidate owns any other touched area.

Also dedupes the consuming loop so a login needed by two areas isn't
requested/removed twice.

5 new tests covering the cross-area guard and its boundaries.

* fix(review-checklist): don't let bot's own reviewer removal create a sticky exclusion

The bot's own removeUnselected/removeRequestedReviewers calls (draft-opened
CODEOWNERS cleanup, stale-exclusion enforcement) fire review_request_removed,
which the workflow also listens on — self-triggering another run. That run
previously read its own bookkeeping removal as a deliberate human decision
and added the login to the persisted exclusion set, permanently blocking
that owner from ever being auto-assigned to the PR again. Guard on
sender.type !== 'Bot' so only human-driven removals become sticky.

* fix(review-checklist): prune CODEOWNERS avalanche regardless of which event wins the race

GitHub's CODEOWNERS engine fires one review_requested event per auto-assigned
owner on PR creation, and each re-triggers this workflow. With concurrency:
cancel-in-progress, whichever run starts last wins — and that's just as
likely to be one of those review_requested runs as the opened run itself.
A surviving review_requested run fell through to the additive-fill branch,
saw every area already "covered" by the avalanche, and pruned nothing.
This hit PR #6479 itself: all 4 CODEOWNERS for .github/ stayed requested.

Branch on whether a checklist comment exists yet instead of which action
survived — that signal is race-resistant: no comment means this is the PR's
first coordination pass no matter which event got here. Threads
hasExistingComment through from run() (defaulting to true, i.e. additive-fill,
on a comment-fetch failure so an API hiccup can't be mistaken for a fresh PR).

2 new tests: the #6479 race itself, and a contrast case confirming a routine
review_requested on an already-established PR still uses additive-fill.

---------

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-06-23 09:51:37 -05:00
Scot Breitenfeld e251b341d8 CI: base draft PR staleness on real activity, not pr.updated_at (#6472)
pr.updated_at is bumped by metadata-only changes (reviewer
requested/removed, labels, milestone, assignee), letting an
abandoned draft dodge the staleness check indefinitely. Use the
latest commit/comment/review/review-comment timestamp instead.
2026-06-19 17:13:20 -05:00
Scot Breitenfeld 2265cf5bdc CI: prune CODEOWNERS avalanche on non-draft PR open (#6474)
* CI: prune CODEOWNERS avalanche on non-draft PR open

GitHub auto-assigns every touched-area CODEOWNER when a non-draft PR
opens. The previous additive-only logic saw all areas already covered
and returned the full auto-assigned set as confirmedRequested, so the
checklist @-mentioned every owner before the final reviewer set was
known.

On opened/reopened (non-draft), now behave like the draft case: prune
to a single load-balanced pick per area via removeUnselected before
posting the checklist, requesting any pick not already on the PR.

* CI: guard reviewer-exclusion update against bot-triggered removals

The bot's own CODEOWNERS cleanup and draft-handling calls fire
review_request_removed events that self-trigger another workflow run.
Without this guard, that run would interpret the bot's own removal as a
deliberate human decision and permanently add the login to the exclusion
set, blocking that owner from ever being auto-assigned to the PR again.
2026-06-19 17:12:53 -05:00
Scot Breitenfeld 5cce874897 Include pull requests in release progress tracking (#6468)
Add PullRequest inline fragment to the project items GraphQL query
so PRs in the GitHub Project are counted alongside issues when
calculating release blocker/must-do progress.
2026-06-19 12:11:40 -05:00
Scot BreitenfeldandH. Joe Lee daa1daf640 CI: fix reviewer-coordination edge cases — draft transitions, unrelated CODEOWNERS, explicit removals (#6465)
review-checklist.js previously tried to enforce a single load-balanced
reviewer per area by actively stripping anyone else who showed up, using a
checklist-comment-existence heuristic (isOpeningRace) to decide when it was
safe to do so. Walking through real PR timelines surfaced several cases
where that heuristic and that stripping behavior did the wrong thing:

- An old PR's first encounter with the bot (or one whose checklist comment
  was deleted) was misdiagnosed as a brand-new PR, forcibly reselecting a
  human-curated reviewer list.
- A CODEOWNER for areas this PR doesn't touch (e.g. a project lead added by
  hand for their judgment) was swept by the same logic as junk
  auto-assignment, with no protection for an existing approval.
- A reviewer who isn't an owner of any touched area was invisible in the
  checklist and untouched by any cleanup path either way.
- A PR that picked up reviewers while non-draft, then was converted to
  draft, had those reviewers wiped out by the next push — converted_to_draft
  isn't in this workflow's trigger list, so there's no event at the actual
  transition to distinguish "noise from this PR's creation" from "a real
  assignment from before it became a draft."
- A reviewer removed via the PR UI could be silently re-added by GitHub's
  own CODEOWNERS engine on a later push, since there was no memory of the
  removal across runs.

Redesign around one principle: the bot should never automatically strip a
reviewer who's already on the PR, manually added or auto-assigned, except
where the policy explicitly calls for it. Concretely:

- coordinateReviewers is now purely additive for non-draft PRs: chooseReviewers
  runs against the *real* existingRequested, so it naturally skips any area
  that already has someone and only fills gaps. This is idempotent and safe
  on every event, eliminating the need for the old race-detection heuristic
  entirely (isOpeningRace, the new-PR/synchronize distinction, and the
  post-request 15s delayed re-clear are all removed as dead weight).
- Pruning — used only where the bot still clears reviewers — is scoped to
  touchedAreaOwners (owners of areas this PR touches, plus catch-all "*"
  owners) instead of the repo-wide allCodeOwners, so a CODEOWNER for
  unrelated areas is never touched.
- Draft handling clears auto-assigned reviewers only at the literal
  opened/reopened-as-draft moment — the one point where "CODEOWNERS noise
  from this PR's creation" and "this PR's actual reviewer state" are
  mechanically the same thing. Every other event while draft leaves existing
  reviewers alone.
- An explicit review_request_removed persists that login to an exclusion
  list embedded as a second hidden marker in the checklist comment (the only
  durable storage available), and every run strips anyone in that list who
  reappears — covering GitHub re-assigning them on a later push. A direct
  review_requested for that exact login overrides the exclusion, since it's
  the strongest available signal that someone individually decided that
  person should be back right now.
- buildBody adds a catch-all "Additional reviewers" line for anyone
  requested who isn't an owner of any touched area, so they're visible and
  their approval is shown (informationally — it doesn't gate any area's
  sign-off), and accepts any owner's approval for an area's sign-off rather
  than only the specifically-assigned one's.

12 new tests cover the buildBody changes and the parseExcluded/serializeExcluded
round-trip; all existing pure-function tests are unchanged.

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-06-19 12:02:45 -05:00
Scot BreitenfeldandH. Joe Lee a7eeb4fa9b Add stale PR policy with assignee alerts. (#6463)
* Add stale PR/issue policy with assignee alerts instead of auto-close

Ready PRs/issues use actions/stale to label inactivity (30/60 days).
Draft PRs get a longer 90-day window and only reset on an explicit
"still working on this" comment, since pushes/CI activity alone
shouldn't make an abandoned draft look fresh. Nothing is auto-closed:
once a staleness label has persisted past its alert threshold, a
custom script pings the assignee (falling back to requested
reviewers, then the author) to decide whether to keep it open or
close it.

* Scope stale policy to PRs only, not issues

Issue staleness is disabled (days-before-issue-stale: -1) and the
alert script now skips any non-PR item defensively, since this
workflow is meant to address PRs sitting unmerged/unreviewed, not
issue triage.

* Draft stale window 60 days (was 90); drop dead pull_request branch

alert-stale.js's pickAlertTargets is now only ever called with PR
items (filtered upstream in runAlertStale), so the pull_request
check around the requested-reviewers lookup was dead code.

* Set persist-credentials: false on checkout steps

Fixes two zizmor notes: these checkouts only need to read local
script files for github-script's require(), so there's no reason
to persist the GITHUB_TOKEN in git config afterward.

* Use a keep-alive checkbox instead of a magic comment phrase for drafts

Checking a box in the bot's own stale-notice comment is more
discoverable than requiring an exact phrase, and the live checkbox
state can be read straight off that comment's current body on each
run instead of scanning new comments for a regex match.

---------

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-06-19 11:32:43 -05:00
Scot Breitenfeld 2c674737e5 Fix stack buffer overflows in h5repack parse_filter (fixes #6433) (#6434)
* Fix stack buffer overflows in h5repack_parse.c (issue #6433)

Five sites in parse_filter() wrote to fixed 16-byte stack buffers
(scomp[16], stype[16]) via unbounded loop indices with no prior bounds
check, causing a stack-buffer-overflow on malformed -f arguments:

  Site 1: scomp[k]  - filter name token before '='
  Site 2: stype[m]  - SZIP pixels_per_block digit sequence
  Site 3: stype[m]  - SOFF scale_factor digit sequence
  Site 4: stype[q]  - UD legacy per-field digit sequence
  Site 5: stype[m]  - all-other-filters digit sequence

Replace all five bare array writes with a single PARSE_BUF_WRITE macro
that checks the index against sizeof(buf)-1 before writing, then frees
obj_list and exits with an error message on overflow — consistent with
all other error handling in parse_filter().

Confirmed with AddressSanitizer: overflow is eliminated and invalid
over-long arguments produce a clean diagnostic exit instead of UB.

* Fix stack buffer overflows in h5repack_parse.c (issue #6433)

All five overflow sites in parse_filter() are now protected via a
PARSE_BUF_WRITE macro that takes an explicit buf_sz argument (passed as
sizeof(buf) at every call site) to avoid silent pointer-decay if the
buffers are ever refactored to heap allocations.

In the SZIP and SOFF inner loops the previous code wrote bare
`smask[l] = c` with no bounds check, and relied on `i = len - 1` to
stop the *outer* loop without also breaking the *inner* `u` loop.
That left one additional iteration where `l` could equal 2 and
`smask[2]` would be written by the next outer iteration before the
outer loop condition was re-evaluated.  Both sites now use
PARSE_BUF_WRITE and add an explicit `break` to exit the inner loop
as soon as the two-character mask is complete.

The NULL guard around free(obj_list_ptr) is removed: free(NULL) is a
safe no-op per C99 §7.20.3.2.
2026-06-19 09:20:13 -05:00
Scot Breitenfeld 4d3559c5b1 CI: fix buildBody sign-off and add non-CODEOWNER reviewer tests (#6461)
* CI: fix buildBody to accept any owner approval, not only the assigned reviewer

PR #6446 narrowed the approver lookup to effectiveReviewers (the confirmed-
requested subset), inadvertently breaking the case where a non-assigned owner
approves. Restore the original intent: any CODEOWNER's approval signs off an
area; fall back to effectiveReviewers only when no CODEOWNER is assigned.

* text updates

* CI: add tests for non-CODEOWNER reviewer sign-off path (#6446)

Three tests cover the nonOwnerReviewers fallback introduced in #6446:
- pending mention when a non-owner is manually assigned
- non-owner approval signs off an area with no CODEOWNER assigned
- non-owner approval does NOT sign off when a CODEOWNER was assigned

* CI: restore review_requested/review_request_removed triggers

These were dropped in #6453 when ready_for_review was added, breaking
automatic checklist updates on manual reviewer changes.

Both events fall through to the preserve-existing path in
coordinateReviewers when a checklist already exists, so the checklist
body is simply rebuilt with the current requested_reviewers set.

Also extend the opening-race detection (previously called isFirstSyncRace)
to cover review_requested: for any PR, CODEOWNERS auto-assignment fires
review_requested shortly after opened, and with cancel-in-progress: true
that run can cancel the opened run. If no checklist exists yet, treat the
review_requested event as a new-PR event and run full selection.

* CI: show only one reviewer per checklist area

Each area's pending mention now shows the primary (first) effective reviewer
rather than all confirmed-requested owners joined with commas.

* CI: revert to showing all confirmed reviewers per checklist area

A manually added person who is also a CODEOWNER for that area should be
shown alongside the load-balanced pick, not hidden. "One reviewer per area"
only describes chooseReviewers' default selection, not a display constraint.
2026-06-17 11:32:50 -05:00
Scot Breitenfeld ec33e7573c CI: fix reviewer workflows for already-reviewed and draft PRs (#6453)
* CI: skip reviewer removal when they have already submitted a review

GitHub's API rejects removeRequestedReviewers for users who have already
reviewed; detect that case upfront via listReviews and report it clearly
instead of surfacing a cryptic API error.

* CI: defer reviewer assignment for draft PRs until ready for review

Add ready_for_review to the pull_request_target trigger so the workflow
fires when a draft is promoted.  In the script, skip requestReviewers
(and the CODEOWNERS auto-assignment cleanup second pass) while the PR is
a draft; still assign the PR author and strip any reviewers GitHub
auto-assigned.  When the PR is marked ready, treat it like a fresh open
and run the full load-balanced reviewer selection.

* CI: fix synchronize race — treat first synchronize as new PR when opened was cancelled

When a fork PR is created, GitHub fires both opened and synchronize.
With cancel-in-progress: true the synchronize run often wins, and the
opened run (which clears CODEOWNERS auto-assignments) is cancelled before
it completes.  Detect this: if synchronize fires but no checklist comment
exists yet, run the full new-PR path (enforceSelection + load-balanced
reviewer assignment) instead of silently carrying forward all auto-assigned
reviewers.

* CI: enforce reviewer selection before posting @mentions

On synchronize, run enforceSelection against the ideal load-balanced
pick before building the checklist body so the @mentions are never
sent until the reviewer list is actually correct.  Re-fetch the PR
after cleanup so confirmedRequested reflects reality, not the pre-
cleanup snapshot.

For workflow_run (review submitted), reviewer assignment is intentionally
left unchanged but @mentions are filtered to the ideal selection so
CODEOWNERS extras don't generate spurious notifications on comment edits.

* CI: preserve manually added reviewers on synchronize

Reverting the enforceSelection call on the non-first-run synchronize
path.  There is no API way to distinguish CODEOWNERS auto-assignments
from manually added reviewers, so enforcing the load-balanced selection
on every synchronize would silently remove intentional additions.

The initial cleanup (opened or first-synchronize via openedWasSkipped)
already produces a correct reviewer list; subsequent synchronize and
workflow_run events carry it forward unchanged.

* CI: pin actions/github-script to commit hash in remove-reviewer.yml

zizmor requires actions to be pinned to a commit hash rather than a tag.
Use the same pinned hash (ed597411d8f924073f98dfc5c65a23a2325f34cd, v8.0.0)
already used in review-checklist.yml.

* CI: refactor review-checklist.js for readability

Extract the reviewer coordination logic out of the monolithic run()
function into named module-level helpers:

  checklistExists()          — single responsibility: does a checklist
                               comment already exist on the PR?
  removeUnselected()         — remove CODEOWNERS auto-assignments not in
                               the load-balanced selection set
  requestReviewers()         — request each reviewer individually so one
                               bad login cannot block the rest
  removeUnselectedAfterDelay() — 15-second wait + re-fetch + re-enforce
                               for GitHub's async auto-assignment race
  coordinateReviewers()      — top-level dispatcher; the four event paths
                               (read-only, synchronize-normal, new-PR/draft,
                               new-PR/non-draft) are now explicit branches
                               with labelled comments instead of nested ifs

run() is now a straight pipeline of 8 numbered steps with no nested
async functions. Behaviour is unchanged.

* CI: extract convertGlobToRegex and use github.paginate consistently

Extract glob-to-regex conversion into a standalone helper and replace
manual pagination loops for listFiles and listReviews with github.paginate,
matching the existing style used for listComments and pulls.list.

* CI: restrict remove-reviewer workflow to main repo only

Adds a github.repository guard so the job does not run in forks,
preventing unintended resource consumption and command execution
against fork PRs.

* CI: restrict test-maven-packages workflow to main repo only

Adds github.repository guards to all three jobs so the workflow
does not evaluate in forks, preventing spurious "workflow file issue"
failures on push events in forked repositories.

* Revert "CI: restrict test-maven-packages workflow to main repo only"

This reverts commit cc1208226d.
2026-06-16 15:12:33 -05:00
Scot Breitenfeld c30dae6682 CI: fix template injection in workflow_dispatch run: blocks (#6447)
* CI: fix template injection in workflow_dispatch run: blocks

Move workflow_dispatch inputs that appeared directly in run: scripts
into step-level env: blocks, then reference them as plain shell
variables. This is the pattern zizmor recommends and eliminates the
injection vector.

Only workflow_dispatch inputs are fixed here — workflow_call inputs
in reusable files are not attacker-controlled (they come from the
calling workflow) so no changes are needed there.

Files changed:
- publish-branch.yml: local_dir, target_dir in aws s3 sync
- java-implementation-test.yml: java_versions, platforms, test_mode
- maven-staging.yml: test_maven_deployment, java_implementation,
  platforms, use_snapshot_version
- maven-build-test.yml: test_deployment, java_implementation,
  platforms, test_examples
- test-maven-packages.yml: version, repository_url throughout
- test-binary-installation.yml: maven_repository, maven_version,
  install_method

* CI: move remaining GHA expressions from run: blocks to env: vars

* CI: exclude pure reusable workflows from zizmor template-injection scan

Pure reusable workflows (workflow_call only, no workflow_dispatch/pull_request/
push/schedule/release triggers) can never be triggered directly by external
users. Template-injection findings against inputs.* in those files are false
positives — inputs arrive from the trusted calling workflow, not from attackers.

Scanning only files with user-facing triggers keeps Security tab alerts
meaningful and prevents developers from dismissing real findings.
2026-06-15 16:39:26 -05:00
Scot Breitenfeld 41ea347fb9 review-checklist: show non-CODEOWNER reviewer when no area owner is assigned (#6446)
* review-checklist: show non-CODEOWNER reviewer when no owner is assigned

If the only assigned CODEOWNER is removed and a non-CODEOWNER is manually
added in their place, that person previously got no checklist mention and
their approval did not check the box.

For any area with no CODEOWNER in the requested set, fall back to
non-CODEOWNER reviewers (anyone assigned who is not an owner of any
touched area). They are shown in the mention and their approval counts
as sign-off for that area.

* review-checklist: update checklist on reviewer add/remove

Adding or removing a reviewer did not trigger a workflow run, leaving
the checklist comment stale until the next push.

Add review_requested and review_request_removed to the pull_request_target
activity types so the checklist updates immediately when a reviewer is
manually added or removed.
2026-06-12 14:21:53 -05:00
Scot Breitenfeld 65fc219bc9 review-checklist: enforce one load-balanced reviewer and fix checklist display (#6439)
* review-checklist: show all requested owners per area in checklist

buildBody was using area.owners.find() — picking the first CODEOWNERS-listed
owner in the requested set. This broke reviewer swaps (removing @A and adding
@B would show @C, the next CODEOWNERS entry, not @B) and also failed to
reflect GitHub's CODEOWNERS auto-assignment, which requests all owners.

Switch to area.owners.filter() so every requested owner for an area is
mentioned in that row. Approval logic is unchanged: any owner approval
still checks the box.

* review-checklist: enforce one load-balanced reviewer per area

GitHub's CODEOWNERS auto-assignment requests all owners of touched files
when a PR opens, before the workflow runs. The script was then seeing them
already assigned and skipping its own selection entirely.

On opened/reopened: select one load-balanced reviewer per area (ignoring
GitHub's pre-assigned set), remove any auto-assigned CODEOWNERS not in the
selection, then add the chosen reviewer. Only code owners are removed —
manually-added non-owner reviewers are left untouched.

On synchronize: keep existing assignments (reviewer may have already started).

confirmedRequested now starts empty and is populated only with the script's
selection, so the checklist only mentions owners that were explicitly chosen.

* review-checklist: retry reviewer cleanup to handle GitHub auto-assign race

GitHub's CODEOWNERS auto-assignment can fire after the workflow starts,
re-adding extra reviewers after we remove them. Add a 15-second wait
followed by a second cleanup pass on opened/reopened events.

Extract the removal loop into enforceSelection() so both passes share
the same logic.

* zizmor: suppress template-injection for PR #6356 workflow files

Add zizmor_config.yml to ignore template-injection findings in the
setup-jextract action and maven/ctest workflow files introduced in
PR #6356, where inputs are caller-controlled but not user-controlled.

* review-checklist: don't re-assign reviewers on synchronize

On synchronize, chooseReviewers saw no owner assigned (because the
reviewer was manually removed) and re-added one via requestReviewers,
overriding the manual removal.

Reviewer assignment now only happens on opened/reopened. Synchronize
only updates the checklist display based on whoever is currently
assigned — manual removals are respected.

* zizmor: skip upload-sarif failure on fork PRs

* zizmor: move config out of workflows dir to fix GitHub Actions parse error

GitHub Actions parses all .yml files in .github/workflows/ as workflow
files; zizmor_config.yml there caused "unexpected value 'rules'" because
rules: is not valid workflow syntax. Moved to .github/zizmor.yml and
updated the --config path in zizmor.yml accordingly.

* review-checklist: retry on transient 401 from GitHub API

GitHub's API intermittently returns 401 on write operations
(issues.addAssignees, issues.createComment) even when the token has
Issues: write and PullRequests: write — read-only calls succeed in the
same run. The github-script action excludes 401 from retries by default.
Removing 401 from retry-exempt-status-codes and setting retries: 3
handles these transient failures with exponential backoff.

* review-checklist: fix checklist mention when non-requested owner approves

The mention in each checklist row was derived from the approver (if any),
so when a different area owner happened to approve first, the mention
changed from the assigned reviewer to the approver. Fix by decoupling
sign-off detection from display: signedOff now uses .some() so any
owner's approval checks the box, while the mention always shows the
confirmed-requested reviewer(s) via .filter(). Also shows multiple
reviewers when one is manually added alongside the load-balanced
selection.

* review-checklist: show approver name when signed off, requested reviewer(s) when pending

When an area is signed off, replace the mention with the approver so the
checklist shows who actually reviewed it (which may differ from whoever
was load-balanced as the requested reviewer). When pending, show all
confirmed-requested reviewers — the one load-balanced pick normally, but
two if a reviewer was manually added alongside it.

* zizmor: remove config file and --config flag

The ignored files (ctest.yml, maven-deploy.yml, maven-staging.yml,
setup-jextract/action.yml) had template-injection findings on
inputs.* references, which are not attacker-controllable. Rather than
suppressing them via a config file, let all findings surface in the
Security tab and address them individually if needed.
2026-06-11 15:12:10 -05:00
Scot BreitenfeldandH. Joe Lee 64036c6d03 CI: use pull_request_target so review-checklist runs on fork PRs (#6435)
* CI: use pull_request_target so review-checklist runs on fork PRs

pull_request events from forks receive a read-only GITHUB_TOKEN and
are skipped by the job condition, so no checklist comment is posted
and no reviewers are assigned.  Switching to pull_request_target fixes
this: GitHub always executes the workflow from the base repo
(HDFGroup/hdf5) with a full write token, regardless of whether the PR
comes from a fork.  The fork's code is never checked out or executed.

The cross-repo guard (head.repo.full_name == github.repository) is
removed since it is no longer needed — pull_request_target only fires
for PRs targeting this repo's branches.

* CI: replace pull_request_target with safe two-workflow pattern for fork PRs

pull_request_target carries a GitHub security warning because it grants
write access to the privileged workflow at trigger time, which is risky
if the checkout is ever changed to use the fork's head ref.

Replace it with the recommended two-workflow pattern:
- review-checklist-gather.yml fires on pull_request (read-only, safe
  for forks) and uploads the PR number as a short-lived artifact.
- review-checklist.yml fires on workflow_run when the gather job
  completes, downloads the artifact for the PR number, then posts the
  checklist comment and assigns reviewers with a full write token.
  It never executes any code from the fork.

The script gains a prNumber parameter so the main workflow can supply
the PR number it read from the artifact; falls back to the event
payload for pull_request_review triggers.

* review-checklist: fix prAuthor crash and stale comment for workflow_run context

In the workflow_run path context.payload.pull_request is undefined, so
reading .user.login from it throws a TypeError. Use prData.user.login
instead — prData is already fetched from the API at step 5.

Also remove the stale comment saying fork PRs require pull_request_target;
they are now handled via the two-workflow pattern.

* CI: revert to pull_request_target, drop two-workflow pattern

The workflow_run approach adds a second hop before the checklist posts
and introduced a context.payload.pull_request crash in the gather path.
pull_request_target is simpler and equally safe here: GitHub always
executes the workflow from HDFGroup/hdf5:develop with a full write
token — the fork's code is never checked out or executed.

* CI: add zizmor workflow for GitHub Actions security analysis

zizmor is a static analyser that catches security issues specific to
GitHub Actions: pull_request_target misuse, expression injection,
unpinned action SHAs, and overly broad permissions.

Runs on push to develop and on PRs that touch .github/, uploads SARIF
to the Security tab (free for public repos via Advanced Security), and
annotates PR diffs inline with any findings.

* CI: run zizmor via pip instead of third-party action

Replace zizmorcore/zizmor-action (third-party) with a direct pip install
of zizmor==1.25.2. SARIF output is still uploaded to the Security tab via
github/codeql-action/upload-sarif (first-party). The only actions used
are from actions/ and github/, both trusted.

* CI: fix codeql-action pin to commit SHA (was tag object SHA)

* CI: add persist-credentials: false to review-checklist checkout

* CI: hash-pin zizmor pip install; suppress expected pull-request-target finding

- zizmor.yml: pin zizmor wheel to its SHA256 hash via --require-hashes so
  the install is fully reproducible and passes zizmor's unpinned-tools audit
- review-checklist.yml: add zizmor: ignore[pull-request-target] comment so
  zizmor does not flag its own host workflow; document why the usage is safe

* CI: update actions to Node.js 24 ahead of June 16 deprecation

- actions/checkout:          v4.2.2 → v5.0.1  (node20 → node24)
- actions/github-script:     v7.0.1 → v8.0.0  (node20 → node24)
- github/codeql-action:      v3.36.2 → v4.36.2 (node20 → node24; v3 deprecated Dec 2026)

* CI: fix pip hash-pinning syntax — hash must be in requirements file

* CI: zizmor step continue-on-error so SARIF upload always runs

* review-checklist: skip pull_request_review on fork PRs (read-only token)

GitHub only grants a read-only GITHUB_TOKEN for pull_request_review events
when the PR originates from a fork, so the comment post fails with 403.
Restrict the review trigger to same-repo PRs; pull_request_target already
handles fork PRs for the open/sync/reopen case with a full write token.

* review-checklist: route reviews through workflow_run for fork PR support

pull_request_review grants only a read-only token for fork PRs, causing
a 403 when posting the checklist comment. Fix with a two-path design:

- pull_request_target handles open/sync/reopen for all PRs (full token,
  immediate)
- A new gather workflow (review-checklist-gather.yml) fires on
  pull_request_review, saves the PR number as an artifact, and exits.
  The main workflow then fires via workflow_run with a full write token.

The script gains prNumber and isReview parameters. isReview suppresses
reviewer assignment (which only applies on open/sync/reopen). prAuthor
is sourced from prData.user.login since context.payload.pull_request is
not populated in the workflow_run context.

* review-checklist: simplify to pull_request_target only, drop gather workflow

pull_request_review triggers a fork workflow approval gate for first-time
contributors, which blocks every PR. There is no pull_request_review_target
equivalent in GitHub Actions.

Instead, rely on pull_request_target (opened/synchronize/reopened) only.
Approval boxes are still evaluated correctly: computeApprovals() re-reads
all existing reviews on every push, so boxes auto-check the next time the
author pushes after a reviewer approves.

Also restores the simpler script signature (no prNumber/isReview params)
and sources prAuthor from prData.user.login which works in all contexts.

* review-checklist: auto-check approval boxes on review via workflow_run

Add a minimal gather workflow (review-checklist-gather.yml) triggered by
pull_request_review that completes immediately with a read-only token. The
main workflow now also triggers via workflow_run on gather completion, giving
it the full write token it needs to update the checklist comment.

In workflow_run context, the script looks up the PR by head SHA (since
workflow_run.pull_requests is empty for fork PRs) and skips reviewer
assignment — only approval box state is updated.

Requires the repo "Fork pull request workflows from outside collaborators"
setting to be "Require approval for first-time contributors", which is
already the current setting.

---------

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
2026-06-09 12:48:51 -05:00
Scot Breitenfeld f3e380ce17 Add per-area review checklist action and restructure CODEOWNERS (#6418)
* Add per-area review checklist action and restructure CODEOWNERS

CODEOWNERS:
- Replace 11-person global catch-all with specific path rules per area,
  assigning reviewers based on their actual strengths
- Global fallback is now @fortnern only for uncovered root files
- Remove @derobins, @epourmal, @qkoziol, @mkitti per team discussion

review-checklist GitHub Action (.github/workflows/review-checklist.yml):
- Posts a per-area sign-off checklist on every PR to develop (non-forks only)
- Reviewer lists and path patterns derived directly from CODEOWNERS
- Assigns ONE reviewer per area using fewest-open-PRs load balancing
- Complex changes (≥ 300 lines or any public/developer header modified)
  always go to the first (senior) owner listed; routine changes are
  load-balanced across all owners (500-line threshold for test/)
- Cohesion: reuses an already-assigned reviewer for related areas where
  owner lists overlap, avoiding e.g. src/ and test/ going to different people
- Skips auto-assign if an area owner is already manually requested
- Checklist auto-checks when an owner approves; tracks latest review state
  so a subsequent "request changes" unchecks the box

The previous regex only matched *public.h and *develop.h, missing hdf5.h
itself (the umbrella header), all VFD driver headers included by hdf5.h
(H5FDcore.h, H5FDmpio.h, H5FDsubfiling.h, etc.), and VOL connector headers
(H5VLconnector.h, H5VLnative.h, etc.).  Changes to any of these now
correctly trigger senior-owner assignment.
2026-06-04 16:21:53 -05:00
Scot Breitenfeld a3892b08aa ☀️ Fix memory safety vulnerabilities in high-level and VFD code (#6140)
* Fix memory safety vulnerabilities in high-level and VFD code

H5FDstdio (src/H5FDstdio.c):
- Fix five error paths in H5FD_stdio_open() that called fclose(f) after
  free(file): the correct resource to close is file->fp.  Reorder to
  fclose before free to match standard cleanup idiom and prevent file
  descriptor leaks under memory-pressure failures.

H5VLnative (src/H5VLnative.c):
- Add assert(obj) and assert(file) to H5VL_native_get_file_struct() to
  catch NULL-pointer programming errors early in debug builds.

H5LT (hl/src/H5LT.c):
- Add NULL check after strdup() in H5LTtext_to_dtype(); push H5E_NOSPACE
  so the HDF5 error stack is populated on OOM.
- Fix H5Tclose(super) leak in H5T_ENUM, H5T_VLEN, H5T_ARRAY, H5T_COMPLEX
  branches of H5LT_dtype_to_text(): super was only closed on the success
  path; any failure between H5Tget_super and the final realloc_and_append
  leaked the type ID.  Super is now closed immediately after use.
- Refactor the repeated "get super-type text and append" pattern (four
  near-identical ~15-line blocks) into static helper append_dtype_super_text().
  Pushes H5E_NOSPACE on internal calloc failure.
- Rewrite realloc_and_append() doc comment to document the asymmetric
  ownership contract (callee frees buf on realloc failure in
  library-managed mode; no free in user-buf mode).
- Move the buf == NULL guard to before the _no_user_buf branch so both
  modes short-circuit identically.

H5TB (hl/src/H5TB.c):
- Clarify H5TBget_field_info() else-branch comment: the two-branch copy
  structure is an efficiency optimization (copy name_len+1 bytes rather
  than HLTB_MAX_FIELD_LEN-1), not a backward-compatibility concern.

CHANGELOG (release_docs/CHANGELOG.md):
- Add entries for the stdio VFD leak fix, VOL NULL checks, and H5LT
  memory-safety improvements.

* HL: add realloc_and_append invariant comments per fortnern review

Document the failure-path contract for realloc_and_append: buf is
passed by value so the caller's pointer is never written by the
function; failure is signaled solely through a NULL return; in
library-managed mode the underlying memory is freed on failure so
callers must not access the original pointer afterward.

Also annotate the point after a successful realloc where the
function can no longer fail.



* HL: reposition 'cannot fail' comment before buf assignment

The comment should mark the transition out of the failure zone —
after the only exit path (goto out), before buf = tmp_realloc.
Also drop the redundant else since the if body always exits.

* HL: fix nested comment syntax error in realloc_and_append Note
2026-06-04 13:35:06 -05:00
Scot Breitenfeld a957dd480d Improve HDF5_BUILD_PARALLEL_TOOLS docs and MFU not-found message (#6428)
* cmake: improve HDF5_BUILD_PARALLEL_TOOLS documentation and MFU error message

The option description for HDF5_BUILD_PARALLEL_TOOLS was too terse to be
useful — it did not mention the required MFU, CIRCLE, or DTCMP external
libraries, nor that HDF5_ENABLE_PARALLEL must also be ON. Expand it with
the dependency list and a link to the mpiFileUtils project.

- CMakeBuildOptions.cmake: rephrase HDF5_BUILD_PARALLEL_TOOLS description
  to "Build MPI-enabled HDF5 tools" (shorter, forward-compatible)
2026-06-03 14:15:44 -05:00
Scot Breitenfeld e4c16e9fca Fortran: simplify H5config_f.inc.cmake to use #cmakedefine directly (#6423)
All #cmakedefine01 CMAKE_H5_* blocks used a five-line pattern:

  #cmakedefine01 CMAKE_H5_HAVE_FOO
  #if CMAKE_H5_HAVE_FOO == 0
  #undef H5_HAVE_FOO
  #else
  #define H5_HAVE_FOO
  #endif

This is exactly what #cmakedefine H5_HAVE_FOO does: it emits
#define H5_HAVE_FOO (no value) when the CMake variable is truthy and
/* #undef H5_HAVE_FOO */ when falsy.  The CMAKE_H5_* intermediate
variables in fortran/src/CMakeLists.txt were only needed to feed these
blocks and are no longer required.

Replace all such blocks with #cmakedefine H5_HAVE_FOO, using the H5_*
variable directly.  MPI_LOGICAL_KIND retains its value via
#cmakedefine H5_MPI_LOGICAL_KIND @H5_MPI_LOGICAL_KIND@.

This also fixes a real bug: H5_FORTRAN_C_BOOL_IS_UNIQUE was emitted as
#define H5_FORTRAN_C_BOOL_IS_UNIQUE 0 when C_BOOL and default LOGICAL
are the same kind (e.g. Apple PowerPC ABI).  #ifdef only tests whether
a macro is defined, not its value, so the guard in H5_test_buildiface.F90
was always true and verify_c_bool was written into tf_gen.F90 regardless,
causing an "Ambiguous interfaces" build failure on that platform.
2026-06-02 08:13:50 -05:00
Scot Breitenfeld 853451a3a7 HL: Fix prefix-match bugs in H5TB field lookup and H5DS/H5IM CLASS checks (#6371)
- H5TB: strcmp replaces strncmp in H5TBfind_field so that field names
  that are a prefix of a requested name (or vice-versa) are no longer
  matched. HLTB_MAX_FIELD_LEN (255) is now public in H5TBpublic.h and
  exposed to Fortran as HLTB_MAX_FIELD_LEN_F in H5TBff.F90.
  H5TBget_field_info documents the buffer-size requirement and truncation
  behaviour. H5TBget_field_info guards against overflow on long names.

- H5IM: H5IMis_image and H5IMis_palette refactored into a shared helper
  (H5IM__class_attr_equals). The helper now reads both fixed-length and
  variable-length CLASS string attributes, using H5Treclaim for VL memory.
  strcmp replaces strncmp for exact-match semantics.

- H5DS: H5DSis_scale and H5DS_is_reserved both support variable-length
  CLASS string attributes via H5Aget_space/H5Aread/H5Treclaim. The
  fixed-length path retains the 16-byte size guard. strcmp is used
  throughout for exact comparison.

- Tests: new test functions test_is_scale_class_prefix,
  test_is_reserved_class_prefix, and test_class_prefix cover fixed-length
  prefix/exact/wrong-value cases and variable-length string cases.
  test_table.c adds write and read field-name prefix rejection cases and
  boundary-length truncation verification. All malloc calls are NULL-checked.

- CHANGELOG updated with a summary of all fixes.
2026-05-22 11:28:24 -05:00
Scot Breitenfeld 013e9cc90c config/HDF5UseZLIB.cmake: resolve ALIASED_TARGET before re-aliasing (#6408)
Before creating the namespaced `hdf5::<name>` alias, query
ALIASED_TARGET so we point the new alias at the real target.  When
the target is not an alias the behaviour is unchanged.

config/HDF5Use{ZLIB,Libaec}.cmake: resolve ALIASED_TARGET before re-aliasing
2026-05-19 13:53:53 -05:00
Scot Breitenfeld 96750d6eda CI: remove redundant macOS brew installs for ninja and curl (#6406)
Both ninja and curl are pre-installed on the GitHub macOS runners,
causing noisy "already installed" warnings. Drop those brew steps
entirely (or just remove ninja/curl where other packages like graphviz
or libaec are still needed).
2026-05-14 10:04:12 -05:00
Scot Breitenfeld b33b9bff3f ci: skip signed-plugins workflow for doc-only changes (#6398) 2026-05-06 17:05:45 -05:00
86bdc78365 ✨[Feature] Digital Signature Verification for HDF5 Plugins (#6198)
feat: add optional digital signature verification for HDF5 filter plugins

Introduce an opt-in plugin signing and verification system that allows
HDF5 deployments to require cryptographically signed filter plugins before
loading them. Disabled by default (HDF5_REQUIRE_SIGNED_PLUGINS=OFF).

New tool: h5sign
- Signs plugin shared libraries by appending an RSA signature and a
  14-byte footer (algo_id | sig_len | 8-byte magic | format_ver) to the
  binary without modifying the original content.
- Supports SHA-512 (default), SHA-256, SHA-384, and their PSS variants
  (-a/--algorithm flag).
- Detects already-signed plugins; --force strips the old signature and
  re-signs.
- Security hardened: keeps the file descriptor open through hashing and
  appending (no TOCTOU window), enforces a 2048-bit minimum RSA key size,
  rolls back partial writes on failure, and rejects paths that are not
  regular files.

Verification (H5PLsig.c)
- At plugin load time, reads the footer, validates the magic and format
  version, then checks the RSA signature against all public keys found in
  the KeyStore directory.
- File is hashed once; per-key verification operates on the pre-computed
  digest (no redundant I/O for multi-key keystores).
- Plugins whose signature hash appears in revoked_signatures.txt are
  rejected regardless of key validity.
- Runtime debug output via HDF5_DEBUG=pl.

KeyStore management
- Trusted public keys are PEM files in a directory specified by
  HDF5_PLUGIN_KEYSTORE_DIR (build time) or HDF5_PLUGIN_KEYSTORE (env var).
- HDF5_LOCK_PLUGIN_KEYSTORE cmake option disables the env-var override for
  security-hardened deployments.

Test infrastructure
- h5signverifytest: positive, negative, tamper, re-sign, and revocation
  test cases.
- CTest fixture-based dependency graph (FIXTURES_SETUP/FIXTURES_REQUIRED)
  replaces fragile DEPENDS chains so tests remain correct under -R filtering.
- Dedicated signed-plugins.yml CI workflow; full test suite scoped to
  H5SIGN and H5PLUGIN-signature tests to avoid unrelated flaky failures.
- Cross-platform: Linux, macOS, and Windows (MSVC-compatible, BIO-based
  OpenSSL I/O, HDsleep/HDsetenv portability wrappers).

Documentation: docs/PLUGIN_SIGNATURE_README.md covers usage, footer
format, revocation file format, FAQ, and troubleshooting.

Co-authored-by: Glenn Song <gsong@hdfgroup.org>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
2026-05-06 09:35:19 -05:00
Scot Breitenfeld 4fbe22a896 ci: remove EOL OS versions from workflows (#6388)
* CI: remove EOL OS versions from workflows

- freebsd.yml: drop FreeBSD 13.5 (EOL Jan 2026); keep 14.3 and 15.0
- openbsd.yml: bump 7.5 → 7.8 (7.5 EOL ~Nov 2024); drop pinned gcc version
- i386.yml: replace Alpine 3.16 x86 (EOL May 2024) with i386/debian:bookworm (EOL Jun 2028)

* CI: install cmake from bookworm-backports for i386 workflow
* CI: fix OpenBSD workflow for 7.8
- Update vmactions/openbsd-vm v1.3.4 → v1.4.0 (supports 7.8)
- Drop GCC; use OpenBSD's built-in Clang (cc/c++) which is always in PATH
- Remove LD_LIBRARY_PATH workaround that was only needed for egcc

* CI: remove gmake from OpenBSD workflow
2026-05-01 16:00:02 -05:00
Scot Breitenfeld f858615b70 Revert "Pin BLOSC2 git fetch to versioned tag (#6380)" (#6384)
This reverts commit c478c6e35f.
2026-04-28 13:09:45 -05:00
Scot Breitenfeld c478c6e35f Pin BLOSC2 git fetch to versioned tag (#6380)
BLOSC2_GIT_BRANCH was "main" causing the inline plugin build to fetch
unreleased c-blosc2 that breaks test output comparisons. Pin to the
tagged release matching HDF5_BLOSC2_VERSION so the git and tgz paths
are consistent.
2026-04-26 04:06:26 -04:00
Scot Breitenfeld cbff0315f3 Default versioned API functions to earliest version for older API settings (#6280)
When a global API version is set (e.g., H5_USE_16_API), functions
introduced after that version now default to their earliest version
(version 1) instead of the latest. This prevents breakage when an
application uses an older API setting but calls functions that were
later versioned.
2026-04-07 11:06:32 -05:00
Scot Breitenfeld 00499095d6 ci: only upload empty SARIF for pull_request events (#6353)
Restrict empty SARIF upload to pull_request events only, so that
push-to-develop (e.g. after merging a text-only PR) does not overwrite
the real CodeQL results in the Security tab with an empty SARIF.
2026-04-07 07:51:12 -05:00
Scot Breitenfeld 970efad7e1 ci: upload empty SARIF when CodeQL analysis is skipped (#6351)
so that the "Require code scanning results" branch
protection rule is satisfied for text-only PRs.
2026-04-06 12:37:20 -05:00
Scot Breitenfeld a8e1719655 ci: add gate job to CodeQL workflow for text-only PRs (#6346)
* ci: add gate job to CodeQL workflow for text-only PRs

Remove paths-ignore from the workflow trigger and add a check-changes
job with dorny/paths-filter to detect code changes at the job level.
This ensures the workflow always triggers so the codeql-complete gate
job can report a passing status when analyze is skipped, preventing
text-only PRs from being blocked by required status checks.

* ci: check both check-changes and analyze results in gate job

Add check-changes to the needs array of codeql-complete so that a
failure in the change-detection job is not silently treated as a
skipped analysis.
2026-04-04 10:39:46 -06:00
Scot Breitenfeld 05676d1abe Consolidate documentation under docs/ directory (#6310)
* Consolidate documentation under doc/ directory

Move user-facing guides from release_docs/ and doxygen/ into a single
doc/ root. release_docs/ now holds only release artifacts (changelogs,
history, release process, maintainer info).

- git mv release_docs/INSTALL*.md, USING_*.md, README_HPC.md,
  BuildSystemNotes.md, AutotoolsToCMakeOptions.md,
  HDF5_Library_2.0.0_Migration_Guide.md → doc/
- git mv doxygen/ → doc/doxygen/
- Update CMakeLists.txt: HDF5_DOXYGEN_DIR and add_subdirectory path
- Update CMakeInstallation.cmake: all install paths for moved files
- Update bin/make_vers: hardcoded doxygen/ path substitution
- Update doc/doxygen/CMakeLists.txt: EXAMPLES_DIRECTORY and comments
- Update README.md, CONTRIBUTING.md, SECURITY.md, config/README.md,
  release_docs/RELEASE_PROCESS.md: links to moved files
- Update doxygen .dox files: release_docs/ URLs for moved guides
- Rewrite release_docs/README.md for narrowed scope

* Add HDF5_DOCS_DIR variable for doc/ root path

Introduce HDF5_DOCS_DIR = \${HDF5_SOURCE_DIR}/doc so that
CMakeInstallation.cmake and future callers reference the doc/
directory symbolically rather than by hardcoded path.
HDF5_DOXYGEN_DIR is now derived from HDF5_DOCS_DIR.
2026-03-31 17:01:54 -06:00
Scot Breitenfeld 05737ece7f H5DSget_label_f returned size fixed (#6327)
* fixed assignment of size in the wrapper

* Call H5DSget_label directly from Fortran wrapper

Replace the intermediate C wrapper h5dsget_label_c with a direct
bind(c) call to H5DSget_label from H5DSget_label_f. This eliminates
the malloc/free of a temporary buffer and the associated failure path
where size was incorrectly set when H5DSget_label failed. The Fortran
wrapper now handles the C-to-Fortran string conversion (equivalent to
HD5packFstring) by blank-padding the buffer from the returned label
length to the end.

* Remove unused h5dsget_label_c C wrapper
2026-03-30 15:45:25 -05:00
Scot Breitenfeld 67c2b782b9 Fix unbounded writes in h5repack_opttable.c: (#6303)
Replace all strcpy calls copying into fixed-size path buffers with snprintf to prevent buffer overflows. Also simplify the path prefix check and the empty-string initialization.

Fixes 6 critical CodeQL cpp/unbounded-write alerts.
2026-03-20 12:55:42 -05:00
Scot Breitenfeld 05758a6168 Security hardening (#6302)
Addresses CodeQL issues (Critical and High).
2026-03-18 22:32:03 -05:00
Scot Breitenfeld 262802ec36 Outlines security policy (#6199)
* Updated SECURITY.md to meet OSSF Badge Standard
* Disable CodeQL on MD file changes
* Updated version support policy
* Suppress non-constant format string CodeQL alerts

Exclude cpp/non-constant-format and cpp/uncontrolled-format-string
rules. All flagged uses are intentional format-string-as-template
patterns for family file naming (%d) and configurable output
formatting (%g), not security vulnerabilities.
* Add justification comments for CodeQL exclusions
* Harden CI workflows: pin actions to SHAs and apply least-privilege permissions
- Pin all third-party actions to commit SHAs in codeql.yml and arm-main.yml
- Add top-level permissions: contents: read to codeql.yml and markdown-link-check.yml
- Move contents: write from top-level to job-level in daily-schedule.yml

* Exclude non-security CodeQL rules and tainted-format-string false positives
- Add cpp/tainted-format-string to exclusions (same justification as
  existing format string rules: bounded snprintf with app-controlled formats)
- Exclude code style rules with no security severity: cpp/long-switch,
  cpp/guarded-free, cpp/commented-out-code, cpp/use-of-goto
- These 350+ alerts are structural C patterns inherent to HDF5, not vulnerabilities

* Add OpenSSF Best Practices badge to README.md
2026-03-17 11:42:07 -05:00
Scot Breitenfeld 71763e74e7 Skip building Doxygen when no documentation inputs have changed (#6245)
Optimize Doxygen build process by skipping rebuilds when input files are unchanged, using timestamp checks in CMake configuration.

Behavior:
Skip Doxygen build if input files unchanged using timestamp check in CMakeLists.txt, hl/src/CMakeLists.txt, and src/CMakeLists.txt.
Introduces stamp files (hdf5lib_doc.stamp, hdf5hllib_doc.stamp) to track changes.
Documentation:
add_custom_command and add_custom_target used to manage Doxygen builds.
Shared Doxygen content inputs defined in top-level CMakeLists.txt for reuse.
Misc:
Reorder add_subdirectory(src) in CMakeLists.txt for visibility of Doxygen variables.
2026-03-10 06:56:39 -05:00
Scot Breitenfeld 6e7fec1cc2 Update release progress badges to use Priority-based categorization (#6270)
* Update release progress badges to use Priority-based categorization

- Replace "Release Blockers"/"Release Must Do" with "Critical Priority"/"High Priority"
- Switch issue filtering from Release gating field to Priority field (P0 - Critical/P1 - High)
- Remove "Nice to Have" badge from workflow and README
- Add failure step: on workflow error, update both badge gist files to bright
  purple (BF00FF) with "FAILURE" message
- Version auto-detected from src/H5public.h for milestone filtering

* Fix stale class docstring: release blocker -> priority issue
2026-03-04 16:11:52 -06:00
Scot Breitenfeld 322b30b6e8 Fix POSIX header guard to include _WIN32 for Intel icx on Windows (#6244)
Intel's icx compiler defines _WIN32 (with underscore) but not WIN32,
causing unistd.h and other POSIX headers to be incorrectly included
on Windows builds.
2026-03-01 07:19:05 -06:00
Scot Breitenfeld ceb978cdcd workflow will no longer post that maven comment to PRs (#6238) 2026-02-26 11:07:18 -06:00
Scot Breitenfeld a7ec64a857 Split static targets into separate optional target [UPDATED] (#6216)
Build-tree exports can't diverge from install-tree exports — the export(EXPORT ...) reads directly from the install export sets. No manual list to keep in sync.

Removed 3 global variables (HDF5_STATIC_LIBRARIES_TO_EXPORT, HDF5_JAVA_LIBRARIES_TO_EXPORT, HDF5_UTILS_TO_EXPORT) and their ~21 set_global_variable calls across tool/utility files.

Fixed the static-only build bug in the PR where the base export set was guarded by BUILD_SHARED_LIBS, breaking tools export.

Removed redundant utils in export files — the PR was dumping tools into all three build-tree export files (java, static, shared). Now they correctly appear only in the base export.
2026-02-24 17:04:25 -06:00
Scot Breitenfeld 49ae2f225a Updates to #6205 (#6208)
* Fix MPI Fortran configuration for HDF5Examples (#6205)

When building Fortran parallel examples standalone without MPI
wrappers, the compiler cannot find mpi.mod because
MPI_Fortran_INCLUDE_DIRS is not populated.

* Use MPI::MPI_Fortran imported target instead of raw MPI variables

Replace manual MPI_Fortran_LIBRARIES, MPI_Fortran_LINK_FLAGS, and
MPI_Fortran_INCLUDE_DIRS usage with the MPI::MPI_Fortran imported
target, which transitively carries all include dirs, compile flags,
and link flags. This eliminates the CMAKE_Fortran_EXE_LINKER_FLAGS
hack and the manual target_include_directories in H5PAR.
2026-02-12 14:10:39 -06:00
Scot Breitenfeld a46a399230 Fixes the "multiple @param documentation sections" doxygen warnings (#6195)
Addresses the "multiple @ param documentation sections" warnings are caused by the interaction of three things:

1. MACRO_EXPANSION = YES with EXPAND_ONLY_PREDEF = NO in Doxyfile.in

2. Versioned API macros in H5version.h like:

\def documentation in APIVersions.dox like:

3. \def H5Tarray_create \api_vers_2{H5Tarray_create,H5Tarray_create1,H5Tarray_create2}

Because Doxygen expands all macros, it sees H5Tarray_create as an alias for H5Tarray_create1. It then associates documentation from both the \def block (for the macro) and the function's own doc block (with \param entries) with the same function — resulting in "multiple @ param documentation sections."

* Update Doxygen version from 1.16.0 to 1.16.1

Use Doxygen 1.16.1 for all platforms. This version includes the latest
bug fixes and improvements while maintaining compatibility with our
documentation format.

* Fix Windows Doxygen installation for version 1.16.1

Handle different Windows binary naming conventions across Doxygen versions:
- 1.16.1 uses: doxygen-VERSION.windows.x64.bin.zip
- 1.16.0 uses: doxygen-VERSION.x64.bin.zip

Try the newer naming convention first, then fall back to the older one
if the download fails. This ensures compatibility across versions.

* Use ssciwr/doxygen-install in setup-doxygen action

Replace direct installation of official Doxygen binaries with
ssciwr/doxygen-install action. This avoids Java documentation parsing
errors that occur with official binaries but not with distribution
builds.

The setup-doxygen action now serves as a thin wrapper around
ssciwr/doxygen-install@f13be16c83 (v1),
using commit hash for security and reproducibility.

* Remove redundant version specification from workflows

The setup-doxygen action already has a default version (1.16.1),
so explicit version specification in workflows is unnecessary.
This follows DRY principles and makes version updates simpler.
2026-02-12 10:45:17 -06:00
Scot Breitenfeld b8b246e14a Fix MPI Fortran configuration for HDF5Examples (#6206)
When building Fortran parallel examples without MPI wrappers, the
compiler couldn't find mpi.mod because MPI_Fortran_INCLUDE_DIRS
was not populated. This occurred because find_package(MPI) was only
called for C, not Fortran.

Added enable_language(Fortran) and find_package(MPI REQUIRED
COMPONENTS Fortran) to properly configure MPI for Fortran examples,
ensuring MPI include directories and libraries are correctly set.
2026-02-06 10:06:35 -06:00
Scot Breitenfeld 5240b804aa Fixes issues caused by #6147 (#6172)
Scope Safety: Refactored parse_hdf5_version from a Macro to a Function to prevent variable namespace pollution.
Implemented a helper macro (_parse_version_component) to handle repetitive regex logic, significantly reducing code size and complexity.
Updated HDF5config.cmake to calculate paths dynamically using REALPATH relative to CMAKE_CURRENT_LIST_FILE, fixing issues with symlinks and execution context.
Decoupled the parsing logic from scope promotion; the helper now parses to a local variable, and the function explicitly handles the PARENT_SCOPE assignment.
2026-01-24 12:02:11 -06:00
Scot BreitenfeldandClaude Sonnet 4.5 16364fe2fa Badges (#6170)
* Fix Release Progress badges and workflow

- Fix update-progress.py to output blocker/mustdo counts in correct format
  The workflow expects blocker_done=, blocker_total=, mustdo_done=, mustdo_total=
  output lines but the script was only outputting human-readable format

- Update README badge links to point to project view 24 instead of base project

This fixes the workflow failures where BLOCKER_DONE and the related environment
variables were not being set, causing badge updates to fail.

The badges were pointing to old unmaintained gists under user X.
Updated to use the gist automatically maintained by the Release Progress
workflow (gist ID: 0ad2eabb63b28eb90d69f5e5b2c1496f).

The workflow now successfully updates these badges every 4 hours with
current release blocker and must-do progress.

The script now auto-detects the HDF5 version from src/H5public.h and
filters release blockers/must-do items by milestone matching that version.

Changes:
- Added get_hdf5_version_from_header() to read version from H5public.h
- Updated GraphQL query to fetch milestone information from issues
- Added milestone filtering logic to only count items for the target release
- Made validation more lenient when using milestone filter (allows 0/0)
- Added MILESTONE_FILTER environment variable for manual override

For develop branch with version 2.1.0 in H5public.h, this will now only
count items with milestone containing "2.1" (e.g., "2.1.0", "HDF5 2.1").

* Add version number to badge labels

The badges now display the version (e.g., "2.1 Release Blockers" instead
of just "Release Blockers") when filtering by milestone.

Changes:
- Python script outputs version to GitHub Actions
- Workflow passes version to badge generation script
- Badge script includes version in label if available
- Labels show "X.Y Release Blockers" / "X.Y Release Must Do"


The three badges now track:
1. Release Blockers - Critical issues that must be resolved
2. Release Must Do - Important items for the release
3. Release Nice to Have - Optional improvements for the release

All three badges show version prefix (e.g., "2.1") and support 0/0
display when no items exist for a milestone.

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-01-22 14:37:45 -06:00
Scot Breitenfeld 17a847faf9 Fix Release Progress badges and workflow (#6168)
- Fix update-progress.py to output blocker/mustdo counts in correct format
  The workflow expects blocker_done=, blocker_total=, mustdo_done=, mustdo_total=
  output lines but the script was only outputting human-readable format

- Update README badge links to point to project view 24 instead of base project

This fixes the workflow failures where BLOCKER_DONE and related environment
variables were not being set, causing badge updates to fail.
2026-01-22 10:32:11 -06:00
Scot Breitenfeld 5711c7466f Update the version to 2.1 (#6147)
Update version to 2.1 and derive version information from H5public.h, removing h5vers script and updating CMake and Java configurations.

Versioning:
Update version to 2.1 in H5public.h.
Derive version strings in H5public.h using macros.
CMake:
Extract version from H5public.h in HDF5config.cmake and HDF5AsSubdirMacros.cmake.
Configure README.md and CHANGELOG.md using CMakeLists.txt.
Java:
Generate H5Version.java from H5public.h for version consistency.
Update H5.java to use H5Version for version constants.
Removals:
Delete bin/h5vers script, previously used for version management.
2026-01-21 23:02:49 -06:00
Scot Breitenfeld 21d916841c Changed CI ctest verbose setting (#6136)
* switched from always verbose ctest output to ONLY on test failures

* Windows and macOS builds will skip clang-format check before building documentation

* resolved Doxygen warnings by adding H5G_link_t=H5L_type_t to the DOXYGEN_PREDEFINED list, by adding the macro definition, Doxygen now knows to treat H5G_link_t as H5L_type_
2025-12-30 13:16:55 -06:00
Scot Breitenfeld e34bbfefbf added codeql workflow (#6121)
Adds CodeQL workflow for C/C++ analysis with scheduled, manual, and branch-specific triggers, including dependency setup, HDF5 configuration, and SARIF handling.
2025-12-22 20:48:43 -06:00
Scot Breitenfeld d3cebd7dc5 Addressed TODO documentation issues (#6063)
* Fix Doxygen comments showing backslash continuations in macros, fixes #3947

* Replace "@todo Under Construction" with complete documentation, fixes 3105

* Completed "under construction" documentation sections:
  * H5Smodule.h: Add dataspace selection and data transfer documentation
  * H5module.h: Add link types and organization documentation
  * H5Emodule.h: Add error handling programming model documentation

* Fix grammar in documentation block of H5FDioc.c and improve consistency.

* rephrased H5Smodule.h documentation
2025-12-08 14:23:49 -06:00
Scot Breitenfeld 083eb93a3c fixes the GitHub Actions CPack failure on macOS (#6074)
* fixes the GitHub Actions CPack failure on macOS

* Disables Spotlight indexing which can interfere with disk image operations
* Implements retry logic (waits 5 seconds and retries once if CPack fails)
2025-12-02 12:42:18 -06:00
Scot Breitenfeld aada8a830b Fix Doxygen comments showing backslash continuations in macros (#6047)
Fixes #3947
2025-11-21 14:38:08 -06:00
Scot Breitenfeld 1d410a55f3 Link Checker badge fix. (#6027)
* fixed badge status reporting
2025-11-11 16:50:17 -06:00
Scot Breitenfeld 91ae855c0b Add Fortran SWMR wrappers (#5985)
* Add Fortran wrappers for H5Fstart_swmr_write, H5Dflush, and H5P{set,get}_append_flush
* corrected C doxygen docs for H5Pget_append_flush, clarified Fortran
2025-11-11 15:32:31 -06:00
Scot Breitenfeld 563ef07128 fixed Doxygen parameter annotation issues (#5993)
Fixed Doxygen parameter annotations in HDF5 header files to accurately reflect data flow direction.
2025-11-11 10:41:55 -06:00
Scot Breitenfeld 459445d717 Added SAFE grant acknowledgment (#5991) 2025-11-07 11:12:46 -06:00
Scot Breitenfeld f64c190237 suppresses redirect spam and prevent duplicate output in the reporting (#5977) 2025-11-05 13:51:29 -06:00
Scot Breitenfeld ff81f820ed Skip fragment-only URLs (no more %23 errors) (#5973) 2025-11-04 15:00:04 -06:00
Scot Breitenfeld a6d7b94c0c Added missing parameters H5F_ACC_SWMR_READ_F and H5F_ACC_SWMR_WRITE_F (#5961) 2025-10-31 12:59:45 -05:00
Scot Breitenfeld e4113ffaa5 Combined INSTALL_parallel and README_HPC content (#5952)
* Combined INSTALL_parallel and README_HPC into a single, README_HPC.md file.
* Removed all autotools references
* Updated All References
2025-10-30 21:57:14 -05:00
Scot Breitenfeld 5047b30ff6 Fortran changes to adhere to the filename/module convention (#5914)
Update the documentation table for using H5* API prefixes convention for naming files and modules.

Builds on PR #5912
2025-10-12 17:32:23 -05:00
Scot Breitenfeld 877cbf79a7 Adds new subfiling API H5FDsubfiling_get_file_mapping() (#5828)
Adds H5FDsubfiling_get_file_mapping() API to map logical HDF5 files to physical subfiles, with Fortran support and comprehensive testing.

Adds H5FDsubfiling_get_file_mapping() in H5FDsubfiling.c to retrieve subfile paths for a logical HDF5 file.
Updates Fortran interface in H5VFDff.F90 to include h5fdsubfiling_get_file_mapping_f().

Updates h5fuse.in to support a list of subfiles for processing with a new -l option.
2025-09-26 11:30:16 -05:00
Scot Breitenfeld 06d63b66be improvements to publishing action (#5860)
✅ Fixed Critical Issues

  1. Corrected variable name (inputs.use_hdf → inputs.file_name)
  2. Pinned action versions to specific, secure releases
  3. Added input validation with regex patterns and path traversal
  protection
  4. Enhanced error handling with file existence checks

  🛡️ Security Enhancements

  5. Restricted S3 sync with explicit include/exclude patterns
  6. Added dry-run mode for safe testing
  7. Input sanitization to prevent malicious inputs
  8. Updated to latest action versions with known security fixes

  🚀 Reliability Improvements

  9. File verification steps before processing
  10. Graceful handling of missing optional files
  11. Better error messages and status reporting
  12. Timeout protection (30 minutes max)
  13. Proper content-type headers for HTML files

  📊 Operational Improvements

  14. Detailed logging with emojis for easy scanning
  15. Comprehensive summary at the end
  16. Conditional execution based on file availability
  17. Proper AWS CLI options (--exact-timestamps, --metadata-directive)
2025-09-24 11:07:48 -05:00
Scot Breitenfeld b0fb7ebb2a Fix issue left hand panel issue (#5855) 2025-09-22 08:05:16 -05:00
Scot Breitenfeld 336318301c fixed linkchecker filename (#5827) 2025-09-15 14:46:42 -05:00
Scot Breitenfeld 54d34a2505 Link Checker Fix (#5826)
* Fixed the link checker error where table headers
  contained (\# of rows) and (\# of columns) with escaped hash symbols that Doxygen was
  incorrectly interpreting as malformed links.
2025-09-15 07:32:04 -05:00
Scot Breitenfeld 0d3045f7cb Switched link checker (#5807)
Switch link checker to Lychee in GitHub workflow to reduce false positives and update workflow schedules and documentation links.
2025-09-12 22:08:22 -05:00
Scot Breitenfeld ffbc655699 Resolve issue with left-hand side panel (#5762)
* enforce light mode for doxygen docs: removed dark theme elements

* fix TOC
2025-09-09 13:53:53 -05:00
Scot Breitenfeld 5b8af7182e Fixes warning message indicating that the H5_addr_defined(bt->right) macro might be evaluated with a null bt pointer (#5781) 2025-09-07 16:04:41 -05:00
Scot Breitenfeld 6826420421 moved link checking to only mergers (#5777)
Modify linkchecker.yml to run link checks only on merged pull requests to the develop branch.
2025-09-05 08:13:33 -05:00
Scot Breitenfeld 0a106e22e6 skip doxygen processing of file and last PR comments updates (#5776)
Exclude CONTRIBUTING.md from Doxygen processing and update hyperlink and typo in CONTRIBUTING.md.
2025-09-04 11:31:26 -05:00
Scot Breitenfeld 6e0b7e5726 Updates CONTRIBUTING.md (#5768)
Updates CONTRIBUTING.md with a comprehensive guide for contributors, modifies a link in README.md, and renames a file path for consistency.
2025-09-04 08:05:14 -05:00
Scot Breitenfeld 2ae060f2fb enforce light mode for doxygen docs: removed dark theme elements (#5754) 2025-08-30 08:04:53 -05:00
Scot Breitenfeld 0ab9576cb9 fixed color and only run the action from main repo (#5750) 2025-08-29 11:08:34 -05:00
Scot Breitenfeld b58574e706 Fix the Gist path (#5749)
* updated gist id, fix path
2025-08-27 23:01:59 -05:00
Scot Breitenfeld 5e85a9387e updated gist id (#5748) 2025-08-27 21:33:37 -05:00
Scot Breitenfeld de6b981fcc Updates to PR #5743 (#5747)
* fix paths
2025-08-27 20:26:46 -05:00
Scot Breitenfeld 40bde85ea9 Added release blocker monitor badge (#5743)
This PR implements an automated release progress monitoring system for the HDF5 project. The changes add a dynamic badge to the README that tracks completion of release-blocking issues from GitHub Project #39, addressing issue #5742. The implementation consists of three components:

- Python script (update-progress.py) that uses GitHub's GraphQL API to query project items, filter for 'Release_Blocker' items, and calculate completion percentage
- GitHub Actions workflow (update-progress.yml) that runs the script every 24 hours, generates badge data, and publishes it via GitHub Gist
- README updates that remove tentative release language and add the new progress badge with color-coded status indicators

The system replaces static release information with real-time progress tracking, providing better transparency into HDF5's feature-driven release process. The badge displays progress percentages with green (90%+), yellow (60-79%), orange (40-59%), and red (<40%) color coding.
2025-08-27 16:41:32 -05:00
Scot Breitenfeld 7f49eae0f3 Add Google Analytics to Doxygen pages. (#5728) 2025-08-18 07:47:23 -05:00
Scot Breitenfeld 3f5a6e272e correct url paths (#5651) 2025-07-09 14:51:45 -05:00