WIP - updated preinstall and postinstall scripts for cross OS compatibility

mariolo1985 commented 5 years ago

Description

Currently, the preinstall and postinstall scripts execute with rm and cp commands that are not available on Windows OS.

This is causing npm install to fail on Windows.

This is tricky since the preinstall script runs before any modules are installed. I could not use modules like rimraf or fs-extra. I used Node to handle the removing and copying files. Node should be installed on machines that runs NPM so it should be reliable.

  "scripts": {
    // other scripts
    "preinstall": "(rm .git/hooks/* && cp ./.githooks/* .git/hooks) || true",
    "postinstall": "rm -f node_modules/web3/index.d.ts node_modules/web3/types.d.ts",
    // other scripts
  }

Note: rm and cp command is used in a dependency gulp-ts-spellcheck and this is causing npm install to fail in this repo.

Related Issue

Motivation and Context

So our community of Windows contributors can continue helping out on this project.

How Has This Been Tested?

Ran npm install

Note: rm and cp command is used in a dependency gulp-ts-spellcheck and this is causing npm install to fail in this repo.

Screenshots (if appropriate):

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

mariolo1985 commented 5 years ago

@TalAter @srfrnk the gulp-ts-spellcheck dependency will need similar updates before this resolves install issues on Windows OS.

TalAter commented 5 years ago

I like this. @srfrnk - Can you test and merge? Also, I know you're against gulp-ts-spellcheck working in Windows, but how about doing the same there?

srfrnk commented 5 years ago

I would like to perform a proper design process for tasks issues that change design patterns being used in our code. Please hold this issue until we can do that.

srfrnk commented 5 years ago

See https://github.com/DAVFoundation/dav-js/commit/ca2ea379b49b81e02fa1b4ac2556b115925827a6 Should make this PR obsolete

TalAter commented 5 years ago

@mariolo1985 Can you verify if this works in Windows?

mariolo1985 commented 5 years ago

Hey @TalAter @srfrnk I am getting the error for gulp-ts-spellcheck. Please see image below.

gulptsspellcheck

srfrnk commented 5 years ago

Hey @mariolo1985 Can you please try now? I removed the postinstall entirely for now.

mariolo1985 commented 5 years ago

Hi @srfrnk the error above remains when I ran npm install. I re-forked this repo to ensure master and my fork are even.

It seems to be erroring out on gulp-ts-spellcheck's post install script (cp ./.githooks/* .git/hooks) || true

srfrnk commented 5 years ago

@mariolo1985 This was an issue with gulp-ts-spellcheck. Published a new version now. Should fix it. Can you please try again?

mariolo1985 commented 5 years ago

It looks like that fix did it @srfrnk :man_dancing: I was able to run npm install without erroring out and npm run jest for a sanity check.

DAVFoundation / dav-js