Skip to content

fix(scripts): stop license checks failing on valid headers - #971

Merged
lizhengfeng101 merged 1 commit into
alibaba:mainfrom
chethanuk:fix/license-script-sigpipe
Aug 17, 2026
Merged

fix(scripts): stop license checks failing on valid headers#971
lizhengfeng101 merged 1 commit into
alibaba:mainfrom
chethanuk:fix/license-script-sigpipe

Conversation

@chethanuk

Copy link
Copy Markdown
Contributor

Description

make license-check fails intermittently on files whose headers are correct, naming a different file each run.

Both license scripts test the header with echo "$header" | grep -q .... grep -q exits the moment it matches, so echo can lose the race on the write end and die of SIGPIPE (141). Both scripts run under set -o pipefail, which promotes that 141 to the pipeline's status, and ! pipeline then reads a header that is plainly present as missing.

Measured on internal/llmloop/compression.go — a 410-byte header, far under the 64KiB pipe buffer — with the CPU saturated: 4 spurious failures in 3000 iterations of the SPDX pipeline alone. End to end under the same load, verify-license.sh failed 5 of 10 runs on current main and 0 of 10 with this change.

has_header() in add-license.sh has the identical race, and there it is worse than a flake. A false negative makes add_header() prepend a second copyright block to a file that already has one — which is the duplicate SPDX header that had to be removed from internal/config/testconnection/testconnection.go during review of #605.

Feeding the header through a here-string removes the second process, so nothing is left to receive SIGPIPE. The year lookup was a grep | grep | head -1 chain carrying the same exposure (head -1 exits after one line), so it now uses bash's own regex match and spawns nothing.

Limitation

This ships without a test. Reproducing the race takes thousands of iterations under CPU saturation, so the test would become the flake it is meant to prevent. The fix is structural rather than probabilistic: with no second process in the pipeline, there is no SIGPIPE to deliver.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual testing (describe below)

make check and make test both pass on current main.

  • Year parity: the new [[ =~ ]] match against the old grep | grep | head -1 chain over 7 inputs, including a 5-digit year, a CopyrightX near-miss, no match at all, and two Copyright lines in one header. Identical results on all 7.
  • Detection is unchanged, checked against a staged probe file: missing SPDX reports (missing SPDX identifier), year 1999 reports (invalid year: 1999), year 2099 reports (invalid year: 2099).
  • make license-add run twice over a header-less file leaves exactly one SPDX-License-Identifier line, and the tree is unchanged on a fully-licensed checkout.
  • Both scripts stay pure ASCII, as they are on main.

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (if applicable)
  • I have signed the CLA

See the Limitation note above on why no test is included.

Related Issues

None open for this; found while rebasing unrelated PRs, where license-check failed on files those branches never touched.

`echo "$header" | grep -q ...` races. grep -q exits on the first match, so
echo can die of SIGPIPE (141); `set -o pipefail` makes that the pipeline's
status, and verify-license.sh reports a header that is present as missing.
Measured on a 410-byte header under CPU load: 4 spurious failures in 3000
iterations, naming a different file each time.

has_header() in add-license.sh has the same race, and there a false negative
makes add_header() prepend a second copyright block to a file that already
has one.

Feed the header through a here-string instead, and read the year with bash's
regex match rather than a grep | grep | head chain that can take SIGPIPE the
same way.
@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

@lizhengfeng101 lizhengfeng101 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

@lizhengfeng101
lizhengfeng101 merged commit 1ae808e into alibaba:main Aug 17, 2026
12 checks passed
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