Different paths for OSX and Linux executables #64

Closed
geekdave opened this Issue Jun 12, 2013 · 10 comments

Comments

Projects
None yet
7 participants

Hello and thanks for this fantastic project!

We are using phantomjs for a large enterprise project with dozens of developers. Rather than have every developer maintain their own npm dependencies, we have our node_modules directory checked-in under source control. This way, developers can pull from the repo, run grunt and everything "just works".

The issue is that we support multiple development environments. We have developers using Windows, Mac OSX, and Linux. Most of our grunt dependencies are platform-agnostic. However, since phantom has different binary executables for these three platforms, we need to have the superset of all three binaries checked in.

So far we have:

For Windows, we checked in:
lib/phantom/phantomjs.exe

For Mac OSX, we checked in:
lib/phantom/bin/phantomjs

We tried to do the same for Linux, but found that the path & filename for the Linux binary are the same as for Mac OSX.

Sure enough, in lib/phantomjs.js there is only a differentiation made between windows and "everything else"

exports.path = process.platform === 'win32' ?
    path.join(__dirname, 'phantom', 'phantomjs.exe') :
    path.join(__dirname, 'phantom', 'bin' ,'phantomjs')

What if we added platform-specific subdirectories in lib/phantom, and then changed the above snippet to:

switch (process.platform) {
  case 'win32':
    exports.path = path.join(__dirname, 'phantom', 'win', 'phantomjs.exe');
    break;
  case 'darwin':
    exports.path = path.join(__dirname, 'phantom', 'mac', 'phantomjs');
    break;
  case 'linux':
  default:
    exports.path = path.join(__dirname, 'phantom', 'linux', 'phantomjs');
    break;
}

This would allow us to check-in platform-specific binaries like:

lib/phantom/win32/phantomjs.exe
lib/phantom/mac/phantomjs
lib/phantom/linux/phantomjs

Thanks!

Contributor

dpup commented Jun 12, 2013

Doesn't seem too crazy. Aren't you going to have problems with your system
as soon as you use any package with v8 bindings?

That said, since this package is basically just an installer is there much
value in using it if you are manually downloading the binaries anyway? You
could put a much smaller script in place that is API compatible and points
to the right binary.

Either way, happy to accept a patch to disambiguate target location.

-- Dan

On Wed, Jun 12, 2013 at 12:21 PM, Dave Cadwallader <notifications@github.com

wrote:

Hello and thanks for this fantastic project!

We are using phantomjs for a large enterprise project with dozens of
developers. Rather than have every developer maintain their own npm
dependencies, we have our node_modules directory checked-in under source
control. This way, developers can pull from the repo, run grunt and
everything "just works".

The issue is that we support multiple development environments. We have
developers using Windows, Mac OSX, and Linux. Most of our grunt
dependencies are platform-agnostic. However, since phantom has different
binary executables for these three platforms, we need to have the superset
of all three binaries checked in.

So far we have:

For Windows, we checked in:
lib/phantom/phantomjs.exe

For Mac OSX, we checked in:
lib/phantom/bin/phantomjs

We tried to do the same for Linux, but found that the path & filename for
the Linux binary are the same as for Mac OSX.

Sure enough, in lib/phantomjs.js there is only a differentiation made
between windows and "everything else"

exports.path = process.platform === 'win32' ?
path.join(__dirname, 'phantom', 'phantomjs.exe') :
path.join(__dirname, 'phantom', 'bin' ,'phantomjs')

What if we added platform-specific subdirectories in lib/phantom, and
then changed the above snippet to:

switch (process.platform)
case 'win32':
exports.path = path.join(__dirname, 'phantom', 'win', 'phantomjs.exe');
break;
case 'darwin':
exports.path = path.join(__dirname, 'phantom', 'mac', 'phantomjs');
break;
case 'linux':
default:
exports.path = path.join(__dirname, 'phantom', 'linux', 'phantomjs');
break;

This would allow us to check-in platform-specific binaries like:

lib/phantom/win32/phantomjs.exe
lib/phantom/mac/phantomjs
lib/phantom/linux/phantomjs

Thanks!


Reply to this email directly or view it on GitHubhttps://github.com/Obvious/phantomjs/issues/64
.

Thanks, Dan. I'll work on a patch for this. Just wanted to make sure it would be accepted. The reason we wanted to use this project instead of a custom script is because we are using grunt-contrib-qunit which depends on this. I understand that it might be overkill since we're manually merging the binaries anyway. But I think it's our best option.

Not sure what you mean about "package with v8 bindings"? Currently we are using phantomjs to run a QUnit suite, and it's working fine across our 3 supported platforms. Is there a specific example of this type of problem?

Contributor

dpup commented Jun 12, 2013

A bunch of packages require architecture specific compilation.

-- Dan

On Wed, Jun 12, 2013 at 12:45 PM, Dave Cadwallader <notifications@github.com

wrote:

Thanks, Dan. I'll work on a patch for this. Just wanted to make sure it
would be accepted. The reason we wanted to use this project instead of a
custom script is because we are using grunt-contrib-qunit which depends on
this. I understand that it might be overkill since we're manually merging
the binaries anyway. But I think it's our best option.

Not sure what you mean about "package with v8 bindings"? Currently we are
using phantomjs to run a QUnit suite, and it's working fine across our 3
supported platforms. Is there a specific example of this type of problem?


Reply to this email directly or view it on GitHubhttps://github.com/Obvious/phantomjs/issues/64#issuecomment-19350569
.

@ChrisWren ChrisWren referenced this issue in gruntjs/grunt-lib-phantomjs Aug 13, 2013

Closed

Allow cross-platform side-by-side phantomjs installs #28

Contributor

dpup commented Aug 13, 2013

I'm going to close this issue out. It seems quite a specific usecase. Will entertain patches if you come up with one.

@dpup dpup closed this Aug 13, 2013

Hi @geekdave, did you ever find a workaround for this?

We just ran into this same issue in our build and are looking around for solutions. We develop on OSX while our build system runs tests on linux instances, so checking in our node_modules as is seems to be a problem

@eloyperez : I just locally modified the file as shown above, and checked in into our source control, along with the linux binaries. Not the cleanest solution, as it involves re-doing that patch whenever we upgrade, but it got the job done.

I would love to see this re-opened. I think its a common usecase with large development teams. I'm currently running into the same issue for one of our projects.

We have the same issue here and I wonder if the changes in the installer as mentioned above are everything I have to adapt - besides the location.js file of course. But then, I have to patch the location.js file depending on which system I am?! @geekdave

jchitel commented Jan 26, 2016

I'd like to see this re-opened too. There are plenty of use cases for this, and npm packages are implied to be platform independent. If there is a file included that is platform-dependent, it should be placed on a special path so that rebuilding on another machine doesn't clobber the existing binary.

Contributor

nicks commented Jan 26, 2016

re: npm packages are implied to be platform independent: This is simply not true. Many node packages have native code.

The standard solution these days is to run npm rebuild to ensure platform-specific dependencies are correct. see:
https://github.com/Medium/phantomjs#cross-platform-repositories

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment