Repository navigation
Conversation
Apache 2.0 §4(a) requires LICENSE in every distributed artifact and §4(d) requires NOTICE to propagate to derivatives. PR #2593 covered the base-template artifact via prepare-base.sh but left the three sibling prepare scripts (core, cli, starter-app) bundling neither file. Mirror the pattern across all four release-artifact prepare scripts and pin the contract with a source-level regression guard. Fixes #2596 Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com>
…in-core-cli-starter-ap
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR correctly addresses the Apache 2.0 §4(a)/§4(d) compliance gap by mirroring the cp LICENSE / cp NOTICE pattern from prepare-base.sh into the three sibling prepare scripts. The shell-script additions and CHANGELOG entry are clean. However, the regression-guard spec (buildArtifactLicenseSpec.cfc) uses POSIX bracket-expression syntax ([[:space:]]) that is not supported by Java's java.util.regex engine — used by Lucee, Adobe CF, and BoxLang alike. On Adobe CF 2023/2025 and BoxLang the regex compiles but silently fails to match actual whitespace, causing all 8 spec assertions to fail even when the scripts are correct. This is a cross-engine correctness defect that should be fixed before merge. (Verdict would be request changes but GitHub prevents a bot from requesting changes on its own PR; flagging here instead.)
Cross-engine
vendor/wheels/tests/specs/buildArtifactLicenseSpec.cfc, lines 39 and 50 — POSIX character class not supported in Java regex
// line 39
"cp[[:space:]]+LICENSE[[:space:]]+""\$\{BUILD_DIR\}",
// line 50
"cp[[:space:]]+NOTICE[[:space:]]+""\$\{BUILD_DIR\}",[[:space:]] is POSIX ERE bracket-expression syntax. Java's java.util.regex (the engine under Lucee, Adobe CF 2023/2025, and BoxLang) does not support POSIX named character classes. It treats [[:space:]] as a character class matching the literal characters :, s, p, a, c, e — not whitespace. Because none of those characters appear between cp and LICENSE in any of the prepare scripts, reFindNoCase returns 0 for every call, hasLicense / hasNotice are always false, and all 8 expect(…).toBeTrue() assertions fail on Adobe CF and BoxLang engines. On Lucee 7 the tests may pass if its runtime happens to accept POSIX syntax, but that is engine-specific behaviour the CI matrix cannot rely on across all engines.
The established pattern in this repo for whitespace in regex is \s — confirmed by vendor/wheels/tests/specs/security/McpCommandInjectionSpec.cfc line 93:
expect(reFind("^wheels\s", "wheels test run") > 0).toBeTrue();Option A — minimal fix, stay with regex:
var hasLicense = reFindNoCase(
"cp\s+LICENSE\s+""\$\{BUILD_DIR\}",
src
) > 0;Option B — preferred, follow sibling spec pattern (no regex needed):
buildInfoSpec.cfc and routesViewBrowserFixturesSpec.cfc — the two specs cited in the comment as the model for this one — use findNoCase/toInclude, not regex. That simpler approach works on every engine with zero ambiguity:
it("copies LICENSE into its BUILD_DIR", () => {
var src = fileRead(repoRoot & "/" & relPath);
expect(findNoCase('cp LICENSE "${BUILD_DIR}', src)).toBeGT(
0,
relPath & " must bundle LICENSE into its build artifact for Apache 2.0 §4(a) compliance."
);
});
it("copies NOTICE into its BUILD_DIR", () => {
var src = fileRead(repoRoot & "/" & relPath);
expect(findNoCase('cp NOTICE "${BUILD_DIR}', src)).toBeGT(
0,
relPath & " must bundle NOTICE into its build artifact for Apache 2.0 §4(d) propagation."
);
});Correctness
The shell-script additions are correct and match the compliance intent. The comment added above each cp block clearly states the legal rationale (§4(a)/§4(d)), and the placement relative to the rsync/cp -r block is consistent across all four scripts. No issues here.
The CHANGELOG entry under [Unreleased] → Fixed is accurate and well-worded.
Commits
chore: bundle LICENSE and NOTICE in core/cli/starter-app build artifacts
Type chore, subject within 100 chars, DCO sign-off present — passes commitlint. The body explains the compliance context. No issues.
Tests
Once the POSIX character class is replaced with \s or findNoCase, the spec structure is sound: the IIFE (function(relPath) { ... })(rel) correctly captures the loop variable to prevent all four describe blocks from aliasing to the last script path. Eight assertions across four scripts for LICENSE + NOTICE each — good coverage for the regression contract.
One note for the follow-up: no CI check here confirms that the actual build artifacts produced by prepare-*.sh contain the files at runtime (the test plan's unchecked CI item). That gap is acceptable for now since a shell-level test would require a build context, but it should stay on the radar.
Wheels Bot — Reviewer B (round 1)A's review is technically sound. The POSIX character class finding is correct, well-evidenced, and the two recommended fixes are both actionable. No sycophancy, no false positives, no missed issues. Aligning with A: changes needed before merge. SycophancyNone detected. A explicitly withholds approval and flags a defect requiring a fix. False positivesNone detected. A's core claim — that A's citation of Missed issuesNone detected. The Verdict alignmentA's ConvergenceAligned with A. The single defect is well-defined: replace |
Wheels Bot — Address Review (no-op: PR already merged)The convergence loop arrived too late for this PR. Reviewer B's Outstanding consensus finding (now in
|
Pick up #2597 (LICENSE/NOTICE bundling for core/cli/starter-app artifacts) which landed on develop while #2600 was in review. Trivial conflict in CHANGELOG.md — both branches added an entry to the top of [Unreleased] > Fixed; both entries kept, ours stays first. Signed-off-by: Peter Amiri <peter@alurium.com> # Conflicts: # CHANGELOG.md
Summary
Apache 2.0 §4(a) requires every distributed artifact to ship
LICENSE, and §4(d) requiresNOTICEto propagate to derivatives. #2593 covered that bundling for the base-template artifact viatools/build/scripts/prepare-base.sh. The three sibling prepare scripts —prepare-core.sh,prepare-cli.sh,prepare-starterApp.sh— were left bundling neither file, so thewheels-core,wheels-cli, andwheels-starter-appartifacts shipped without the license text or attribution notices.This PR mirrors the
cp LICENSE/cp NOTICEpattern fromprepare-base.shinto the other three scripts so every release artifact is Apache 2.0 §4(a)/§4(d) compliant. A new source-level regression guard,vendor/wheels/tests/specs/buildArtifactLicenseSpec.cfc, pins the contract for all four scripts so the bundling can't drift back.Related Issue
Closes #2596
Type of Change
Feature Completeness Checklist
Signed-off-by: wheels-bot[bot].vendor/wheels/tests/specs/buildArtifactLicenseSpec.cfcasserts each of the fourprepare-*.shscripts contains bothcp LICENSE "${BUILD_DIR}…"andcp NOTICE "${BUILD_DIR}…". Failing-then-passing locally: 6 fail → 0 fail after the implementation lands; 8/8 pass.bot-update-docs.ymlworkflow will add follow-up doc commits if it identifies anything worth surfacing..ai/wheels/change required (no framework-layer surface area).[Unreleased]→### Fixed.Test Plan
vendor/wheels/tests/specs/buildArtifactLicenseSpec.cfcexercises every release-artifact prepare script:prepare-base.sh,prepare-core.sh,prepare-cli.sh,prepare-starterApp.sh. Asserted on the source so it runs without spawning a real build context.compat-matrixrun stays green and that theBuild & Publish Releasejob, when it next runs, dropsLICENSE+NOTICEnext to the existing files inside each ofbuild-wheels-core/wheels/,build-wheels-cli/wheels-cli/, andbuild-wheels-starterApp/.Screenshots / Output