Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Stops yeoman from looking for win32 binaries in ../vendor/ #519

Merged
merged 1 commit into from

4 participants

Kevin Mårtensson Sindre Sorhus b-long Frederick Ros
Kevin Mårtensson
Collaborator

This fixes an issue where Yeoman searches for optipng.exe and jpegtran.exe in ../vendor/ on Windows. Since those files aren't there anymore this will render an error when you are running build.

This commit will treat Windows like any other system and search for the libraries in PATH instead.

Sindre Sorhus

Have you tested that this actually works on Windows?

Kevin Mårtensson
Collaborator

Yes, this works, provided that you have optipng and jpegtran in PATH.

b-long

@sindresorhus Any thoughts on this ? ( #485 )

Sindre Sorhus sindresorhus merged commit b1ca678 into from
Frederick Ros sleeper referenced this pull request
Closed

Tests are not passing #600

Frederick Ros
Owner

I was just creating the same ;)

Seems like the logic has changed and potentially some part were left off ...
Before the merge:

   if ( !win32 || !/optipng|jpegtran/.test( cmd ) ) {
      return which( cmd, cb );
    }
    var cmdpath = cmd === 'optipng' ? '../vendor/optipng-0.7.1-win32/optipng.exe' :
      '../vendor/jpegtran-8d/jpegtran.exe';
    cb(null, path.join(__dirname, cmdpath));

after the merge:

    if ( win32 || !/optipng|jpegtran/.test( cmd ) ) {
      return which( cmd, cb );
    }

e.g.:

  • Before the merge the which was called if we were not running windows, or (in case we ran windows) if the command was neither optipng nor jpegtran
  • After the merge, it is called if we're running on windows or if the command is not optipng or jpegtran

So logic wise this is almost the contrary (i.e. in my case, on OSX, in the "before" version which was always called, no it is called only if the command is not optipng nor jpegtran).

Kevin Mårtensson
Collaborator

Hey guys, sorry for the inconvenience. Didn't have a chance to test this on OS X. I can confirm that d1ed27e works on Windows though.

Márton Szinovszki szinya referenced this pull request from a commit in menthainternet/yeoman
Sindre Sorhus sindresorhus Fix #519 breaking build on OS X b21f8a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 8 deletions.
  1. +2 −8 cli/tasks/img.js
10 cli/tasks/img.js
View
@@ -145,17 +145,11 @@ module.exports = function(grunt) {
}
});
- // **which** helper, wrapper to isaacs/which package plus some fallback logic
- // specifically for the win32 binaries in vendor/ (optipng.exe, jpegtran.exe)
+ // **which** helper, wrapper to isaacs/which package
grunt.registerHelper('which', function(cmd, cb) {
- if ( !win32 || !/optipng|jpegtran/.test( cmd ) ) {
+ if ( win32 || !/optipng|jpegtran/.test( cmd ) ) {
return which( cmd, cb );
}
-
- var cmdpath = cmd === 'optipng' ? '../vendor/optipng-0.7.1-win32/optipng.exe' :
- '../vendor/jpegtran-8d/jpegtran.exe';
-
- cb(null, path.join(__dirname, cmdpath));
});
};
Something went wrong with that request. Please try again.