GeezerVison is showing github.com — fix(schematics): use a POSIX path for the Cloud Run main.js entry by fr-esco · Pull Request #3274 · angular/angularfire
Conversation
fr-esco
commented
Oct 7, 2022
•
edited by armando-navarro
Loading
Uh oh!
There was an error while loading. Please reload this page.
Description
Running ng deploy from Windows with the Cloud Run SSR option writes the generated run/package.json entry paths with backslashes ("main": "dist\\my-project\\server\\main.js"), which the Linux-based Cloud Run runtime cannot resolve. This PR builds that path with forward slashes explicitly, since the target runtime is always Linux regardless of the deploying machine.
The branch originally also added shell: true to the deploy runner, fixing spawn gcloud ENOENT on Windows. That half was dropped when the branch was brought current with main:
- The package-manager commands now route through cross-spawn, which resolves the platform executable on Windows. The gcloud calls still use plain spawn, so the
spawn gcloud ENOENT half of spawn gcloud ENOENT #3098 is not fixed by this PR.
- The deploy hardening work relies on arguments not passing through shell parsing, so reintroducing a shell would undo part of it.
Refs #3098
Original author notes (2022):
Checklist
- Issue number for this PR: spawn gcloud ENOENT #3098
- Docs included?: No
- Test units included?: No
- In a clean directory,
yarn install, yarn test run successfully? Yes
Description
Running the deploy schematic on Windows:
- Child processes that need
gcloud cannot properly locate it;
- The path to
main.js is printed escaped by \\ instead of /, even though target runtime is Linux based.
Code sample
N. A.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fr-esco Thank you so much and our apologies for taking so long to get to it. Since this is an older PR and there are a lot of merging PRs in flight it's going to take a bit to get this tested and merged so hang tight.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Resolves the spawnAsync conflict by taking main's side: the runner now
receives command and args separately through cross-spawn, which handles
the Windows executable resolution that shell: true addressed, and a
shell would reintroduce the argument parsing the injection fixes
removed. The main.js path fix carries through unchanged.
|
Sorry this sat for so long. I picked this up to land it with your authorship: I merged current main into your branch, and the two halves of your fix went different ways.
- The
main.js path fix carries through unchanged and is the whole diff now. It is still a real bug on today's main.
- The
shell: true half was dropped in the merge. The recent injection hardening moved the package-manager commands to cross-spawn and depends on arguments not being shell-parsed. The gcloud spawns still bypass cross-spawn today, so the ENOENT you originally hit is not fixed by this PR and needs its own follow-up.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
armando-navarro
added a commit
that referenced
this pull request
Oct 10, 2026
)
Running ng deploy from Windows with the Cloud Run SSR option wrote the
generated run/package.json entry paths with backslashes, which the
Linux-based Cloud Run runtime cannot resolve. The manifest path is now
built with forward slashes explicitly, since the target runtime is
Linux regardless of the deploying machine.
Fixes #3098
Co-authored-by: Armando Navarro <[email protected]>
(cherry picked from commit 73acea7)
Reviewers
+1 more reviewer
Reviewers whose approvals may not affect merge requirements
Uh oh!
There was an error while loading. Please reload this page.
Labels
backport: 20.1.x
Cherry-pick onto the 20.1.x branch for a 20.1 patch release
bump: patch
Size of the change: a fix or docs change that reaches users
comp: schematics
ng add / deploy schematics (src/schematics).
type: bug
Defect: expected behavior doesn't happen.
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|