Skip to content
This repository has been archived by the owner on Jul 24, 2019. It is now read-only.

io.js fails on copying extracted folder #275

Closed
nicks opened this issue Jan 20, 2015 · 21 comments
Closed

io.js fails on copying extracted folder #275

nicks opened this issue Jan 20, 2015 · 21 comments

Comments

@nicks
Copy link
Contributor

nicks commented Jan 20, 2015

Reported by @corbinu in this comment:
#271 (comment)

'Copying extracted folder /var/folders/pt/_mpbrdv90p3741h3xyt253pw0000gn/T/phantomjs/phantomjs-1.9.8-macosx.zip-extract-1421713554555/phantomjs-1.9.8-macosx -> /Users/corbinu/Developer/tests/node_modules/phantomjs/lib/phantom'

@corbinu can you post more complete repro steps, most notably (1) the complete log, and (2) the version of io.js that you're using?

@corbinu
Copy link

corbinu commented Jan 20, 2015

There is no log as it never errors out and every version after 1.0.1

@zaggino
Copy link

zaggino commented Jan 21, 2015

Same on Windows with io.js 1.0.3

npm info build C:\DEV\weather-forecast2\node_modules\karma-phantomjs-launcher\node_modules\phantomjs
npm info linkStuff [email protected]
npm info install [email protected]

> [email protected] install C:\DEV\weather-forecast2\node_modules\karma-phantomjs-launcher\node_modules\phantomjs
> node install.js

Downloading https://bitbucket.org/ariya/phantomjs/downloads/phantomjs-1.9.8-windows.zip
Saving to C:\Users\martinz\AppData\Local\Temp\phantomjs\phantomjs-1.9.8-windows.zip
Receiving...
  [========================================] 100% 0.0s
Received 7292K total.
Extracting zip contents
Removing C:\DEV\weather-forecast2\node_modules\karma-phantomjs-launcher\node_modules\phantomjs\lib\phantom
Copying extracted folder C:\Users\martinz\AppData\Local\Temp\phantomjs\phantomjs-1.9.8-windows.zip-extract-1421816704172\phantomjs-1.9.8-windows -> C:\DEV\weather-forecast2\node_modules\karma-phantomjs-launcher\node_modules\phantomjs\lib\phantom

and stays frozen forever...

@nicks
Copy link
Contributor Author

nicks commented Jan 21, 2015

I filed a bug against ncp for this AvianFlu/ncp#79

@jprichardson
Copy link

I have imported ncp into fs-extra https://www.npmjs.com/package/fs-extra and now works on io.js. So maybe this would be an option. You could then also get rid of mkdirp and rimraf as dependencies as fs-extra uses rimraf and imported mkdirp.

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

merged @corbinu 's PR and released an npm package as 1.9.14

@nicks nicks closed this as completed Jan 27, 2015
@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

The PR has been rolled back, because we were getting reports of it breaking things. see #279 for more information.

It appears that fs-extra has its own set of problems. The fs.move claims to complete successfully, but the fs.chmod claims the moved file doesn't exist. fs-extra adds a lot of hooks into both these methods, and it is unclear which one is lying. I didn't spend a lot of time looking into it.

@nicks nicks reopened this Jan 27, 2015
@corbinu
Copy link

corbinu commented Jan 27, 2015

Is this project on Travis CI I see the yml file but not seeing it on travis obviously want to get to the bottom of this

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

yep, looking at the travis ci build right now

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

OK, here's a travis run of the fs-extra changes: https://travis-ci.org/Medium/phantomjs/jobs/48503467
and it appears to work ok there...

@corbinu
Copy link

corbinu commented Jan 27, 2015

no thats your rollback... I am going to add my fork with the changes to travis also

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

an alternative solution might be to just fork ncp, because they do not seem interested in fixing their package.

@corbinu
Copy link

corbinu commented Jan 27, 2015

True that is your call

@corbinu
Copy link

corbinu commented Jan 27, 2015

Are you still supporting Node 0.6 and 0.8 given they are in the .travis?

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

travis ci has been in this weird state lately where you can specify node 0.6 and 0.8, but the version of npm they have installed by default is incompatible with 0.8 due to the semver changes. There's some incantation to tell it to use the right npm version, but for now, I just removed them.

@corbinu
Copy link

corbinu commented Jan 27, 2015

Builds fine for me to :/ weird https://travis-ci.org/corbinu/phantomjs

@corbinu
Copy link

corbinu commented Jan 27, 2015

I also tried installing v0.10.33 on my Ubuntu 14.04 server and then installing. I can't replicate the issue

@jprichardson
Copy link

It appears that fs-extra has its own set of problems. The fs.move claims to complete successfully, but the fs.chmod claims the moved file doesn't exist. fs-extra adds a lot of hooks into both these methods, and it is unclear which one is lying. I didn't spend a lot of time looking into it.

If anyone can help me reproduce the problems, that'd be incredibly helpful. fs-extra doesn't hook any methods.

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

I'm able to repro, and am trying to narrow it down to a nice repro case. I'm pretty sure it's related to fs.move across devices.

@nicks
Copy link
Contributor Author

nicks commented Jan 27, 2015

filed as jprichardson/node-fs-extra#108

@jprichardson
Copy link

I fixed jprichardson/node-fs-extra#108 and published [email protected]. You can try it. If for some reason fs.move still doesn't work, you could stick with fs.copy like what you had previously (8a96d60) specifically keeping lines: https://github.com/Medium/phantomjs/blob/master/install.js#L351-L360. Then at least you'd have io.js support immediately.

@nicks
Copy link
Contributor Author

nicks commented Jan 28, 2015

ok, released as v1.9.15

@nicks nicks closed this as completed Jan 28, 2015
sanderboom added a commit to sanderboom/generator-hottowel that referenced this issue May 15, 2015
davide added a commit to davide/dalek-browser-phantomjs that referenced this issue Mar 26, 2017
davide added a commit to davide/dalek that referenced this issue Mar 26, 2017
We need to have phantoms 1.9.15 as a minimum to work around this bug:
Medium/phantomjs#275
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants