build: fix build scripts on macOS #33854
Conversation
|
You can preview 56f3074 at https://pr33854-56f3074.ngbuilds.io/. |
gkalpak
added
area: build & ci
| const srcDir = `${bazelBin}/packages/${pkg}/npm_package`; | ||
| const destDir = `${absDestPath}/${pkg}`; | ||
|
|
||
| if (existsSync(srcDir) && statSync(srcDir).isDirectory()) { |
There was a problem hiding this comment.
Optional idea: You could also use shelljs.test('-d', srcDir) (same for other calls to existsSync).
There was a problem hiding this comment.
Good catch, I had forgitten about test().
(Too bad I saw this too late 😞)
There was a problem hiding this comment.
I ended up making the change in #33895 (because it keeps the code closer to the original bash script and might be easier for people to follow) 😃
|
@gkalpak did you say that this change came with a significant performance penalty for running bazel? can you please clarify / share details? |
|
ah.. nevermind. I see that the issue is now resolved. |
|
@gkalpak can you please rebase? |
In angular#33823, `scripts/package-builds.sh` (which is used by both `build-packages-dist.sh` and `build-ivy-npm-packages.sh`) was updated to use `realpath`. It turns out that `realpath` does not exist on macOS, so the build scripts do not work there. In order to fix this (and also reduce the likelihood of introducing similar issues in the future), this commit changes these bash scripts to Node.js scripts (using [ShellJS](https://github.com/shelljs/shelljs) for a cross-platform implementation of Unix shell commands where necessary).
IgorMinar
force-pushed
the
build-fix-build-on-mac
branch
from
56f3074
to
659d835
Compare
|
@gkalpak nvm, I rebased it for you |
|
You can preview 659d835 at https://pr33854-659d835.ngbuilds.io/. |
In #33823, `scripts/package-builds.sh` (which is used by both `build-packages-dist.sh` and `build-ivy-npm-packages.sh`) was updated to use `realpath`. It turns out that `realpath` does not exist on macOS, so the build scripts do not work there. In order to fix this (and also reduce the likelihood of introducing similar issues in the future), this commit changes these bash scripts to Node.js scripts (using [ShellJS](https://github.com/shelljs/shelljs) for a cross-platform implementation of Unix shell commands where necessary). PR Close #33854
This makes the JS script more similar to the original bash script it replaced (see angular#33854) and will be easier to follow for people that are less familiar with Node.js scripting. Discussed in: angular#33854 (comment)
This makes the JS script more similar to the original bash script it replaced (see #33854) and will be easier to follow for people that are less familiar with Node.js scripting. Discussed in: #33854 (comment) PR Close #33895
This makes the JS script more similar to the original bash script it replaced (see #33854) and will be easier to follow for people that are less familiar with Node.js scripting. Discussed in: #33854 (comment) PR Close #33895
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In #33823,
scripts/package-builds.sh(which is used by bothbuild-packages-dist.shandbuild-ivy-npm-packages.sh) was updated to userealpath. It turns out thatrealpathdoes not exist on macOS, so the build scripts do not work there.In order to fix this (and also reduce the likelihood of introducing similar issues in the future), this commit changes these bash scripts to Node.js scripts (using ShellJS for a cross-platform implementation of Unix shell commands where necessary).
This is the same as #33840. Opened a new PR, because #33840 was cluttered with irrelevant preview comments and force-pushes, while I was investigating.