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

Install on mac broken since 3.2.3 #149

Closed
danez opened this issue Feb 7, 2016 · 14 comments
Closed

Install on mac broken since 3.2.3 #149

danez opened this issue Feb 7, 2016 · 14 comments

Comments

@danez
Copy link
Contributor

danez commented Feb 7, 2016

npm ERR! Darwin 15.2.0
npm ERR! argv "/Users/user/.nvm/versions/node/v4.2.6/bin/node" "/Users/user/.nvm/versions/node/v4.2.6/bin/npm" "install"
npm ERR! node v4.2.6
npm ERR! npm  v3.7.1
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn

npm ERR! gonzales-pe@3.2.5 postinstall: `./scripts/postinstall.sh`
npm ERR! spawn ENOENT
npm ERR! 
npm ERR! Failed at the gonzales-pe@3.2.5 postinstall script './scripts/postinstall.sh'.
@danez
Copy link
Contributor Author

danez commented Feb 7, 2016

It might be that the check for 'lib' is wrong, I'm not sure what the cwd is when calling the script.

@bgriffith
Copy link
Collaborator

Same problem - any version greater than 3.2.2 won't install via npm (but does install/build using github repo url)

@tonyganch
Copy link
Owner

Hold on, working on it

@tonyganch
Copy link
Owner

Done. Please try with @3.2.6.

@logiclabs
Copy link

This fix appears to have broken Windows compatibility. "-e was unexpected at this time."

if [ -e ./scripts/postinstall.sh ]; then ./scripts/postinstall.sh; fi

@stukennedy
Copy link

yeah, broken on windows ... this is a bash-specific script you've got here.

@DanPurdy
Copy link
Collaborator

DanPurdy commented Mar 9, 2016

Hi @tonyganch sorry to ask again but these scripts are preventing any install on windows currently (being bash specific) and blocking us releasing our update to use 3.2.6 😢 Is there any chance you could just remove these scripts again for now and roll a quick patch release? Thanks!

Also appveyor have some good support for testing on windows machines https://www.appveyor.com/ and are free for open source projects if you're looking to avoid issues with windows in the future (without having to touch one of those PC's!).

@clcpolevaulter
Copy link

@tonyganch thoughts on @DanPurdy's request? We'd love to have his changes for another issue on scss-lint.

@mven
Copy link

mven commented Mar 21, 2016

@DanPurdy @tonyganch I just tested on my Windows installation using the manual build instructions (3) in the readme, as well as modifying the package.json and I was able to successfully npm install the dev branch (3.2.6). Here's my modifications to the package.json. I know it's ugly, but I had to prepend bash to the script calls or Windows would not recognize the . or it would complain about -e flag etc (I've changed to -f since it wasn't giving me problems).

  "scripts": {
    "autofix-tests": "bash ./scripts/build.sh && bash ./scripts/autofix-tests.sh",
    "build": "bash ./scripts/build.sh",
    "init": "bash ./scripts/init.sh",
    "log": "bash ./scripts/log.sh",
    "prepublish": "bash ./scripts/prepublish.sh",
    "postinstall": "[ ! -f ./scripts/postinstall.sh ] || bash ./scripts/postinstall.sh",
    "postpublish": "bash ./scripts/postpublish.sh",
    "test": "bash ./scripts/build.sh && bash ./scripts/test.sh"
  },

@mven
Copy link

mven commented Mar 22, 2016

^ I used Git Bash terminal for this.

@DanPurdy
Copy link
Collaborator

@mven yeah that would be fine but it's not a workable solution for this. The issue is these scripts aren't even necessary when just using the package as a dependency from npm and unless you take some manual steps that 99.9% of windows users or indeed anyone not using a bash shell won't then it fails to install and causes npm to throw, which means we will be inundated with broken install issues if we were to push our release out.

@mven
Copy link

mven commented Mar 22, 2016

@DanPurdy After posting, I thought this might be the case wherein the dev would be forced to use bash, which might not be for all Windows users. It makes sense with your explanation.

tonyganch added a commit that referenced this issue Apr 28, 2016
Postinstall script failed install on Windows. Since I can't find a
proper way to fix the issue, I'm removing the script to not block
installation for users and hope to find solution later.

See #149
tonyganch added a commit that referenced this issue Apr 28, 2016
Postinstall script failed install on Windows. Since I can't find a
proper way to fix the issue, I'm removing the script to not block
installation for users and hope to find solution later.

See #149
@colin-marshall
Copy link

@DanPurdy is this fixed now so you don't have to use a fork anymore?

@DanPurdy
Copy link
Collaborator

DanPurdy commented May 5, 2016

@colin-marshall this is true but currently the fork doesn't really differ from the base repo here so we wont really need to change it until we update the base repo. @tonyganch was kind enough to give both @bgriffith and I write access to this repo too so we can hopefully help out merging PR's and also working directly on the AST itself, which we intend to do when we have time.

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

No branches or pull requests

9 participants