search

phovea / generator-phovea

Yeoman Generator for Phovea
https://www.npmjs.com/package/generator-phovea
BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

Add Windows support to newly generated workspace #429

Open puehringer opened 4 years ago

puehringer commented 4 years ago

We have found some issues when running the new workspace on a Windows machine (with a MINGW shell).

Issue 1: npm run joins

Currently, we rely on the joining of npm run ... & npm run ... & npm run .... This however fails on windows machines for infinite tasks and only executes the very first command. This effects for example dev-repos:compile:watch, which executes `tsc -w causing an infinite task. Also effected are dev-repos:compile and dev-repos:copy and maybe more...

We already have a solution in place, i.e. by using npm-run-all --parallel ... ... ... instead of the current joining.

Example:

"dev-repos:compile": "npm-run-all --parallel compile:repo1 compile:repo2 compile:repo3" works, whereas the original npm run ... & npm run ... only executes the first task.

Issue 2: if [ -d ]

With Windows, the if [ -d src/... ]; fails because -d is not a valid option. We will have to find an alternative syntax which works for both.

General

I think we should focus on never using bash commands as they are very prone to be system-incompatible, and use npm scripts instead (as we already do with rimraf, npm-run-all, ...). Also, testing should be done on a pure Windows machine without WSL[2].

alexandervervust commented 4 years ago

Related to issue 2: E.g. for assets: Using [ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/ instead of if [ -d src/assets]; ... worked for me

oltionchampari commented 3 years ago

I tested the above issues on Windows and can confirm the bugs.

Issue 1

From my tests npm-run-all --parallel works both in linux and windows.

Issue 2

The library https://www.npmjs.com/package/copyfiles offers a solution that works both on Windows and Linux. For the below scripts 

    "copy-assets": "copyfiles --up 1 src/assets/\"**/*\" src/templates//\"**/*\" dist/ ",
    "copy-styles": "copyfiles --up 1 src/scss/\"**/*\" dist/",
    "copy-app-assets": "copyfiles --up 1 src/\"*\".txt  src/\"*\".html src/\"*\".ejs dist/",

I couldn't find a solution using shx to achieve the same result, since shx does not support the necessary cmds. I have created a branch where these changes can be tested.

thinkh commented 3 years ago

Related to issue 2: E.g. for assets: Using [ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/ instead of if [ -d src/assets]; ... worked for me

@alexandervervust Thanks for your suggestion. I tried this with PowerShell 5.1 and it does not work for me. Which PowerShell version do/did you use?

> taco@5.0.1-SNAPSHOT copy-assets D:\test\taco\taco
> [ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/

Der Befehl "[" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
cp -R src/assets/. dist/assets/
cp: no such file or directory: src/assets/.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! taco@5.0.1-SNAPSHOT copy-assets: `[ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the taco@5.0.1-SNAPSHOT copy-assets script.
thinkh commented 3 years ago

Here is an alternative that works on Windows and Linux using shx in the npm scripts only:

"copy": "npm run copy-assets && npm run copy-styles  && npm run copy-app-assets",
"copy-assets": "shx --verbose cp -R src/assets/. dist/assets/ || echo 'no file copied'",
"copy-app-assets": "(shx --verbose cp src/*.txt dist/ || echo 'no file copied') && (shx --verbose cp src/*.html dist/ || echo 'no file copied') && (shx --verbose cp src/*.ejs dist/ || echo 'no file copied')",
"copy-styles": "shx --verbose cp -R src/scss/. dist/scss || echo 'no file copied'",,

Basically, all commands need to be wrapped in parentheses and require a fallback on error (e.g., || echo 'no file copied').

thinkh commented 3 years ago

Issue 1: npm run joins

Currently, we rely on the joining of npm run ... & npm run ... & npm run .... This however fails on windows machines for infinite tasks and only executes the very first command. This effects for example dev-repos:compile:watch, which executes `tsc -wcausing an infinite task. Also effected aredev-repos:compile and dev-repos:copy and maybe more...

We already have a solution in place, i.e. by using npm-run-all --parallel ... ... ... instead of the current joining.

Example:

"dev-repos:compile": "npm-run-all --parallel compile:repo1 compile:repo2 compile:repo3" works, whereas the original npm run ... & npm run ... only executes the first task.

@puehringer I cannot confirm the error on Windows 10 with PowerShell 5.1. I setup a workspace with TACO and cloned an additional phovea_ui into the workspace to have two dev repos. After adding phovea_ui to the .yo-rc-workspace.json the commands look like this:

    "dev-repos:compile": "npm run compile:taco & npm run compile:phovea_ui",
    "dev-repos:compile:watch": "npm run compile:watch:taco & npm run compile:watch:phovea_ui",
    "dev-repos:copy": "npm run copy:taco & npm run copy:phovea_ui",
    "dev-repos:copy:watch": "npm-watch dev-repos:copy",

When I run npm run dev-repos:compile it runs both repos in "parallel" (using #453). 

PS D:\test\taco> npm run dev-repos:compile

> phovea_workspace@0.0.1 dev-repos:compile D:\test\taco
> npm run compile:taco & npm run compile:phovea_ui

> phovea_workspace@0.0.1 compile:taco D:\test\taco
> cd taco && npm run compile

> taco@5.0.1-SNAPSHOT compile D:\test\taco\taco
> tsc

> taco@5.0.1-SNAPSHOT postcompile D:\test\taco\taco
> npm run copy

> taco@5.0.1-SNAPSHOT copy D:\test\taco\taco
> npm run copy-assets && npm run copy-styles  && npm run copy-app-assets

> taco@5.0.1-SNAPSHOT copy-assets D:\test\taco\taco
> shx --verbose cp -R src/assets/. dist/assets/ || echo 'no file copied'

cp -R src/assets/. dist/assets/
cp: no such file or directory: src/assets/.
'no file copied'

> taco@5.0.1-SNAPSHOT copy-styles D:\test\taco\taco
> shx --verbose cp -R src/scss/. dist/scss || echo 'no file copied'

cp -R src/scss/. dist/scss

> taco@5.0.1-SNAPSHOT copy-app-assets D:\test\taco\taco
> (shx --verbose cp src/*.txt dist/ || echo 'no file copied') && (shx --verbose cp src/*.html dist/ || echo 'no file copied') && (shx --verbose cp src/*.ejs dist/ || echo 'no file copied')

cp src/*.txt dist/
cp src/*.html dist/
cp src/*.ejs dist/

> phovea_workspace@0.0.1 compile:phovea_ui D:\test\taco
> cd phovea_ui && npm run compile

> phovea_ui@5.0.1-SNAPSHOT compile D:\test\taco\phovea_ui
> tsc

> phovea_ui@5.0.1-SNAPSHOT postcompile D:\test\taco\phovea_ui
> npm run copy

> phovea_ui@5.0.1-SNAPSHOT copy D:\test\taco\phovea_ui
> npm run copy-assets && npm run copy-styles

> phovea_ui@5.0.1-SNAPSHOT copy-assets D:\test\taco\phovea_ui
> shx --verbose cp -R src/assets/. dist/assets/ || echo 'no file copied'

cp -R src/assets/. dist/assets/

> phovea_ui@5.0.1-SNAPSHOT copy-styles D:\test\taco\phovea_ui
> shx --verbose cp -R src/scss/. dist/scss || echo 'no file copied'

cp -R src/scss/. dist/scss
PS D:\test\taco>

However, I experience some other issues... (see next comment).

thinkh commented 3 years ago

Issue 3: npm install fails with Maximum call stack size exceeded

npm install works fine when taco is the only (dev) repo in the workspace. However, when adding phovea_ui and run npm install I get the following output:

PS D:\test\taco> npm i

> phovea_workspace@0.0.1 preinstall D:\test\taco
> rimraf node_modules package-lock.json || exit 0

Der Befehl "rimraf" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
npm WARN deprecated popper.js@1.16.1: You can find the new Popper v2 at @popperjs/core, this package is dedicated to the legacy v1
npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated har-validator@5.1.5: this library is no longer supported
npm WARN deprecated fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
npm WARN deprecated fsevents@2.1.3: Please update to v 2.2.x
npm WARN deprecated request-promise-native@1.0.9: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@^2.1.2 (node_modules\jest-haste-map\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.2.1: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@^1.2.7 (node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.13: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.1.2 (node_modules\watchpack\node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN sass-loader@9.0.3 requires a peer of sass@^1.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@9.0.3 requires a peer of fibers@>= 3.1.0 but none is installed. You must install peer dependencies yourself.
npm WARN jest-config@26.6.3 requires a peer of ts-node@>=9.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN jsdom@16.4.0 requires a peer of canvas@^2.5.0 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.4.1 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.4.1 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.

npm ERR! Maximum call stack size exceeded

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\<user>\AppData\Roaming\npm-cache\_logs\2020-12-28T15_52_10_560Z-debug.log

The last part of the log states the following:

25025 warn sass-loader@9.0.3 requires a peer of sass@^1.3.0 but none is installed. You must install peer dependencies yourself.
25026 warn sass-loader@9.0.3 requires a peer of fibers@>= 3.1.0 but none is installed. You must install peer dependencies yourself.
25027 warn jest-config@26.6.3 requires a peer of ts-node@>=9.0.0 but none is installed. You must install peer dependencies yourself.
25028 warn jsdom@16.4.0 requires a peer of canvas@^2.5.0 but none is installed. You must install peer dependencies yourself.
25029 warn ws@7.4.1 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
25030 warn ws@7.4.1 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.
25031 verbose stack RangeError: Maximum call stack size exceeded
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:432:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25031 verbose stack     at failedDependency (C:\Program Files\nodejs\node_modules\npm\lib\install\deps.js:448:9)
25032 verbose cwd D:\test\taco
25033 verbose Windows_NT 10.0.19042
25034 verbose argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "i"
25035 verbose node v12.13.1
25036 verbose npm  v6.12.1
25037 error Maximum call stack size exceeded
25038 verbose exit [ 1, true ]

With trail and error, I found out that the postinstall script is not working with two or more repos in the workspace package.json:

"postinstall": "npm-run-all --parallel delete-dependencies:*",

In my case this matches:

"delete-dependencies:phovea_ui": "cd phovea_ui && npm run delete-dependencies",
"delete-dependencies:taco": "cd taco && npm run delete-dependencies",

npm install succeeds with taco only (i.e., removing the phovea_ui script) or phovea_ui only. But it fails as described above on PowerShell 5.1.

Edit:

Removing the --parallel flag from "npm-run-all --parallel delete-dependencies:*" seems to work also with both delete dependency scripts:

PS D:\test\taco> npm i

> phovea_workspace@0.0.1 preinstall D:\test\taco
> rimraf node_modules package-lock.json || exit 0

Der Befehl "rimraf" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
npm WARN deprecated popper.js@1.16.1: You can find the new Popper v2 at @popperjs/core, this package is dedicated to the legacy v1
npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated har-validator@5.1.5: this library is no longer supported
npm WARN deprecated fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
npm WARN deprecated fsevents@2.1.3: Please update to v 2.2.x
npm WARN deprecated request-promise-native@1.0.9: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142

> node-sass@4.14.1 install D:\test\taco\node_modules\node-sass
> node scripts/install.js

Cached binary found at C:\Users\<user>\AppData\Roaming\npm-cache\node-sass\4.14.1\win32-x64-72_binding.node

> @fortawesome/fontawesome-free@5.15.1 postinstall D:\test\taco\node_modules\@fortawesome\fontawesome-free
> node attribution.js

Font Awesome Free 5.15.1 by @fontawesome - https://fontawesome.com
License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License)

> ejs@2.7.4 postinstall D:\test\taco\node_modules\ejs
> node ./postinstall.js

Thank you for installing EJS: built with the Jake JavaScript build tool (https://jakejs.com/)

> nodemon@1.19.4 postinstall D:\test\taco\node_modules\nodemon
> node bin/postinstall || exit 0

> node-sass@4.14.1 postinstall D:\test\taco\node_modules\node-sass
> node scripts/build.js

Binary found at D:\test\taco\node_modules\node-sass\vendor\win32-x64-72\binding.node
Testing binary
Binary is fine

> phovea_workspace@0.0.1 postinstall D:\test\taco
> npm-run-all delete-dependencies:*

> phovea_workspace@0.0.1 delete-dependencies:phovea_ui D:\test\taco
> cd phovea_ui && npm run delete-dependencies

> phovea_ui@5.0.1-SNAPSHOT delete-dependencies D:\test\taco\phovea_ui
> rimraf node_modules

> phovea_workspace@0.0.1 delete-dependencies:taco D:\test\taco
> cd taco && npm run delete-dependencies

> taco@5.0.1-SNAPSHOT delete-dependencies D:\test\taco\taco
> rimraf node_modules

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@^2.1.2 (node_modules\jest-haste-map\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.2.1: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@^1.2.7 (node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.13: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.1.2 (node_modules\watchpack\node_modules\chokidar\node_modules\fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})
npm WARN sass-loader@9.0.3 requires a peer of sass@^1.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@9.0.3 requires a peer of fibers@>= 3.1.0 but none is installed. You must install peer dependencies yourself.
npm WARN jest-config@26.6.3 requires a peer of ts-node@>=9.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN jsdom@16.4.0 requires a peer of canvas@^2.5.0 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.4.1 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.4.1 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.

added 1460 packages from 1006 contributors and audited 1465 packages in 78.188s
found 1 low severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details
PS D:\test\taco>

I guess that the symbolic links and file permission are working different on Windows and cause the error when running in parallel mode. Especially, when two repositories are depending on each other (e.g., taco depends on phovea_ui).

alexandervervust commented 3 years ago

Related to issue 2: E.g. for assets: Using [ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/ instead of if [ -d src/assets]; ... worked for me

@alexandervervust Thanks for your suggestion. I tried this with PowerShell 5.1 and it does not work for me. Which PowerShell version do/did you use?

> taco@5.0.1-SNAPSHOT copy-assets D:\test\taco\taco
> [ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/

Der Befehl "[" ist entweder falsch geschrieben oder
konnte nicht gefunden werden.
cp -R src/assets/. dist/assets/
cp: no such file or directory: src/assets/.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! taco@5.0.1-SNAPSHOT copy-assets: `[ ! -d src/assets ] || shx --verbose cp -R src/assets/. dist/assets/`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the taco@5.0.1-SNAPSHOT copy-assets script.

@thinkh I use cmder. I did not test it with PowerShell. You're solution to add || echo 'no file copied' is definitely better.

thinkh commented 3 years ago

@alexandervervust Thanks for the info.