Skip to content

fix(tools): honor dependency declarations without a colon - #2263

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
jweidenh:fix/overlap-dependencies-without-colon
Oct 10, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
jweidenh:fix/overlap-dependencies-without-colon

Conversation

@jweidenh

@jweidenh jweidenh commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Honor the Depends on #N form documented in CONTRIBUTING when checking pull-request overlaps. The original checker recognized only Depends on: and missed the dependency declared in #1810.

Keep colon declarations authoritative, including their multiline lists. When none is present, accept a plain declaration with inline references separated by commas or and. Stop before other prose so an unrelated reference cannot hide an overlap.

Eight regressions cover both declaration forms, CRLF, list boundaries, unrelated references, checklist precedence and dependency-based overlap suppression in both directions. Register them as pr_overlap_dependencies, alongside the existing tooling tests. Upstream paginated REST retrieval, export detection, UTF-8 report writing and all other test registrations are preserved.

Tested

Current head 5fcac960e8bbe7b7c24e57b3b0a1649adfee116b is one commit directly on main 57f96fd2b36563e8380c603a92b1ef0d9d3a6aa4, with no PR merge commits. This addresses the requested rebase. The rebased tree is identical to the previous branch merged with that main; the parser change and all eight dependency tests are unchanged.

Private self-hosted Linux run 38074957306 passed at that exact head, using reviewed workflow 7b16e6c3d713427d815e7427aa77670d757b609d on lab-github-actions-runner (Ubuntu 24.04 x86-64, GCC 13.3, CMake 3.28.3, Python 3.12.3):

  • Relinker-only Release build passed (146 Ninja steps). All 99 registered CTest groups passed, with zero CTest failures/skips, in 7.37 s. All four pr_overlap groups passed; pr_overlap_dependencies took 0.101 s.
  • All 60 Python tooling unit tests passed with -B -W error in 0.582 s, with a 30-second process deadline.
  • The unmodified registration checker found all 14 test executables registered. Test limits remain 30 seconds; the workflow checks all four overlap registrations and their deadlines.
  • The unrelated hardware_oracle group reports one internal unittest skip out of 12; its non-verbose output does not retain the reason. CTest's zero skipped groups does not mean that subcase ran.
  • Full CTest output, inventory, build, registration-checker and Python logs are retained in the private run; artifact access requires access to the validation repository.

Local macOS arm64, Python 3.12.14: all 60 Python tooling unit tests passed with -B -W error in 0.450 s at the current head, with a 30-second process deadline. Source/description conventions and whitespace checks passed. No local macOS C++ rebuild or local Windows run was performed for this rebase; upstream CI results follow.

Automatic upstream Build run 38075121112 completed successfully for head 5fcac960. Its relinker jobs each verified that exact PR head and merged it into main 9c5d67fbe27518bb5485c34f2fdcfbcb249bd800 before building: Linux 99/99, Windows 95/95, and macOS 92/92 CTest groups passed, including pr_overlap_dependencies on all three platforms. These results apply to those CI merge builds. The full Linux and Windows runtime jobs were skipped, not validated by this run. The latest conventions check also passed.

Earlier validation at 377787416695a7ff4e67ff8996ebae40482e8b33 (before this rebase):

  • Private self-hosted Linux run 38049954047: relinker-only Release build, 49 CTest groups and 50 Python tooling tests passed on Ubuntu 24.04 x86-64/GCC 13.3. The hardware-oracle group reported one internal unittest skip.
  • Local macOS arm64/AppleClang 21: relinker-only build, 44 CTest groups and 50 Python tooling tests passed. The hardware-oracle suite skipped one assembly case because ld.lld was unavailable.
  • Independent Windows validation reported by Mouhand-Kaddo: Windows 11, WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7, CMake 4.3.1 and Ninja; all 50 Python unit tests and all four pr_overlap CTest cases passed on that older standalone head. These are the contributor's results, not a locally reproduced Windows run or validation of the rebased head.

Earlier validation against original base d70b8998: the no-colon parsing and overlap-suppression regressions failed without the fix and passed with it. A read-only replay of 536 PR descriptions changed only #1810's parsed dependency set, from empty to {861}.

The full Linux/Windows runtime jobs from the requested official Build workflow remain outstanding; its automatic relinker matrix is now complete. A collaborator with upstream write access must dispatch the full run for the rebased PR. Local relinker/tooling tests do not replace it. No full runtime build or title/homebrew execution is claimed for this tooling change.

Checklist

  • Based on reviewed upstream 57f96fd2; one commit with no merge commits
  • One topic per PR; follow-ups go in a new PR
  • No code comments added
  • Unimplemented paths and runtime stubs unchanged
  • No investigation notes, logs or images added to the upstream repository
  • No third-party code added
  • Generic behaviour
  • Depends on: none
  • AI-assisted: yes, OpenAI Codex

@github-actions github-actions Bot added build needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 9, 2026
@Mouhand-Kaddo

Copy link
Copy Markdown
Contributor

Additional platform validation on standalone PR head 377787416695a7ff4e67ff8996ebae40482e8b33:

Windows: passed — Windows 11, supported WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7; CMake 4.3.1 / Ninja.

50 Python unit tests plus all 4 pr_overlap CTest cases passed. Commands: python -B -W error -m unittest discover -s tools/tests -v; relinker-only configure with BUILD_TESTING=ON, then ctest -R pr_overlap --no-tests=error.

@MarcZX10

Copy link
Copy Markdown
Contributor

The parser change and its dependency-specific regression cases look well scoped. The local Debian run and the Windows validation report provide useful evidence for the Python tool and its four CTest registrations.

One repository-required check is still missing: the current PR check list contains only Conventions, while the full Linux/Windows Build and Linux/Windows/macOS relinker matrix have not run for head 377787416695a7ff4e67ff8996ebae40482e8b33. CONTRIBUTING.md requires a collaborator to dispatch Actions → Build on the current PR head. Could a collaborator run it before merge? The standalone Windows test report does not include the required full Build/relinker workflow.

@jweidenh
jweidenh marked this pull request as ready for review October 10, 2026 11:55
@oneandonlydean

Copy link
Copy Markdown
Collaborator

The branch has two merge commits from main (95dd51d, 3777874); please rebase onto main and force-push instead. The parser change itself is fine.

@jweidenh
jweidenh force-pushed the fix/overlap-dependencies-without-colon branch from 3777874 to 5fcac96 Compare October 10, 2026 18:16
@oneandonlydean
oneandonlydean merged commit 1f6a7aa into boykopovar:main Oct 10, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants