Skip to content

chore: make dev tooling cross-platform - #173

Open
freema wants to merge 3 commits into
mainfrom
chore/cross-platform-dev-tooling
Open

chore: make dev tooling cross-platform#173
freema wants to merge 3 commits into
mainfrom
chore/cross-platform-dev-tooling

Conversation

@freema

@freema freema commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Split out of #169, which @juliandescottes asked me to break into smaller pieces. Dev tooling only, no src/ changes.

  • npm run clean shelled out to rm -rf, which does not exist in the cmd.exe that npm runs scripts through. That also broke prepublishOnly, so npm publish could not run on Windows at all.
  • npm run build:mcpb shelled out to mkdir -p for the same reason. cmd.exe does not fail on that one outright: its mkdir reads -p as a second directory name, so the build quietly leaves a stray -p directory in the repo root, and a later run with -p present and dist-mcpb gone exits 1 and takes execSync down with it.
  • npm run test:mozlog hardcoded /tmp/firefox-mozlog-test.log, which resolves to C:\tmp on Windows and does not exist, so the script died with ENOENT before launching Firefox. It now uses the platform temp directory, as the sibling scripts already do.
  • The Taskfile's clean task shelled out to rm -rf.

Two things I changed relative to #169: the .gitattributes part is gone, since #164 already landed it, and I kept the test:integration:win entry, because the runner it points at is only removed by the integration-suite commit that stays in #169.

The last commit is a YAML fix rather than a portability one, and it is worth a word. A plain YAML scalar cannot contain a colon followed by a space, and the replacement clean command has two of them inside { recursive: true, force: true }, so task refused to parse the file and reported mapping values are not allowed in this context. Dropping the spaces after those colons fixes it. While confirming that, I found the same problem already on main, in the bump-version description:

desc: Bump version in package.json and plugin.json (usage: task bump-version -- 0.9.8)

task cannot parse the Taskfile today because of that line, so clean would have stayed unreachable even after this PR. I quoted it here. Happy to split that one line out if you would rather keep this PR to portability alone.

Verified on Windows 11, Node 22.22.0. npm run clean, npm run build:mcpb and npm publish --dry-run (which runs prepublishOnly) all fail on main and pass here. npm run test:mozlog runs end to end and captures its log under %TEMP%. task --list now parses the whole file, and task clean removes dist, node_modules and package-lock.json as intended.

freema added 2 commits August 25, 2026 20:02
`npm run test:mozlog` hardcoded /tmp/firefox-mozlog-test.log, which resolves
to C:\tmp on Windows and does not exist, so the script died with ENOENT
before launching Firefox. Use the platform temp directory, as the sibling
scripts already do.

The Taskfile's clean task shelled out to `rm -rf`, unavailable on Windows.
Two failures a Windows contributor hits before writing any code:

- `npm run clean` shelled out to `rm -rf`, which does not exist in the
  cmd.exe that npm runs scripts through. That also broke prepublishOnly, so
  `npm publish` could not run on Windows at all.

- `npm run build:mcpb` shelled out to `mkdir -p` for the same reason.
@freema
freema requested a review from juliandescottes August 25, 2026 18:10
A plain YAML scalar cannot contain a colon followed by a space, so task
refused to parse the file at all and every task was unavailable.

The clean command hit this through the rmSync options object; dropping the
spaces after the colons is enough. The bump-version description hit it
through its "(usage: ...)" parenthetical, which predates this branch, and
clean stays unreachable while that line is unquoted.
@freema
freema marked this pull request as ready for review August 25, 2026 20:27
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.

1 participant