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

Garbage characters in Gulp.cmd after first install #828

Closed
dev-rowbot opened this issue Oct 12, 2016 · 13 comments
Closed

Garbage characters in Gulp.cmd after first install #828

dev-rowbot opened this issue Oct 12, 2016 · 13 comments

Comments

@dev-rowbot
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
I installed yarn in Windows as per the instructions and then ran yarn in a clean project directory - no existing node_modules. Yarn picked up the package.json file and pulled down all the correct modules.

However, when I attempted to execute gulp using the command .\node_modules.bin\gulp it returned the error:
s" was unexpected at this time.

I compared the gulp.cmd file with a version installed using npm3 and there are extra unexpected characters at the end of the file:

@IF EXIST "%~dp0\node.exe" (
  "%~dp0\node.exe"  "%~dp0\..\gulp\bin\gulp.js" %*
) ELSE (
  @SETLOCAL
  @SET PATHEXT=%PATHEXT:;.JS;=;%
  node  "%~dp0\..\gulp\bin\gulp.js" %*
)s" %*
)

These characters should not be there: s" %*)

I removed the node_modules and reinstalled and there are no errors now

If the current behavior is a bug, please provide the steps to reproduce.
Steps as per above

What is the expected behavior?
The gulp.cmd should not have any bad characters after install

Please mention your node.js, yarn and operating system version.

  • yarn - 0.15.1
  • node - v4.4.5
  • Windows 7

Additional Info

My package.json file is as follows:

{
  "name": "ci-scripts",
  "version": "1.0.0",
  "description": "CI Support Scripts",
  "main": "gulpfile.js",
  "dependencies": {
    "del": "^2.2.1",
    "gulp": "github:gulpjs/gulp#4.0",
    "gulp-cli": "^1.2.2",
    "jshint": "^2.9.2",
    "lodash": "^4.15.0",
    "node-etcd": "^5.0.3",
    "q": "^1.4.1",
    "request": "^2.74.0",
    "yamljs": "^0.2.8",
    "yargs": "^4.8.1"
  }
}
@dev-rowbot
Copy link
Author

For sanity's sake I also just check the bash script after deleting and reinstalling the node_modules and it is also broken

#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -x "$basedir/node" ]; then
  "$basedir/node"  "$basedir/../gulp/bin/gulp.js" "$@"
  ret=$?
else 
  node  "$basedir/../gulp/bin/gulp.js" "$@"
  ret=$?
fi
exit $ret
it $ret

That last it $ret should not be there

@yerol
Copy link

yerol commented Oct 26, 2016

I'm having a similar issue with grunt. I have this simple install command in package.json scripts:

"install": "grunt build"

yarn executes the command fine (at least it looks that way)

$ grunt build

Bu I get the following error immediately after:

t" build was unexpected at this time.

Feels like certain characters in these commands are problematic, or maybe this is specific to Grunt I'm not sure. I'm running bower, eslint, mocha etc exactly the same way and they work perfectly fine.

My environment details:
Windows 10
Node 6.9.1
npm 3.10.9
yarn 0.16.1
grunt 1.0.1
grunt-cli 1.2.0

@busches
Copy link
Contributor

busches commented Dec 1, 2016

Can you reproduce this with yarn@latest? I'm not able to recreate the issue using the latest Yarn v0.17.10. Both gulp and gulp.cmd are correct.

@stefda
Copy link

stefda commented Dec 2, 2016

@yerol, I had exactly the same issue and resolved it by deleting node_modules. Something messed up my environment after a series of merges. Can you confirm that fixes it for you too?

@yerol
Copy link

yerol commented Dec 2, 2016

@stefda that seems to be the fix. thanks.

@devbyray
Copy link

devbyray commented Dec 2, 2016

I have updated to yarn@latest, bit still the problem happens with Karma 👎

@DanReyLop
Copy link
Contributor

DanReyLop commented Jan 2, 2017

An easier fix is to force yarn to regenerate the .bin\* files, for example, running yarn install. Doesn't always work.

I'm having the same problem, NodeJS 6.9.1, Yarn 0.18.1 (latest), Windows 10.

By the look of it, it seems like, for some reason, during the initial installation the .bin\* and .bin\*.cmd files are being written twice, first with slightly longer program argument, and then written again with the correct one (without erasing the file first). This is why it only affects short packages (grunt, karma).

Seeing the code of cmd-shim, there's no way the file would be written twice there, so the problem must be in the yarn logic, which is calling cmd-shim twice.

If this rings a bell to any maintainer that would be awesome. If not, I'm willing to help solve this (I'll probably need to read some high-level code structure document, this is a complex codebase).

@DanReyLop
Copy link
Contributor

Ok I got this, I couldn't go to sleep with a mistery unanswered 😄

The problem is a race condition on cmd-shim, and it only happens with packages that provide the same CLI bin. In this example, both karma-cli and karma provide the bin karma. I'm willing to bet that grunt and gulp have the same situation.

cmd-shim logic:

  • Remove program and program.cmd (if they exist).
  • Write new program and program.cmd.

yarn install logic:

  • Link karma-cli (async)
  • Link karma (async)
  • From cmd-shim('karma-cli'): Remove karma and karma.cmd (if they exist)
  • From cmd-shim('karma'): Remove karma and karma.cmd (if they exist)
  • From cmd-shim('karma-cli'): Write new karma and karma.cmd
  • From cmd-shim('karma'): Write new karma and karma.cmd

So, if the first karma.cmd written is something like this (simplified):
"%~dp0\node.exe" "%~dp0\..\karma-cli\bin.js" %*

Then the final written karma.cmd will be (note the extra 4 chars at the end):
"%~dp0\node.exe" "%~dp0\..\karma\bin.js" %*" %*

Not sure how to solve this though. Without knowing much about how yarn works, it would make sense for yarn to make a "dedupe" of sorts, instead of writing a symlink or exec file that will be overwritten 1ms later. On the other hand, I feel like cmd-shim should be resilient to these kind of things. I'll wait for the opinion of folks more experienced in this codebase than me (i.e. anyone who spent more than 10 minutes reading it).

@ghost
Copy link

ghost commented Feb 21, 2017

We are encountering the same problem as @DanReyLop with karma. Every second/third build on the buildserver fails because the cmd-file is corrupted.
Does someone has a workaround for this problem or is there a bugfix in sight?

@wadim
Copy link

wadim commented Feb 24, 2017

Duplicate of #2356

@wadim
Copy link

wadim commented Feb 24, 2017

@DanReyLop Thanks for sharing!

I just got the same error with npm.

> [email protected] test D:\jenkins\workspace\My Project\frontend
> cross-env NODE_ENV=test jest


s"  was unexpected at this time.
D:\jenkins\workspace\My Project\frontend>)s" 
npm ERR! Test failed.  See above for more details.

@farrago
Copy link

farrago commented May 8, 2017

This is still happening for me in v0.23.4.
Occurs when doing yarn install for a https://github.com/zurb/foundation-emails-template project.

Platform: Windows 10, i7-45100U, 16GB RAM, slow spinning disk in a laptop.

@bestander
Copy link
Member

@farrago please open a new issue with steps to reproduce.

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

10 participants