Repository navigation
fix(scripts): resolve argv[1] before the entry-module check (macOS prepush_focus red) - #49
Closed
Zelys-DFKH wants to merge 2 commits into
Closed
Zelys-DFKH wants to merge 2 commits into
Zelys-DFKH wants to merge 2 commits into
Conversation
…inked folder still runs the CLI
The pre-push focus tests failed only on macOS: the focus list came back empty and the note read "recorded commit is not an ancestor" with a blank sha. The cause is not bash 3.2. prepush-failed.mjs, prepush-tree-readers.mjs, test-durations.mjs and lock-commit-subject.mjs decided they were the entry module with import.meta.url === pathToFileURL(process.argv[1]).href. Node resolves the entry module's symlinks for import.meta.url but leaves argv[1] as written, and macOS temp folders sit behind a symlink (/var and /tmp point into /private), so the comparison was false, the CLI branch was skipped, and the script exited 0 with no output. `record` wrote no store, `read` printed nothing, and focus_decide took an empty record as a blank sha. The same bug let the commit-msg lock check pass any subject when started through a symlinked path.
All four now compare pathToFileURL(realpathSync(process.argv[1])).
Tests: prepush_focus.test.ts now reaches its sandbox through a directory junction/symlink, scripts_main_through_symlink.test.ts runs each script through a link, and guards/entry_module_check_resolves_symlinks.test.ts rejects a bare comparison. Against the old scripts on Windows the focus tests failed with the CI messages ("expected [] to deeply equal [ 'src/a.ts', ... ]" and "win leg full: recorded commit is not an ancestor of this push"); with the fix they pass.
Collaborator
Author
|
Closing without merging; branch macfix2 is left for landing. |
…h a resolve fallback The first fix called realpathSync(process.argv[1]) with no fallback, so an entry path that does not exist (node -e "import(...)" foo) crashed at import, and in lock-commit-subject that crash would fail the commit-msg gate. scripts/is-main-module.mjs now resolves both sides with plain realpathSync (the ESM loader's own form, which matches import.meta.url on Windows with a lowercase drive letter where .native does not) and falls back to path.resolve, so a missing entry is a plain "not main". Every entry check under scripts/ uses it: the four prepush and lock scripts, the five scripts that carried a private realOrResolved copy plus parser-fingerprint, the two that matched on file name, and the four eval-*.ts scripts that compared path.resolve(argv[1]), which does not follow symlinks (tsx reports the real path in import.meta.url, checked by running one through a junction). verify-native-dist.mjs keeps its own copy because it ships in the npm package and is copied alone by its tests. Tests: scripts_main_through_symlink.test.ts now asserts each script's real output through a link (stderr and stdout, not only the exit status), imports each script with a nonexistent entry path, and unit-tests the helper. The guard reads the whole file with comments stripped, covers .mjs, .js, .cjs and .ts, and requires the helper wherever argv[1] sits beside import.meta.url or __filename. The junction tests skip, and say so, on a machine that cannot create one. Red runs: the guard failed on eval-gate.ts and prepush-failed.mjs with the old comparisons put back, and six tests failed with ENOENT on the helper without its fallback.
Collaborator
Author
|
Green on all jobs; closing unmerged, branch macfix2 left for landing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft to run test-macos. The pre-push focus tests failed only on macOS because four scripts compared import.meta.url with the unresolved argv[1]; macOS temp folders are symlinks, so the CLI branch was skipped silently. See the commit message for the evidence. Close without merging.