Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yarn rwt copy depends on rsync, which isn't available on Windows #679

Closed
Tobbe opened this issue Jun 10, 2020 · 13 comments · Fixed by #694
Closed

yarn rwt copy depends on rsync, which isn't available on Windows #679

Tobbe opened this issue Jun 10, 2020 · 13 comments · Fixed by #694

Comments

@Tobbe
Copy link
Member

Tobbe commented Jun 10, 2020

redwood@d51ade08118c17459cebcdb496197ea52485364a
windows 10
GitBash (mintty 3.1.6)
yarn v1.22.4

I tried copying over a modified version of the Redwood Framework to a Redwood App, to test some changes I've made. But trying to run yarn rwt copy I get an error message about rsync not being available. Which makes sense, since I'm on Windows. See log below

$ yarn rwt copy ../redwood
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\test-store\node_modules\.bin\rwt copy ../redwood
Redwood Framework Path:  C:\Users\tobbe\dev\redwood\redwood
'rsync' is not recognized as an internal or external command,
operable program or batch file.
(node:35440) UnhandledPromiseRejectionWarning: Error: Command failed with ENOENT: rsync -rtvu --delete 'C:\Users\tobbe\dev\redwood\redwood/packages/' 'C:\Users\tobbe\dev\redwood\test-store/node_modules/@redwoodjs/'
spawn rsync ENOENT
    at notFoundError (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:6:26)
    at verifyENOENT (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:40:16)
    at ChildProcess.cp.emit (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:27:25)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
(node:35440) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:35440) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Done in 1.23s.
@Tobbe
Copy link
Member Author

Tobbe commented Jun 10, 2020

rsync for git-for-windows can be downloaded from http://repo.msys2.org/msys/x86_64/ see more here: https://serverfault.com/questions/310337/using-rsync-from-msysgit-for-binary-files/872557#872557

Installing that however, only got me a tiny step further

I now have this error

$ yarn rwt copy ../redwood
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\test-store\node_modules\.bin\rwt copy ../redwood
Redwood Framework Path:  C:\Users\tobbe\dev\redwood\redwood
The source and destination cannot both be remote.
rsync error: syntax or usage error (code 1) at main.c(1303) [Receiver=3.1.3]
(node:35616) UnhandledPromiseRejectionWarning: Error: Command failed with ENOENT: rsync -rtvu --delete 'C:\Users\tobbe\dev\redwood\redwood/packages/' 'C:\Users\tobbe\dev\redwood\test-store/node_modules/@redwoodjs/'
spawn rsync ENOENT
    at notFoundError (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:6:26)
    at verifyENOENT (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:40:16)
    at ChildProcess.cp.emit (C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\cli\node_modules\cross-spawn\lib\enoent.js:27:25)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
(node:35616) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:35616) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Done in 0.95s.

@thedavidprice
Copy link
Contributor

@Tobbe Admittedly, we never tested this workflow and CLI command on Windows. Could definitely use your help here if you'd be available, which doesn't mean tackling this alone. @peterp and I are prioritizing Windows support right now -- helping us diagnose so we can fix is a huge help in and of itself.

Have you looked at the code for the command yet? It's possible to add the rysnc package as well as swap it for something that offers better cross-platform support (if applicable):
https://github.com/redwoodjs/redwood/blob/master/packages/cli/src/redwood-tools.js

The other tools in the CONTRIBUTING workflow are the tasks here:
https://github.com/redwoodjs/redwood/tree/master/tasks

Not sure if you've tried them out yet but we'd also like to make sure they're working for Windows as well.

@thedavidprice thedavidprice added bug/confirmed We have confirmed this is a bug topic/cli topic/windows labels Jun 11, 2020
@Tobbe
Copy link
Member Author

Tobbe commented Jun 11, 2020

Could be worth it to try with https://github.com/mattijs/node-rsync as the last commit message (except the version bump) is "Update rsync.js with Windows support", which does sound promising 🙂

If that doesn't work out we could try this: https://github.com/jprichardson/node-fs-extra/blob/master/docs/copy.md

I noticed you use rsync -rtvu --delete. -r is the default behavior (special handling for the top dir might be needed), -t is handled by preserveTimestamps, -v isn't super important, at least not for a poc, -u can be implemented by the use of the filter function. --delete isn't as straight forward. Not sure how to best go about implementing that. Is it really needed?

If no one beats me to it I can probably play around a bit with it this weekend

@thedavidprice
Copy link
Contributor

Looping in @peterp on this as he authored the command and could better address the rsync options.

Rsync does look like it will work cross-platform. I wonder if the hiccup is actually happening due to Execa. See these lines:

await execa('rsync', ['-rtvu --delete', `'${src}'`, `'${dest}'`], {
shell: true,
stdio: 'inherit',
cleanup: true,
})

Check out the Execa docs for the shell option stating it's not cross-platform:
https://www.npmjs.com/package/execa#shell

Maybe we don't need the option and/or could have logic to check for sys and call corresponding shell setting?

@Tobbe
Copy link
Member Author

Tobbe commented Jun 14, 2020

So, this was the error message I got (as pasted above)

$ C:\Users\tobbe\dev\redwood\test-store\node_modules\.bin\rwt copy ../redwood
Redwood Framework Path:  C:\Users\tobbe\dev\redwood\redwood
The source and destination cannot both be remote.
rsync error: syntax or usage error (code 1) at main.c(1303) [Receiver=3.1.3]
(node:35616) UnhandledPromiseRejectionWarning: Error: Command failed with ENOENT: rsync -rtvu --delete 'C:\Users\tobbe\dev\redwood\redwood/packages/' 'C:\Users\tobbe\dev\redwood\test-store/node_modules/@redwoodjs/'

Taking the command it's trying to run, and running it manually, straight on the command line, I get this

$ rsync -rtvu --delete 'C:\Users\tobbe\dev\redwood\redwood/packages/' 'C:\Users\tobbe\dev\redwood\test-store/node_modules/@redwoodjs/'
The source and destination cannot both be remote.
rsync error: syntax or usage error (code 1) at main.c(1303) [Receiver=3.1.3]

So I don't think the problem is with execa.

Running pwd, I get this

$ pwd
/c/Users/tobbe/dev/redwood/test-store

Using that path format instead with rsync, and we have success!

$ rsync -rtvu --delete /c/Users/tobbe/dev/redwood/redwood/packages/ /c/Users/tobbe/dev/redwood/test-store/node_modules/\@redwoodjs/
sending incremental file list
./
deleting api/LICENSE
deleting cli/LICENSE
deleting cli/node_modules/prettier/third-party.js
[...]
web/src/graphql/index.js
web/src/graphql/withCell.js

sent 5,277,162 bytes  received 54,836 bytes  969,454.18 bytes/sec
total size is 5,790,155  speedup is 1.09

So, once again we're seeing an issue with Windows path format 😁

I could try looking in to fixing that, but that would still leave us with having to tell all Windows users to install rsync, and that wasn't a very user-friendly experience 🙁 Because of that, I'm voting for replacing rsync with some npm package to do the copying for us.

@peterp
Copy link
Contributor

peterp commented Jun 14, 2020

Amazing!

I could try looking in to fixing that, but that would still leave us with having to tell all Windows users to install rsync, and that wasn't a very user-friendly experience

redwood-tools is only required when contributing to the Redwood framework, so it's not something that's expected to work for each user. Maybe we could add a guide for installing it via choco: https://chocolatey.org/packages?q=rsync

I wouldn't want to move to something else since rsync is so fast and reliable.

Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 14, 2020
@Tobbe
Copy link
Member Author

Tobbe commented Jun 14, 2020

PowerShell is giving me a headache

/c/Users/... Doesn't work

PS C:\Users\tobbe\dev\redwood\test-store> rsync -rtvu --delete /c/Users/tobbe/dev/redwood/redwood/packages /c/Users/tobbe/dev/redwood/test-store/node_modules/@redwoodjs/
sending incremental file list
rsync: change_dir "/c/Users/tobbe/dev/redwood/redwood" failed: No such file or directory (2)
rsync: mkdir "/c/Users/tobbe/dev/redwood/test-store/node_modules/@redwoodjs" failed: No such file or directory (2)
rsync error: error in file IO (code 11) at main.c(657) [Receiver=3.1.2]

C:/Users/... doesn't work

PS C:\Users\tobbe\dev\redwood\test-store> rsync -rtvu --delete C:/Users/tobbe/dev/redwood/redwood/packages C:/Users/tobbe/dev/redwood/test-store/node_modules/@redwoodjs/
The source and destination cannot both be remote.
rsync error: syntax or usage error (code 1) at main.c(1274) [Receiver=3.1.2]
PS C:\Users\tobbe\dev\redwood\test-store>

C:\Users\... doesn't work

PS C:\Users\tobbe\dev\redwood\test-store> rsync -rtvu --delete C:\Users\tobbe\dev\redwood\redwood\packages C:\Users\tobbe\dev\redwood\test-store\node_modules\@redwoodjs\
The source and destination cannot both be remote.
rsync error: syntax or usage error (code 1) at main.c(1274) [Receiver=3.1.2]

Hah! //localhost/c$/Users/... works!

PS C:\Users\tobbe\dev\redwood\test-store> rsync -rtvu --delete //localhost/c$/Users/tobbe/dev/redwood/redwood/packages //localhost/c$/Users/tobbe/dev/redwood/test-store/node_modules/@redwoodjs/
sending incremental file list
packages/
packages/api/
packages/api/.babelrc.js
[...]
packages/web/src/graphql/index.js
packages/web/src/graphql/withCell.js

sent 5,869,436 bytes  received 20,412 bytes  165,911.21 bytes/sec
total size is 5,791,554  speedup is 0.98

The good news is that it works in GitBash as well, so both PS and GitBash can use the //localhost syntax. The bad news is that it's really slow. I timed both versions in GitBash /c/Users/... finished in 1.56 seconds, //localhost/c$/Users/... finished in 22.21 seconds 🙁

@peterp
Copy link
Contributor

peterp commented Jun 15, 2020

Oh wow, that's crazy - is there something else on Windows that people can use to keep files in sync?

@Tobbe
Copy link
Member Author

Tobbe commented Jun 15, 2020

I still haven't given up on rsync, I actually think I'm on to something 🙂 I'll be back with an update later tonight, after work.

@peterp
Copy link
Contributor

peterp commented Jun 15, 2020

Tack!

Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 15, 2020
@Tobbe
Copy link
Member Author

Tobbe commented Jun 15, 2020

The problem with PowerShell wasn't really with PowerShell, but rather with the version of rsync it used. I installed cwRsync (top result on that chocolatey search result page linked above) and that seemed to have caused a lot of the problems. When I figured out how to use the msys2 version of rsync in PowerShell as well, everything started working, and it was fast too 🙂

I documented the installation here https://tlundberg.com/blog/2020-06-15/installing-rsync-on-windows/

Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 15, 2020
Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 15, 2020
@thedavidprice
Copy link
Contributor

@Tobbe excited to see your progress here! Just wanted to make sure I understood and tied together next step in progress:

  1. you'll confirm Powershell is working so this can be merged "rwt copy fix for Windows" rwt copy fix for Windows #694
  2. update CONTRIBUTING section instructions with the Windows install for Rsync (or link to your blog post)

Anything missing? And happy to help on step 2 -- just let me know or assign an issue to me.

🚀

Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 16, 2020
@Tobbe
Copy link
Member Author

Tobbe commented Jun 16, 2020

@thedavidprice

  1. confirmed
  2. I added a link. If you want me to inline the whole blog post, I'll do that instead.

One thing that unfortunately came up during my testing is that the build:watch + copy:watch workflow doesn't work on Windows. See #701

Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 16, 2020
Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 16, 2020
Tobbe added a commit to Tobbe/redwood that referenced this issue Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants