build: run the build when installed from git - #47
Conversation
`dist/` is gitignored and `main` points at `./dist/index.js`, so installing this package from a git URL produced a package whose entry point did not exist. npm installs devDependencies and runs `prepare` for git dependencies, and typescript is already a devDependency, so the build just needs to be wired to that lifecycle hook. Installing from the npm registry is unaffected: the published tarball already ships `dist/`, and `prepare` does not run for registry installs of a packed tarball.
`prepare` runs `build`, and `build` chained a POSIX-only `copy-assets`: mkdir -p dist/transitions/shaders && cp src/transitions/shaders/*.glsl ... npm runs lifecycle scripts through cmd.exe on Windows, where `cp` does not exist and `mkdir -p` creates a directory literally named `-p`. Before this PR that only broke an explicit `npm run build`; with `prepare` it breaks plain `npm install` too — including the very command this PR exists to fix, `npm i git+https://github.com/shreyaskarnik/argo`, which would now fail during prepare rather than at runtime. The README documents Windows (choco install ffmpeg) and every CI job is ubuntu-latest, so nothing here would have caught it. A script file rather than an inline `node -e`, because quoting is the same hazard one level down: cmd.exe and sh disagree about nested quotes, which is what made the original break. It copies only *.glsl. A recursive directory copy — the obvious one-liner — also drags index.ts and a README into dist/, which the old glob did not. Verified: build emits 5 shaders, `npm pack` carries all 5, `npm run prepare` reproduces them from a clean dist/, 774 tests pass.
|
Thanks @Joilence — good find, and the diagnosis in the description is exactly right. Also verified the things a one-line
I pushed one commit to your branch (d6119e0) rather than sending you round again — hope that's alright.
npm runs lifecycle scripts through Two details in the replacement worth flagging, since both are easy to get wrong:
One thing worth a line in the README, not a change request. The presence of a Verified on the updated branch: build emits 5 shaders, |
`prepare` makes copy-assets run inside `npm pack`, and the pack-smoke job captures that command's stdout as the tarball filename: echo "TARBALL=$(pwd)/$(npm pack --silent)" >> "$GITHUB_ENV" The progress line landed on stdout, so the capture became two lines and the step failed with: ##[error]Invalid format 'argo-video-cli-0.39.1.tgz' stderr keeps the build feedback without contaminating a stream something else parses. `--silent` suppresses npm's own output, not a lifecycle script's, so any future addition to this path has the same constraint. Worth noting the pack-smoke job caught this on its first real outing — the failure only exists once `prepare` is wired, which is what that job is for.
|
Correction to my last comment — that ended up being two commits, not one. The first push broke My replacement script logged a progress line to stdout. Once Moved to stderr in the follow-up commit. Two things I'd note from that, neither a change request:
Sorry for the extra churn on your branch — the fix is entirely on my side of it, not yours. |
Upstream merged shreyaskarnik#47 and shreyaskarnik#48, so dev's own drafts of the Gemini PCM work are superseded. All five tts files resolve to upstream: diffing the two sides showed every line unique to dev was something shreyaskarnik#48 had since fixed, namely the `gemini-2.5-flash` default that answers an AUDIO request with 400, the `describe.runIf` guard that skips silently instead of failing, and `expect(header.audioFormat).toBe(3)`, which ffmpeg 6 fails because it tags float32 as EXTENSIBLE. package.json takes upstream's `node scripts/copy-assets.mjs` over the shell one-liner, which upstream introduced alongside the `prepare` hook from shreyaskarnik#47: `prepare` now runs on the installer's machine, where `mkdir -p` and `cp` are not a given.
Why
mainpoints at./dist/index.js, butdist/is gitignored. A git install therefore has no entry point:Installs from npm are fine, since the tarball ships
dist/viafiles. That is the only path CI covers.What
+ "prepare": "npm run build",npm runs
preparefor git dependencies after installing devDependencies, andtypescriptis already one. Tarball installs skip it, so npm users see no change.Test
Same git URL both ways: without the fix
dist/is missing and the CLI crashes, with itargo --versionprints0.39.1. Suite: 753 pass.Note:
npm installhere now builds too, so CI builds twice.