Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Resolve Copying on Multiplatform #596

Merged
merged 3 commits into from
Oct 18, 2019
Merged

Resolve Copying on Multiplatform #596

merged 3 commits into from
Oct 18, 2019

Conversation

Stuyk
Copy link
Contributor

@Stuyk Stuyk commented Oct 9, 2019

Added a small script that copies ripemd.es5.js to the dist folder. Otherwise this repo does not currently work on windows.

@GreenBusDriver
Copy link
Contributor

GreenBusDriver commented Oct 18, 2019

@Stuyk Thanks so much for submitting this PR! :)

I don't have a Windows system to verify the issue and the fix. So we're sure we're understanding, could you provide more description on when and what happens that fails, and how your script fixes it?
My best understanding at this point is

  • During the eosjs transpilation process, ripemd.es5.js is typically be copied on a linux-based system.
  • On Windows, it is not because you're running a cp command in cmd.exe context.
  • Your script fixes this issue by moving the copy out into a node.js script (which is cross-platform).
  • Final test case for us to confirm we're not breaking linux/mac systems: confirm ripemd5.js gets copied to dist folder (and you can do the same once this is merged).

Please correct my understanding above as needed.

@Stuyk
Copy link
Contributor Author

Stuyk commented Oct 18, 2019

@Stuyk Thanks so much for submitting this PR! :)

I don't have a Windows system to verify the issue and the fix. So we're sure we're understanding, could you provide more description on when and what happens that fails, and how your script fixes it?
My best understanding at this point is

  • During the eosjs transpilation process, ripemd.es5.js is typically be copied on a linux-based system.
  • On Windows, it is not because you're running a cp command in cmd.exe context.
  • Your script fixes this issue by moving the copy out into a node.js script (which is cross-platform).
  • Final test case for us to confirm we're not breaking linux/mac systems: confirm ripemd5.js gets copied to dist folder (and you can do the same once this is merged).

Please correct my understanding above as needed.

Your understanding is correct; sometimes on Windows depending on the environment the cp can be problematic. At the time that I originally wrote this small request; I was running into this issue constantly and narrowed it down to the copy command. This is merely a cross-platform solution for it.

As a side note; I haven't been able to reproduce it today after trying a few different ways I was doing it before.

@Stuyk
Copy link
Contributor Author

Stuyk commented Oct 18, 2019

Additional Comment. Just figured out how I came across it. Able to reproduce by making a clone of this repo; and doing npm install.

Then we're left with:

'cp' is not recognized as an internal or external command,
operable program or batch file.

 eosjs@20.0.1 build: `tsc -p ./tsconfig.json && cp src/ripemd.es5.js dist/ripemd.js`

And that's the issue that is being resolved. :)

@GreenBusDriver
Copy link
Contributor

Perfect @Stuyk . Totally clear now. Lastly, final confirmation... you're seeing this on the cmd.exe commandline, not trying to run in WSL (Window Linux Subsystem), right? I'm curious if this works (as I suspect it would) in the WSL environment. Not to beat a dead horse, but changes we don't add are changes that are most thoroughly tested and reliable. :) So I'd like to know as precisely as possible what we're accomplishing and what other options we might have in the future...

Thanks again for your support on this.

@Stuyk
Copy link
Contributor Author

Stuyk commented Oct 18, 2019

Perfect @Stuyk . Totally clear now. Lastly, final confirmation... you're seeing this on the cmd.exe commandline, not trying to run in WSL (Window Linux Subsystem), right? I'm curious if this works (as I suspect it would) in the WSL environment. Not to beat a dead horse, but changes we don't add are changes that are most thoroughly tested and reliable. :) So I'd like to know as precisely as possible what we're accomplishing and what other options we might have in the future...

Thanks again for your support on this.

I'm seeing this in Powershell, and CMD. I currently do not have a subsystem installed since I just dual boot into Linux.

@GreenBusDriver GreenBusDriver merged commit 3cbda6f into EOSIO:master Oct 18, 2019
@GreenBusDriver
Copy link
Contributor

Thanks again @Stuyk . Until next time :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants