Skip to content

deps: bump libarchive to v3.8.7, fix all dep-update workflows - #29289

Merged
Jarred-Sumner merged 3 commits into
mainfrom
jarred/libarchive-bumper
Apr 14, 2026
Merged

Jarred-Sumner merged 3 commits into
mainfrom
jarred/libarchive-bumper

Conversation

@Jarred-Sumner

@Jarred-Sumner Jarred-Sumner commented Apr 14, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

The scheduled update-*.yml workflows have been failing since #28640 removed cmake/targets/*.cmake — they were still reading pinned commits from those files. The dependency definitions now live in scripts/build/deps/*.ts as const <NAME>_COMMIT = "...".

Workflows fixed — update-{libarchive,cares,hdrhistogram,highway,libdeflate,lolhtml,lshpack,zstd}.yml now read/write the _COMMIT constant in the corresponding scripts/build/deps/*.ts file. The replacement step also moved to the safe env: pattern instead of inlining ${{ }} into the shell.

libarchive bumped — 3.8.1 → 3.8.7 (compare). archive_write_add_filter_gzip.c.patch rebased onto the new upstream, which reformatted the surrounding code; no semantic change — still adds the gzip:os option used by bun pm pack for reproducible tarballs.

The other 7 deps were not version-bumped here; their now-working workflows will open bump PRs on the next scheduled run.

Closes oven-sh/bun#26652
Closes oven-sh/bun#26432
Closes oven-sh/bun#26209
Closes oven-sh/bun#25955
Closes oven-sh/bun#25818
Closes oven-sh/bun#25726
Closes oven-sh/bun#25625
Closes oven-sh/bun#25507
Closes oven-sh/bun#25380

How did you verify your code works?

  • bun scripts/build.ts --target=libarchive — patches apply cleanly, builds libarchive.a with ARCHIVE_VERSION_NUMBER 3008007
  • bun bd test test/js/bun/archive.test.ts — 99 pass
  • bun bd test test/cli/install/bun-pack.test.ts — 70 pass
  • bun pm pack gzip header byte 9 = 0xff, confirming the rebased patch is functional
  • All 8 workflows: extraction sed returns valid 40-char hash from the live .ts file (GNU sed)
  • All 8 workflows: replacement sed rewrites exactly one line, round-trips back through extraction
  • All 8 workflows: YAML parses, no cmake references remain

The update-libarchive.yml workflow was failing because it read the
pinned commit from cmake/targets/BuildLibArchive.cmake, which was
removed in #28640 when the build moved to scripts/build/deps/*.ts.

- Point the workflow at scripts/build/deps/libarchive.ts and update
  the sed extraction/replacement to match the
  `const LIBARCHIVE_COMMIT = "..."` format.
- Bump libarchive 3.8.1 -> 3.8.7 (ded82291ab41d5e3).
- Rebase archive_write_add_filter_gzip.c.patch onto the new upstream
  source, which reformatted the compression-level branches around
  compressed[8]. Semantics unchanged: still adds the `os` option,
  defaults to Unix, and narrows crc to uint32_t.

The other update-*.yml workflows have the same stale cmake reference
and will be fixed separately.
@robobun

robobun commented Apr 14, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 10:09 PM PT - Apr 13th, 2026

❌ @Jarred-Sumner, your commit 57abf42 has some failures in Build #45612 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 29289

That installs a local version of the PR into your bun-29289 executable, so you can run:

bun-29289 --bun

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. deps: update libarchive to v3.8.6 #26652 - Also bumps libarchive (to v3.8.6); superseded by this PR's bump to v3.8.7
  2. deps: update libarchive to v3.8.5 #26432 - Also bumps libarchive (to v3.8.5); superseded by this PR's bump to v3.8.7
  3. deps: update libarchive to v3.8.5 #26209 - Also bumps libarchive (to v3.8.5); superseded by this PR's bump to v3.8.7
  4. deps: update libarchive to v3.8.5 #25955 - Also bumps libarchive (to v3.8.5); superseded by this PR's bump to v3.8.7
  5. deps: update libarchive to v3.8.4 #25818 - Also bumps libarchive (to v3.8.4); superseded by this PR's bump to v3.8.7
  6. deps: update libarchive to v3.8.4 #25726 - Also bumps libarchive (to v3.8.4); superseded by this PR's bump to v3.8.7
  7. deps: update libarchive to v3.8.4 #25625 - Also bumps libarchive (to v3.8.4); superseded by this PR's bump to v3.8.7
  8. deps: update libarchive to v3.8.4 #25507 - Also bumps libarchive (to v3.8.4); superseded by this PR's bump to v3.8.7
  9. deps: update libarchive to v3.8.4 #25380 - Also bumps libarchive (to v3.8.4); superseded by this PR's bump to v3.8.7

🤖 Generated with Claude Code

Same root cause as the libarchive fix in the previous commit: these
seven workflows still read the pinned commit from cmake/targets/*.cmake
files that were removed in #28640. Point each one at the corresponding
scripts/build/deps/<name>.ts file and update the sed patterns to match
the `const <NAME>_COMMIT = "..."` format.

Covers cares, hdrhistogram, highway, libdeflate, lolhtml, lshpack, zstd.
@Jarred-Sumner Jarred-Sumner changed the title deps: bump libarchive to v3.8.7, fix updater workflow deps: bump libarchive to v3.8.7, fix all dep-update workflows Apr 14, 2026
@coderabbitai

coderabbitai Bot commented Apr 14, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 607075ae-a850-4444-b400-f8f336365a84

📥 Commits

Reviewing files that changed from the base of the PR and between b22db75 and 57abf42.

📒 Files selected for processing (1)
  • test/js/node/process/process.test.js

Walkthrough

Workflows were changed to read/update dependency SHAs from TypeScript dependency files instead of CMake targets; the libarchive dependency SHA was bumped; a gzip compressor patch adds a configurable OS field and changes the CRC type; a test expectation was updated to the new libarchive SHA.

Changes

Cohort / File(s) Summary
CI workflow updates (switch from CMake to TS)
/.github/workflows/update-libarchive.yml, /.github/workflows/update-cares.yml, /.github/workflows/update-hdrhistogram.yml, /.github/workflows/update-highway.yml, /.github/workflows/update-libdeflate.yml, /.github/workflows/update-lolhtml.yml, /.github/workflows/update-lshpack.yml, /.github/workflows/update-zstd.yml
Each workflow now extracts a 40-hex SHA from const <NAME>_COMMIT = "<sha>"; in scripts/build/deps/*.ts (using sed), validates that value, and updates that constant in-place using an env: LATEST variable. PR add-paths and error messages were updated to reference the TS files.
Dependency commit file changed
scripts/build/deps/libarchive.ts
Updated LIBARCHIVE_COMMIT value from 9525f90ca4bd14c7b335e2f8c84a4607b0af6bdf to ded82291ab41d5e355831b96b0e1ff49e24d8939.
Other TS dependency files referenced by workflows
scripts/build/deps/cares.ts, scripts/build/deps/hdrhistogram.ts, scripts/build/deps/highway.ts, scripts/build/deps/libdeflate.ts, scripts/build/deps/lolhtml.ts, scripts/build/deps/lshpack.ts, scripts/build/deps/zstd.ts
Workflows were changed to target these TypeScript files for reading/updating <NAME>_COMMIT constants instead of corresponding CMake target files (no commit values in these files were modified in this PR except libarchive).
Gzip compression patch
patches/libarchive/archive_write_add_filter_gzip.c.patch
Introduced os field in gzip compressor state (default Unix=3), added parsing/mapping for an "os" option (FAT, Amiga, VMS, Unknown, etc.), adjusted options parsing flow and gzip header byte population to use data->os, and changed CRC type from unsigned long to uint32_t.
Test update
test/js/node/process/process.test.js
Updated expected libarchive commit SHA in test from 9525f9...bdf to ded8229...d8939.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: bumping libarchive to v3.8.7 and fixing all eight dependency-update workflows that were broken after cmake file removal.
Description check ✅ Passed The description follows the template with both required sections (What does this PR do, How did you verify your code works) fully completed with comprehensive details, context, and verification steps.
Linked Issues check ✅ Passed The PR successfully addresses the core objective of all linked issues: bumping libarchive past version 3.8.1 (they were targeting 3.8.4–3.8.6). This PR bumps to 3.8.7, superseding all prior attempts and fixing the underlying workflow issue preventing automatic updates.
Out of Scope Changes check ✅ Passed All changes are directly in scope: eight workflow files updated to read from new locations, libarchive version bumped with patch rebased, and test expectations updated accordingly. No extraneous changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
patches/libarchive/archive_write_add_filter_gzip.c.patch (1)

33-52: 🧹 Nitpick | 🔵 Trivial

Add a regression test for gzip:os=Unknown.

This is Bun-specific carry-patch behavior. Please extend test/cli/install/bun-pack.test.ts (or equivalent) with an assertion that gzip header byte 9 is 0xff, so the next libarchive rebase can't silently fall back to 3 again.

Also applies to: 62-62

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@patches/libarchive/archive_write_add_filter_gzip.c.patch` around lines 33 -
52, The gzip OS mapping patch sets data->os = 255 for "Unknown" but we need a
regression test to ensure libarchive writes byte 9 of the gzip header as 0xFF
when using gzip:os=Unknown; add a test to test/cli/install/bun-pack.test.ts (or
the equivalent CLI pack test) that creates a gzip with the option
gzip:os=Unknown (or exercises the bun pack path that emits that header), reads
the produced gzip file bytes and asserts that header byte index 9 equals 0xFF,
and add the same assertion for any duplicate OS-handling code path (the other
"os" branch) to prevent future regressions where it falls back to 3.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/update-libarchive.yml:
- Around line 23-24: The sed pattern reading/writing CURRENT_VERSION is too
strict and will break if whitespace or the trailing semicolon/quote style
changes; update the regexes that reference const LIBARCHIVE_COMMIT in the
workflow (the lines that set DEP_FILE and CURRENT_VERSION and the other
occurrence around line 80) to use a more tolerant pattern that allows optional
whitespace, optional semicolon, and either single or double quotes around the
SHA (e.g., match LIBARCHIVE_COMMIT\s*=\s*['"]([0-9a-f]{40})['"]\s*;?) so the
updater can still find and replace the SHA in scripts/build/deps/libarchive.ts
even after formatting-only edits.

---

Outside diff comments:
In `@patches/libarchive/archive_write_add_filter_gzip.c.patch`:
- Around line 33-52: The gzip OS mapping patch sets data->os = 255 for "Unknown"
but we need a regression test to ensure libarchive writes byte 9 of the gzip
header as 0xFF when using gzip:os=Unknown; add a test to
test/cli/install/bun-pack.test.ts (or the equivalent CLI pack test) that creates
a gzip with the option gzip:os=Unknown (or exercises the bun pack path that
emits that header), reads the produced gzip file bytes and asserts that header
byte index 9 equals 0xFF, and add the same assertion for any duplicate
OS-handling code path (the other "os" branch) to prevent future regressions
where it falls back to 3.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: af192f6e-99b8-41fe-bb91-cf5fa5abe520

📥 Commits

Reviewing files that changed from the base of the PR and between ccbaed9 and 62caf1d.

📒 Files selected for processing (3)
  • .github/workflows/update-libarchive.yml
  • patches/libarchive/archive_write_add_filter_gzip.c.patch
  • scripts/build/deps/libarchive.ts

Comment on lines +23 to +24
DEP_FILE=scripts/build/deps/libarchive.ts
CURRENT_VERSION=$(sed -nE 's/^const LIBARCHIVE_COMMIT = "([0-9a-f]{40})";$/\1/p' "$DEP_FILE")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Loosen the sed patterns before a formatting-only edit breaks the updater.

Both expressions require the line to stay exactly const LIBARCHIVE_COMMIT = "<sha>";. Extra whitespace or a dropped semicolon in scripts/build/deps/libarchive.ts will make the workflow stop reading or rewriting the pin even though the value is still there.

💡 More tolerant `sed` patterns
-          CURRENT_VERSION=$(sed -nE 's/^const LIBARCHIVE_COMMIT = "([0-9a-f]{40})";$/\1/p' "$DEP_FILE")
+          CURRENT_VERSION=$(sed -nE 's/^[[:space:]]*const[[:space:]]+LIBARCHIVE_COMMIT[[:space:]]*=[[:space:]]*"([0-9a-f]{40})";?[[:space:]]*$/\1/p' "$DEP_FILE")

-          sed -i -E 's/^(const LIBARCHIVE_COMMIT = ")[0-9a-f]{40}(";)$/\1'"$LATEST"'\2/' scripts/build/deps/libarchive.ts
+          sed -i -E 's/^([[:space:]]*const[[:space:]]+LIBARCHIVE_COMMIT[[:space:]]*=[[:space:]]*")[0-9a-f]{40}(";?[[:space:]]*)$/\1'"$LATEST"'\2/' scripts/build/deps/libarchive.ts

Also applies to: 80-80

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/update-libarchive.yml around lines 23 - 24, The sed
pattern reading/writing CURRENT_VERSION is too strict and will break if
whitespace or the trailing semicolon/quote style changes; update the regexes
that reference const LIBARCHIVE_COMMIT in the workflow (the lines that set
DEP_FILE and CURRENT_VERSION and the other occurrence around line 80) to use a
more tolerant pattern that allows optional whitespace, optional semicolon, and
either single or double quotes around the SHA (e.g., match
LIBARCHIVE_COMMIT\s*=\s*['"]([0-9a-f]{40})['"]\s*;?) so the updater can still
find and replace the SHA in scripts/build/deps/libarchive.ts even after
formatting-only edits.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward workflow fix pointing to the correct dep file, clean version bump, and a semantically identical patch rebase.

Extended reasoning...

Overview

Three files changed: (1) the update-libarchive workflow is fixed to read/write LIBARCHIVE_COMMIT from scripts/build/deps/libarchive.ts instead of the deleted cmake file, (2) LIBARCHIVE_COMMIT is bumped from 3.8.1→3.8.7, and (3) the gzip patch is rebased onto the new upstream context lines with no semantic changes.

Security risks

The workflow change moves the SHA interpolation into an env: block (LATEST: ${{ steps.check-version.outputs.latest }}) rather than inline in the run: shell script — this is the correct pattern and avoids expression-injection risk. No auth, crypto, or permission-sensitive code is touched.

Level of scrutiny

Low. This is a dependency version bump + workflow repair with thorough manual verification documented in the PR description (build, archive tests, pack tests, byte-9 confirmation). The patch rebase is mechanical — only context lines shifted, the actual diff hunks are identical in effect.

Other factors

The only reported issue is a pre-existing fragility in the tag SHA dereference (annotated vs lightweight tags) that is entirely unmodified by this PR and does not affect correctness today. The fix is self-contained and unblocks a scheduled workflow that has been broken since the cmake migration.

Comment on lines 73 to 87

- name: Update version if needed
if: success() && steps.check-version.outputs.current != steps.check-version.outputs.latest
env:
LATEST: ${{ steps.check-version.outputs.latest }}
run: |
set -euo pipefail
# Handle multi-line format where COMMIT and its value are on separate lines
sed -i -E '/[[:space:]]*COMMIT[[:space:]]*$/{n;s/[[:space:]]*([0-9a-f]+)[[:space:]]*$/ ${{ steps.check-version.outputs.latest }}/}' cmake/targets/BuildLibArchive.cmake
sed -i -E 's/^(const LIBARCHIVE_COMMIT = ")[0-9a-f]{40}(";)$/\1'"$LATEST"'\2/' scripts/build/deps/libarchive.ts

- name: Create Pull Request
if: success() && steps.check-version.outputs.current != steps.check-version.outputs.latest
uses: peter-evans/create-pull-request@v7
with:
token: ${{ secrets.GITHUB_TOKEN }}
add-paths: |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing fragility: the two-step tag SHA dereference at lines 53–68 only works for annotated tags; if libarchive ever switches to lightweight tags, step 2 (GET /git/tags/{sha}) would return 404 and jq would yield 'null', failing the workflow. This code is unmodified by the PR — libarchive currently uses annotated tags so it works in practice.

Extended reasoning...

The tag SHA resolution block uses a two-step dereference pattern:

Step 1: GET /git/refs/tags/{LATEST_TAG} → extracts .object.sha as LATEST_TAG_SHA
Step 2: GET /git/tags/{LATEST_TAG_SHA} → extracts .object.sha as LATEST_SHA

This is correct only for annotated tags. In that case, step 1 returns a tag-object SHA (not a commit SHA), and step 2 dereferences the tag object to get the underlying commit SHA.

For lightweight tags, the behavior breaks: step 1 returns the commit SHA directly (there is no intermediate tag object). Step 2 then calls GET /git/tags/{COMMIT_SHA}, which returns HTTP 404 because no tag object exists for that SHA. The jq -r '.object.sha' call on a 404 response body returns null, causing LATEST_SHA to be set to the string "null".

The subsequent validation (if [ -z "$LATEST_SHA" ] || [ "$LATEST_SHA" = "null" ]) does catch this case and exits with an error — so the failure is loud, not silent. However, the workflow would then stop producing automatic update PRs until the code is fixed.

Concrete proof:

  1. libarchive releases lightweight tag v4.0.0
  2. GET /git/refs/tags/v4.0.0 → { "object": { "sha": "abc123...commit_sha", "type": "commit" } }
  3. LATEST_TAG_SHA=abc123...commit_sha
  4. GET /git/tags/abc123...commit_sha → HTTP 404 (no tag object)
  5. jq -r '.object.sha' on the 404 body → null
  6. Validation: [ "null" = "null" ] → true → exit 1

This code is pre-existing and entirely unmodified by this PR. The PR changes only: the CURRENT_VERSION extraction (awk→sed), the env block for LATEST, and the add-paths list. libarchive currently uses annotated tags so this does not fail in practice, but switching to lightweight tags would break the workflow.

Fix: use the /git/commits/{sha} endpoint or check the .object.type field from step 1 — if it's "commit", use that SHA directly; if it's "tag", do the second dereference.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/update-highway.yml:
- Around line 23-24: The workflow duplicates the sed-based extract/replace logic
for DEP_FILE and HIGHWAY_COMMIT; extract that routine into a single reusable
helper (either a shell script e.g., scripts/update-commit.sh or a composite
GitHub Action) and call it from this workflow and the other duplicate spots (the
same pattern at the other occurrence around the 95-99 block). The helper should
accept the dep file path (DEP_FILE) and the commit constant name
(HIGHWAY_COMMIT) and perform the sed extraction and rewrite so future format
changes need one edit.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 82e89e20-2757-4a0e-943e-a1dbb30f9b93

📥 Commits

Reviewing files that changed from the base of the PR and between 62caf1d and b22db75.

📒 Files selected for processing (7)
  • .github/workflows/update-cares.yml
  • .github/workflows/update-hdrhistogram.yml
  • .github/workflows/update-highway.yml
  • .github/workflows/update-libdeflate.yml
  • .github/workflows/update-lolhtml.yml
  • .github/workflows/update-lshpack.yml
  • .github/workflows/update-zstd.yml

Comment on lines +23 to +24
DEP_FILE=scripts/build/deps/highway.ts
CURRENT_VERSION=$(sed -nE 's/^const HIGHWAY_COMMIT = "([0-9a-f]{40})";$/\1/p' "$DEP_FILE")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider extracting the *_COMMIT update routine into a shared helper.

This same sed extraction and rewrite pattern is duplicated across the dependency update workflows touched in this PR. A small shared script or composite action would make the next dep-file format change a one-place edit.

Also applies to: 95-99

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/update-highway.yml around lines 23 - 24, The workflow
duplicates the sed-based extract/replace logic for DEP_FILE and HIGHWAY_COMMIT;
extract that routine into a single reusable helper (either a shell script e.g.,
scripts/update-commit.sh or a composite GitHub Action) and call it from this
workflow and the other duplicate spots (the same pattern at the other occurrence
around the 95-99 block). The helper should accept the dep file path (DEP_FILE)
and the commit constant name (HIGHWAY_COMMIT) and perform the sed extraction and
rewrite so future format changes need one edit.

Comment on lines 76 to 94

- name: Update version if needed
if: success() && steps.check-version.outputs.current != steps.check-version.outputs.latest
env:
LATEST: ${{ steps.check-version.outputs.latest }}
run: |
set -euo pipefail
# Handle multi-line format where COMMIT and its value are on separate lines
sed -i -E '/[[:space:]]*COMMIT[[:space:]]*$/{n;s/[[:space:]]*([0-9a-f]+)[[:space:]]*$/ ${{ steps.check-version.outputs.latest }}/}' cmake/targets/BuildHdrHistogram.cmake
sed -i -E 's/^(const HDRHISTOGRAM_COMMIT = ")[0-9a-f]{40}(";)$/\1'"$LATEST"'\2/' scripts/build/deps/hdrhistogram.ts

- name: Create Pull Request
if: success() && steps.check-version.outputs.current != steps.check-version.outputs.latest
uses: peter-evans/create-pull-request@v7
with:
token: ${{ secrets.GITHUB_TOKEN }}
add-paths: |
cmake/targets/BuildHdrHistogram.cmake
scripts/build/deps/hdrhistogram.ts
commit-message: "deps: update hdrhistogram to ${{ steps.check-version.outputs.tag }} (${{ steps.check-version.outputs.latest }})"
title: "deps: update hdrhistogram to ${{ steps.check-version.outputs.tag }}"
delete-branch: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 In update-hdrhistogram.yml, the annotated-tag dereference uses a silent fallback (2>/dev/null + .object.sha // empty) that cannot distinguish a legitimate lightweight tag from a transient network error on the second curl; if the second call fails while the tag is annotated, the fallback silently pins LATEST_TAG_SHA (a tag-object SHA, not a commit SHA) into hdrhistogram.ts, creating a PR with a wrong SHA that passes 40-char hex validation. The other three workflows fixed in this same PR (update-highway.yml, update-lolhtml.yml, update-lshpack.yml) avoid this by checking .object.type from the first API response instead.

Extended reasoning...

The hdrhistogram workflow resolves a tag to a commit SHA using this pattern (lines ~61-65 of the updated file):

LATEST_SHA=$(curl -sL ".../git/tags/$LATEST_TAG_SHA" 2>/dev/null | jq -r '.object.sha // empty')
if [ -z "$LATEST_SHA" ]; then
  LATEST_SHA="$LATEST_TAG_SHA"
fi

The intent is correct for the lightweight-tag case: if HdrHistogram uses a lightweight tag, GET /git/tags/{sha} returns 404, jq gets no parseable JSON, and .object.sha // empty produces an empty string, so the fallback correctly reuses the commit SHA from step 1.

The problem is that "empty output" is also what you get when curl itself fails. In bash with set -euo pipefail, a command substitution pipeline does not abort the outer script even under -e, so a failed curl inside $(...) is swallowed silently. The 2>/dev/null additionally suppresses any stderr. If the call produces no JSON (connection timeout, etc.), jq -r '.object.sha // empty' outputs an empty string — identical to the legitimate lightweight-tag path.

Why this produces incorrect output, not just an error: When HdrHistogram uses annotated tags (common for release tags), LATEST_TAG_SHA is the SHA of a tag object, not of the underlying commit. If the second curl call fails, the fallback sets LATEST_SHA = LATEST_TAG_SHA (a tag-object SHA). The subsequent 40-char hex validation passes — tag object SHAs are also 40-character lowercase hex strings. The workflow then writes this tag-object SHA into scripts/build/deps/hdrhistogram.ts and creates a PR. A build that tries to fetch at that SHA will fail because the SHA refers to a tag object, not a commit/tree.

Concrete failure path:

  1. HdrHistogram releases annotated tag v0.12.0
  2. GET /git/refs/tags/v0.12.0 succeeds → LATEST_TAG_SHA = abc123...tag_object_sha (type: tag)
  3. GET /git/tags/abc123...tag_object_sha fails due to transient network error; curl exits 0 with no output; 2>/dev/null hides stderr
  4. jq -r '.object.sha // empty' on empty input → empty string → LATEST_SHA=""
  5. Fallback: LATEST_SHA="$LATEST_TAG_SHA" = tag-object SHA
  6. 40-char hex validation passes (tag-object SHAs are valid 40-char hex)
  7. Workflow writes tag-object SHA into hdrhistogram.ts and opens a PR
  8. Downstream build fetches at a tag-object SHA → build failure

Contrast with the three correctly-fixed workflows in this same PR:

  • update-highway.yml: reads TAG_OBJECT_TYPE=$(... | jq -r '.object.type') from the first response; branches on "commit" vs "tag"; error-exits on unexpected type
  • update-lolhtml.yml: same pattern — checks TAG_OBJECT_TYPE from the first call, only does second dereference if type is "tag"
  • update-lshpack.yml: same pattern

The fix is to extract .object.type from the first /git/refs/tags/ call (which already succeeded) and branch on it, eliminating any reliance on the second curl outcome to distinguish the two cases. No extra HTTP round-trip is needed.

Comment on lines 20 to 36
run: |
set -euo pipefail

# Extract the commit hash from the line after COMMIT
CURRENT_VERSION=$(awk '/[[:space:]]*COMMIT[[:space:]]*$/{getline; gsub(/^[[:space:]]+|[[:space:]]+$/,"",$0); print}' cmake/targets/BuildCares.cmake)
DEP_FILE=scripts/build/deps/cares.ts
CURRENT_VERSION=$(sed -nE 's/^const CARES_COMMIT = "([0-9a-f]{40})";$/\1/p' "$DEP_FILE")

if [ -z "$CURRENT_VERSION" ]; then
echo "Error: Could not find COMMIT line in BuildCares.cmake"
echo "Error: Could not find CARES_COMMIT in $DEP_FILE"
exit 1
fi

# Validate that it looks like a git hash
if ! [[ $CURRENT_VERSION =~ ^[0-9a-f]{40}$ ]]; then
echo "Error: Invalid git hash format in BuildCares.cmake"
echo "Error: Invalid git hash format in $DEP_FILE"
echo "Found: $CURRENT_VERSION"
echo "Expected: 40 character hexadecimal string"
exit 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Four of the eight updated workflows (update-libarchive, update-cares, update-libdeflate, update-zstd) still use a naive two-step tag dereference that assumes all upstream releases use annotated tags; if any of those four projects publishes a lightweight tag, the second API call returns 404, jq yields 'null', and the workflow exits 1, silently stopping auto-updates. The fix was already applied to the other four workflows (update-highway, update-lolhtml, update-lshpack, update-hdrhistogram) in this very PR, making the omission an oversight — apply the same type-aware dereference pattern to the remaining four.

Extended reasoning...

What the bug is and how it manifests

This PR updates all eight dep-update workflows to read/write pinned commits from TypeScript files instead of CMake files. Four of the workflows (update-highway, update-lolhtml, update-lshpack, update-hdrhistogram) were additionally upgraded to handle both lightweight and annotated Git tags by checking .object.type from the GitHub refs API. The remaining four (update-libarchive, update-cares, update-libdeflate, update-zstd) received only the TypeScript migration and still use a naive two-step annotated-tag-only dereference pattern.

The specific code path that triggers it

For the four affected workflows, the SHA resolution works as follows:

  • Step 1: GET /git/refs/tags/{TAG} → extracts .object.sha as LATEST_TAG_SHA
  • Step 2: GET /git/tags/{LATEST_TAG_SHA} → extracts .object.sha as LATEST_SHA

Step 2 unconditionally assumes LATEST_TAG_SHA is an annotated tag object SHA. If the upstream project publishes a lightweight tag instead, step 1 returns the commit SHA directly (.object.type = "commit"), and step 2 calls GET /git/tags/{COMMIT_SHA} — an endpoint that returns HTTP 404 because no tag object exists for a raw commit SHA.

Why existing code doesn't prevent it

The null checks (if [ -z "$LATEST_SHA" ] || [ "$LATEST_SHA" = "null" ]) will catch the 404 and exit 1, so the failure is not silent per se. However, they do not recover — the workflow simply dies with an error, and no update PR is created. The real impact is that auto-updates silently stop working until the code is manually fixed.

Step-by-step proof

  1. libarchive (or c-ares / libdeflate / zstd) releases v4.0.0 as a lightweight tag
  2. LATEST_TAG_SHA=$(curl .../git/refs/tags/v4.0.0 | jq -r '.object.sha') → returns the commit SHA, e.g. abc123… (type=commit)
  3. LATEST_SHA=$(curl .../git/tags/abc123… | jq -r '.object.sha') → GitHub returns HTTP 404; jq on the 404 body yields null
  4. [ "$LATEST_SHA" = "null" ] is true → exit 1 — no update PR is generated
  5. The scheduled weekly run will keep failing until someone manually fixes the workflow

How to fix it

Apply the same type-aware pattern already used in update-highway.yml (or the // empty fallback in update-hdrhistogram.yml). For each of the four affected workflows, read .object.type alongside .object.sha from the refs API response: if type == "commit", use that SHA directly; if type == "tag", perform the second dereference to get the underlying commit SHA.

Context on the pre-existing review comment

A prior inline comment on this PR flagged the libarchive workflow's two-step dereference as a pre-existing fragility and said the code was "unmodified by the PR". However, all eight workflows previously used cmake-based awk extraction — none had this two-step dereference pattern before this PR. The pattern was introduced by this PR, but applied inconsistently: four workflows received the type-aware fix, four did not, making the omission an oversight within this PR rather than a pre-existing issue.

@Jarred-Sumner
Jarred-Sumner merged commit edde070 into main Apr 14, 2026
60 of 61 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the jarred/libarchive-bumper branch April 14, 2026 04:11
structwafel pushed a commit to structwafel/bun that referenced this pull request Apr 25, 2026
…h#29289)

## What does this PR do?

The scheduled `update-*.yml` workflows have been failing since oven-sh#28640
removed `cmake/targets/*.cmake` — they were still reading pinned commits
from those files. The dependency definitions now live in
`scripts/build/deps/*.ts` as `const <NAME>_COMMIT = "..."`.

**Workflows fixed** —
`update-{libarchive,cares,hdrhistogram,highway,libdeflate,lolhtml,lshpack,zstd}.yml`
now read/write the `_COMMIT` constant in the corresponding
`scripts/build/deps/*.ts` file. The replacement step also moved to the
safe `env:` pattern instead of inlining `${{ }}` into the shell.

**libarchive bumped** — 3.8.1 → 3.8.7
([compare](libarchive/libarchive@9525f90...ded8229)).
`archive_write_add_filter_gzip.c.patch` rebased onto the new upstream,
which reformatted the surrounding code; no semantic change — still adds
the `gzip:os` option used by `bun pm pack` for reproducible tarballs.

The other 7 deps were not version-bumped here; their now-working
workflows will open bump PRs on the next scheduled run.

Closes [oven-sh#26652](oven-sh#26652)
Closes [oven-sh#26432](oven-sh#26432)
Closes [oven-sh#26209](oven-sh#26209)
Closes [oven-sh#25955](oven-sh#25955)
Closes [oven-sh#25818](oven-sh#25818)
Closes [oven-sh#25726](oven-sh#25726)
Closes [oven-sh#25625](oven-sh#25625)
Closes [oven-sh#25507](oven-sh#25507)
Closes [oven-sh#25380](oven-sh#25380)

## How did you verify your code works?

- [x] `bun scripts/build.ts --target=libarchive` — patches apply
cleanly, builds `libarchive.a` with `ARCHIVE_VERSION_NUMBER 3008007`
- [x] `bun bd test test/js/bun/archive.test.ts` — 99 pass
- [x] `bun bd test test/cli/install/bun-pack.test.ts` — 70 pass
- [x] `bun pm pack` gzip header byte 9 = `0xff`, confirming the rebased
patch is functional
- [x] All 8 workflows: extraction sed returns valid 40-char hash from
the live `.ts` file (GNU sed)
- [x] All 8 workflows: replacement sed rewrites exactly one line,
round-trips back through extraction
- [x] All 8 workflows: YAML parses, no `cmake` references remain
xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026
…h#29289)

## What does this PR do?

The scheduled `update-*.yml` workflows have been failing since oven-sh#28640
removed `cmake/targets/*.cmake` — they were still reading pinned commits
from those files. The dependency definitions now live in
`scripts/build/deps/*.ts` as `const <NAME>_COMMIT = "..."`.

**Workflows fixed** —
`update-{libarchive,cares,hdrhistogram,highway,libdeflate,lolhtml,lshpack,zstd}.yml`
now read/write the `_COMMIT` constant in the corresponding
`scripts/build/deps/*.ts` file. The replacement step also moved to the
safe `env:` pattern instead of inlining `${{ }}` into the shell.

**libarchive bumped** — 3.8.1 → 3.8.7
([compare](libarchive/libarchive@9525f90...ded8229)).
`archive_write_add_filter_gzip.c.patch` rebased onto the new upstream,
which reformatted the surrounding code; no semantic change — still adds
the `gzip:os` option used by `bun pm pack` for reproducible tarballs.

The other 7 deps were not version-bumped here; their now-working
workflows will open bump PRs on the next scheduled run.

Closes [oven-sh#26652](oven-sh#26652)
Closes [oven-sh#26432](oven-sh#26432)
Closes [oven-sh#26209](oven-sh#26209)
Closes [oven-sh#25955](oven-sh#25955)
Closes [oven-sh#25818](oven-sh#25818)
Closes [oven-sh#25726](oven-sh#25726)
Closes [oven-sh#25625](oven-sh#25625)
Closes [oven-sh#25507](oven-sh#25507)
Closes [oven-sh#25380](oven-sh#25380)

## How did you verify your code works?

- [x] `bun scripts/build.ts --target=libarchive` — patches apply
cleanly, builds `libarchive.a` with `ARCHIVE_VERSION_NUMBER 3008007`
- [x] `bun bd test test/js/bun/archive.test.ts` — 99 pass
- [x] `bun bd test test/cli/install/bun-pack.test.ts` — 70 pass
- [x] `bun pm pack` gzip header byte 9 = `0xff`, confirming the rebased
patch is functional
- [x] All 8 workflows: extraction sed returns valid 40-char hash from
the live `.ts` file (GNU sed)
- [x] All 8 workflows: replacement sed rewrites exactly one line,
round-trips back through extraction
- [x] All 8 workflows: YAML parses, no `cmake` references remain
Jarred-Sumner added a commit that referenced this pull request Aug 30, 2026
…29295)

## What does this PR do?

The `process.versions` test in `test/js/node/process/process.test.js`
hardcoded the expected commit hash for every vendored dependency. Each
dep bump — including the automated `update-*.yml` workflows fixed in
#29289 — required a matching edit here, and forgetting it broke CI.

This rewrites the test to read each pinned commit out of
`scripts/build/deps/<name>.ts` at test time using the same `^const
X_COMMIT = "([0-9a-f]{40})";$` pattern the workflows use. Single source
of truth: bumping a dep no longer requires touching this test, and it
still catches the real failure mode (build didn't propagate the
source-tree commit through `depVersionsHeader.ts` →
`bun_dependency_versions.h` → `process.versions`).

## How did you verify your code works?

- [x] `bun bd test test/js/node/process/process.test.js` — 101 pass, 0
fail
- [x] `USE_SYSTEM_BUN=1 bun test ... -t "^process.versions$"` — fails
(system bun has older libarchive than source tree)
- [x] Negative check: temporarily edited `ZSTD_COMMIT` in
`scripts/build/deps/zstd.ts` to a dummy hash → test fails with clear
expected/received diff

Co-authored-by: robobun <robobun@oven.sh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants