Skip to content

gate: fix generated target name collisions (part of #9237) - #9279

Merged
TrapsterDK merged 2 commits into
mainfrom
fix-9237-typecheck-names
Nov 7, 2026
Merged

TrapsterDK merged 2 commits into
mainfrom
fix-9237-typecheck-names

Conversation

@TrapsterDK

@TrapsterDK TrapsterDK commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #9237.

  • bad.mdx renamed to page.mdx; rule_names binary renamed rule_names_bin (name collisions).
  • Deleted the six unread Cargo.lock files (cli/cli, cli/ide, cli/update, cli/worktree, quality/adapter, tools/testing), as in repo: delete the unused, stale cli/cli/Cargo.lock #5466.
  • cli/adopt lock_hubs: lock discovery skips nested modules (families/js has its own MODULE.bazel), so it no longer wires their pnpm-lock into the root. New test for nested modules.

The bulk dx generate BUILD edits were reverted: they dropped deps the code uses. Those are generator bugs, filed as #9295, #9296, #9297, #9298. #9237 stays open until they are fixed and generate is clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7

@TrapsterDK
TrapsterDK marked this pull request as ready for review November 7, 2026 12:37
@TrapsterDK TrapsterDK added the needs-review Touches review paths; needs a second-session Review: comment label Nov 7, 2026
@TrapsterDK

TrapsterDK commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review: changes requested

The renames (bad.mdx to page.mdx with its expected output, rule_names to rule_names_bin with the CI and error message updated), the lock_hubs nested-module skip, and its test are fine. The new test would fail without the fix. The six deleted Cargo.lock files are not referenced anywhere, so deleting them repeats #5466. The problem is that the bulk dx generate output was applied without checking it. It drops deps that the sources still use and renames a target without updating its only caller:

  1. python/rules/BUILD.bazel:256: generate renamed bench_plugin to dx_bench_plugin, but python/rules/defs.bzl:143 still adds Label("//python/rules:bench_plugin"). Every python_benchmark fails analysis, including python/tests/fixtures/hello:45 and all consumer benchmarks.
  2. cli/cli/tests/agents/BUILD.bazel:52: this drops //cli/path:dx_path, but tests/dev_launcher.rs:7 calls dx_path::state::launcher_dir without a cfg gate. dev_launcher_test will not compile on any platform.
  3. libs/launcher/BUILD.bazel:26: this drops //tools/testing:dx_testing, but src/launch_tests.rs:194 and :239 use it under #[cfg(unix)]. exec_launcher_test will not compile on Linux or macOS.
  4. python/tests/fixtures/native_ext/BUILD.bazel:42: this drops :dxlinked, :dxnative and the Windows runfiles dep, but native_ext_test.py:10-11 imports both. The test is Windows-only, so a Linux generate run never sees the ImportError.
  5. python/tests/BUILD.bazel:14: this drops @rust_sdist_pypi//rpds_py, but rust_sdist_test.py:46 and :70 import rpds, which is what the test is checking.
  6. haskell/tools/BUILD.bazel:35: the new deps_resolve_test runs the same file that deps_resolve_unit_test (py_unittest_test) already runs.

Suggested fix: restore these deps and the bench_plugin reference, or update defs.bzl. Then fix the generator, or mark the deps with # keep, so "0 changes" holds without breaking builds. #2 and #3 show up in bazel build //cli/cli/tests/agents/... //libs/launcher/.... #1 needs an explicit target such as //python/tests/fixtures/hello:<bench> because benchmarks are tagged manual.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7

TrapsterDK and others added 2 commits November 7, 2026 13:47
…get names do not collide (part of #9237)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7
…lock files, skip nested modules for lock hubs (part of #9237)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7
@TrapsterDK

Copy link
Copy Markdown
Collaborator Author

Addressed the review:

Tests (with #9282 applied locally, because main does not load without it): //cli/adopt:all //tools/docs/rule_names:all //python/rules:all //libs/launcher:all //haskell/tools:all pass. //cli/cli/tests/agents:dev_launcher_test compiles, but 2 cases fail (an_execroot_with_an_unusable_marker_is_never_entered, a_marker_naming_another_execroot_is_not_followed: the real dx status ran instead of the stub). That package is identical to main. Builds pass for //python/tests/fixtures/hello:parse_bench, //python/tests:rust_sdist_test and //python/tests/fixtures/native_ext:native_ext_test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7

@TrapsterDK

Copy link
Copy Markdown
Collaborator Author

Review: approve

Re-reviewed f5b2253 (12 files). All six findings from my earlier review are resolved because the bulk generate edits were reverted. The generator bugs are filed as #9295 to #9298.

  • families/js/javascript/tests/typecheck/BUILD.bazel:30 and mixed.stdout.expected:3-5: the rename to page.mdx is applied consistently. No reference to bad.mdx is left in the package. I could not confirm from the code that tsc keeps the vue-then-mdx order in the expected output after the rename, so please confirm mixed_stdout_test passes.
  • tools/docs/rule_names/BUILD.bazel:5 and rule_names.py:92: no //tools/docs/rule_names reference outside the package uses the old binary name. CI only runs :rule_names_test.
  • cli/adopt/src/lock_hubs.rs:335: nested modules are now skipped. The new test at :703 would fail without the fix, because sub/Cargo.lock would produce a "not wired" note.
  • The six deleted Cargo.lock files have no references in any .bazel, .bzl or MODULE file, so deleting them repeats repo: delete the unused, stale cli/cli/Cargo.lock #5466.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7

@TrapsterDK

Copy link
Copy Markdown
Collaborator Author

//javascript/tests/typecheck:mixed_stdout_test passes at f5b2253. I ran it in the families/js workspace with #9282 applied locally and not committed. The passing result came from the disk cache.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xut2hzpb4qWNZKpDCrwUz7

@TrapsterDK
TrapsterDK marked this pull request as draft November 7, 2026 12:57
@TrapsterDK
TrapsterDK marked this pull request as ready for review November 7, 2026 12:57
@TrapsterDK
TrapsterDK marked this pull request as draft November 7, 2026 13:02
@TrapsterDK
TrapsterDK marked this pull request as ready for review November 7, 2026 13:44
@TrapsterDK
TrapsterDK merged commit e98562f into main Nov 7, 2026
@TrapsterDK
TrapsterDK deleted the fix-9237-typecheck-names branch November 7, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review Touches review paths; needs a second-session Review: comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant