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

fails if bundledDependency is missing in .tgz file #886

Closed
raytiley opened this issue Oct 12, 2016 · 23 comments
Closed

fails if bundledDependency is missing in .tgz file #886

raytiley opened this issue Oct 12, 2016 · 23 comments

Comments

@raytiley
Copy link

Running yarn on 0.15.1 on OS X and having issues installing an ember-cli project.

ui -zsh 97x25 2016-10-12 10-06-36

package.json

{
  "name": "carousel-ui",
  "version": "0.0.1",
  "description": "User interface for Carousel API",
  "private": true,
  "directories": {
    "doc": "doc",
    "test": "tests"
  },
  "scripts": {
    "start": "ember server",
    "build": "ember build",
    "test": "ember test"
  },
  "repository": "",
  "engines": {
    "node": ">= 0.10.0"
  },
  "author": "",
  "license": "MIT",
  "devDependencies": {
    "broccoli-asset-rev": "^2.4.2",
    "ember-ajax": "^2.0.1",
    "ember-cli": "2.7.0",
    "ember-cli-app-version": "^1.0.0",
    "ember-cli-autoprefixer": "0.6.0",
    "ember-cli-babel": "^5.1.6",
    "ember-cli-bootstrap-sassy": "0.0.18",
    "ember-cli-dependency-checker": "^1.3.0",
    "ember-cli-deploy": "0.6.4",
    "ember-cli-deploy-build": "0.1.1",
    "ember-cli-deploy-gzip": "0.2.3",
    "ember-cli-deploy-s3": "0.3.1",
    "ember-cli-deploy-slack": "0.1.0",
    "ember-cli-deploy-webhooks": "0.3.0",
    "ember-cli-flash": "1.3.16",
    "ember-cli-htmlbars": "^1.0.3",
    "ember-cli-htmlbars-inline-precompile": "^0.3.5",
    "ember-cli-ic-ajax": "0.2.5",
    "ember-cli-inject-live-reload": "^1.4.0",
    "ember-cli-jshint": "^1.0.0",
    "ember-cli-mirage": "0.2.1",
    "ember-cli-moment-shim": "2.0.0",
    "ember-cli-nouislider": "0.9.1",
    "ember-cli-pagination": "2.2.2",
    "ember-cli-qunit": "^3.0.0",
    "ember-cli-release": "^0.2.9",
    "ember-cli-rollbar": "0.3.0",
    "ember-cli-sass": "5.5.1",
    "ember-cli-selectize": "0.5.5",
    "ember-cli-sri": "^2.1.0",
    "ember-cli-test-loader": "^1.1.0",
    "ember-cli-uglify": "^1.2.0",
    "ember-concurrency": "^0.7.8",
    "ember-cpm": "2.1.2",
    "ember-data": "^2.7.0",
    "ember-data-model-fragments": "2.3.2",
    "ember-export-application-global": "^1.0.5",
    "ember-font-awesome": "2.1.1",
    "ember-freestyle": "0.2.13",
    "ember-i18n": "4.2.2",
    "ember-in-viewport": "2.0.8",
    "ember-load-initializers": "^0.5.1",
    "ember-lodash": "0.0.10",
    "ember-modal-dialog": "0.8.8",
    "ember-moment": "6.1.0",
    "ember-one-way-controls": "0.9.2",
    "ember-owner-test-utils": "0.1.1",
    "ember-page-title": "3.0.6",
    "ember-pikaday": "2.1.0",
    "ember-plupload": "raytiley/ember-plupload#fix-flaky-test-failure",
    "ember-power-select": "0.10.11",
    "ember-radio-button": "1.0.7",
    "ember-resolver": "^2.0.3",
    "ember-responsive": "1.2.7",
    "ember-route-action-helper": "1.0.0",
    "ember-sortable": "1.8.2",
    "ember-spectrum-color-picker": "0.5.1",
    "ember-suave": "4.0.0",
    "ember-test-selectors": "0.0.3",
    "ember-tether": "0.3.1",
    "ember-truth-helpers": "1.2.0",
    "ember-welcome-page": "^1.0.1",
    "liquid-fire": "0.24.1",
    "loader.js": "^4.0.11",
    "trms-common-components": "trms/trms-common-components"
  },
  "main": "index.js"
}

bower.json

{
  "name": "carousel-ui",
  "dependencies": {
    "Faker": "~3.0.0",
    "bootstrap-sass": "~3.3.4",
    "dinosheets": "0.1.0",
    "ember": "~2.7.0",
    "ember-cli-moment-shim": "0.1.0",
    "ember-cli-shims": "0.1.1",
    "ember-qunit-notifications": "0.1.0",
    "font-awesome": "~4.4.0",
    "highlightjs": "9.1.0",
    "jquery-date-range-picker": "~0.2.1",
    "jquery.browser": "^0.1.0",
    "jqueryui": "~1.11.4",
    "lodash": "~3.7.0",
    "moment-timezone": ">= 0.1.0",
    "nouislider": "^8.0.1",
    "pikaday": "^1.4.0",
    "plupload": "~2.1.3",
    "pretender": "^1.1.0",
    "remarkable": "1.6.2",
    "respond": "~1.4.2",
    "selectize": "~0.12.1",
    "spectrum": "^1.8.0",
    "tether": "~0.7.2"
  },
  "version": "0.0.1",
  "description": "User interface for Carousel API",
  "main": "index.js",
  "license": "MIT"
}
@glenjamin
Copy link

Does this happen every time consistently? What are the contents of the ic-ajax folder listed in the error message?

@raytiley
Copy link
Author

Yup... I tried it with modules already installed from npm and with a clean node_modules folder. The path doesn't actually exist. there is no node_modules in that folder.

ember-cli-ic-ajax -zsh 97x31 2016-10-12 10-24-17

That folders package.json

{
  "name": "ember-cli-ic-ajax",
  "version": "0.2.5",
  "description": "Include ic-ajax into an ember-cli application.",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/rjackson/ember-cli-ic-ajax"
  },
  "keywords": [
    "ember-addon"
  ],
  "author": "Robert Jackson",
  "license": "MIT",
  "dependencies": {
    "ic-ajax": "~2.0.1"
  },
  "bundledDependencies": [
    "ic-ajax"
  ],
  "bugs": {
    "url": "https://github.com/rjackson/ember-cli-ic-ajax/issues"
  },
  "homepage": "https://github.com/rjackson/ember-cli-ic-ajax",
  "devDependencies": {}
}

@mattboldt
Copy link

👍 having the same issue w/ ember-cli. No node_modules folder in ember-cli-ic-ajax

@mattboldt
Copy link

mattboldt commented Oct 12, 2016

Update: it appears to be due to bundledDependencies in ember-cli-ic-ajax's package.json. I forked the project and removed that line, and Yarn installed everything correctly.

@dennislaumen
Copy link

As @mattboldt states in rwjblue/ember-cli-ic-ajax#22 (comment), his workaround only seems to remove this error but still causes issues.

@dmfenton
Copy link

Seems like this is either a big enough bug or not-yet implemented feature that it deserves at least some documentation as a known limitation.

@busches
Copy link
Contributor

busches commented Dec 1, 2016

As an update, this is still an issue in v0.17.10.

@bestander bestander self-assigned this Dec 6, 2016
@bestander
Copy link
Member

Another similar case:

yarn add [email protected]
yarn add v0.19.0-0
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
error Couldn't find a package.json file in ".../node_modules/tap/node_modules/tap-consumer"

Will bump the priority

@TrustNik
Copy link

TrustNik commented Dec 16, 2016

Can also confirm that this is an issue with yarn v0.19.1

@ELLIOTTCABLE
Copy link

Still seeing this at 0.20.0, on tap-consumer.

@ftrimble
Copy link

ftrimble commented Feb 9, 2017

pdf3json is generating the same issue

@jean-francois-labbe
Copy link

jean-francois-labbe commented Feb 24, 2017

I just got the same issue:

yarn install v0.21.2
[1/4] Resolving packages...
[2/4] Fetching packages...
error Couldn't find a package.json file in "/root/.cache/yarn/npm-readable-stream-2.2.2-a9e6fec3c7dda85f8bb1b3ba7028604556fc825e"

It works if i run yarn install a second time, this is not suitable for continuous integration

@bestander
Copy link
Member

PR is welcome

@jklmli
Copy link

jklmli commented Mar 13, 2017

I had a similar issue with circleCI and yarn install --network-concurrency 8 (manually invoked on 0.21.3 per #2829 since I was seeing issues a la #2629):

error An unexpected error occurred: "https://registry.yarnpkg.com/grunt/-/grunt-0.4.5.tgz: ENOENT: no such file or directory, open '/home/ubuntu/.cache/yarn/npm-grunt-0.4.5-56937cd5194324adff6d207631832a9d6ba4e7f0/.yarn-tarball.tgz'".

It turns out this was partially due to running with the --production flag, which is known to sometimes break installs (#761, #1379).

After I removed that, my build succeeded but I noticed 2 issues during yarn install because of concurrent yarn installs. I'm a little bit worried, because yarn doesn't appear to be thread-safe (#683).
Edit: the build after it failed with the same ENOENT error, and I managed to reproduce the error locally as well. I ended up creating a separate yarn cache for each subproject via a command-line argument.


Run with yarn 0.21.3 on node 6.9.4 (local) and 6.10.0 (circleCI).
Issues related to this issue: #1864, #1058

@bestander
Copy link
Member

In the case of pdf3json and [email protected] the packages contain incorrect data.
They claim to have bundledDependencies but the .tgz does not have them.
I suppose npm CLI is loose on this and installs the missing bundledDependencies from npm.

I think Yarn could follow this behavior.
At the moment we expect the community members to pick this issue and send the PR.

@bestander bestander changed the title Install fails with couldn't find package.json fails if bundledDependency is missing in .tgz file May 24, 2017
@bestander
Copy link
Member

bestander commented May 24, 2017

PR to fix this is welcome.
In the end the issue is in the packages as they declare bundledDependencies but don't bundle them.
I think Yarn should fetch the non bundled dependency anyway so a fix is relatively straightforward.

@bestander
Copy link
Member

Raising pri.
We should fix it before 1.0

@rally25rs
Copy link
Contributor

rally25rs commented Jul 25, 2017

With Yarn v0.27...

As far as I can tell, there is no actual code in Yarn for handling bundledDependencies on package install. We simply assume that they will be properly bundled.

The code that actually fails here is when we try to link up bin scripts in package-linker

    // link up the `bin` scripts in bundled dependencies
    if (pkg.bundleDependencies) {
      for (const depName of pkg.bundleDependencies) {
        const loc = path.join(this.config.generateHardModulePath(ref), this.config.getFolder(pkg), depName);

        const dep = await this.config.readManifest(loc, remote.registry);

        if (dep.bin && Object.keys(dep.bin).length) {
          deps.push({dep, loc});
        }
      }
    }

If I comment out that code then I can yarn add [email protected] and it finishes successfully, but of course the missing bundled dependency isn't installed.


The latest ember-cli and ember-cli-ic-ajax seem to already be fixed. [email protected] is weird because it lists 3 bundledDependencies and actually has 2 of them bundled. Only 1 is missing. Gotta wonder how that happened 😕

I'm not a big fan of trying to fix broken packages, but ...

If we attempt to resolve this by installing missing bundledDependencies then we should also make sure it doesn't get hoisted to a different directory. It should stay in the node_modules folder underneath the package that listed it as a dependency.

Bundled deps also don't end up in the lock file, so this will raise some issues about deterministic builds. If the bundledDependency is a range and we try to install one that is missing, and it doesn't get a lock file entry, then it becomes non-deterministic. If we do make a lock file entry, then it may cause issues if the package is fixed and upgraded. If it starts including it's bundled dep properly then we need to remove our lock entry.


(edit)

NPM 5 does seem to install the missing dependency, but does it in a strange way...

If you compare package.json dependencies of actual [email protected], it does not list tap-consumer. However after npm i [email protected] it actually seems to add that dependency to the installed package.json ... that seems odd 😕 Maybe its some sort of "fix broken packages" code and just modifies the package.json? Seems like a bad idea to me...

Strangely, it adds the dependency "tap-consumer": "*", but what it actually installs is version 1.0.0 which is not the latest version...

$ npm i [email protected]

$ grep version node_modules/tap/node_modules/inherits/package.json
  "version": "1.0.0"

$ grep version node_modules/inherits/package.json
  "version": "2.0.3"

So as usual the NPM behavior just confuses me more as to what is "correct"...


I propose going with a warning in the next version. If we agree, I can submit a PR for this:

$ yarn add [email protected]
yarn add v0.28.0
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning "[email protected]" is missing a bundled dependency "tap-consumer". This should be reported to the package maintainer.
[4/4] 📃  Building fresh packages...
success Saved lockfile.
success Saved 17 new dependencies.
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]
✨  Done in 0.87s.

@BYK
Copy link
Member

BYK commented Jul 28, 2017

@rally25rs I agree with your proposed solution. I was actually thinking the very same thing: install the package anyways but give a warning. We can probably be clever and try using the issues field or display the author information in the warning.

@BYK BYK assigned rally25rs and unassigned bestander Jul 28, 2017
@rally25rs
Copy link
Contributor

I'll get a PR put together as soon as I figure out how to unit test this 😆

On a side-note, part of the other problem if trying to "fix" the broken packages is that there is no way to check for missing bundled deps at the resolution step of the Yarn workflow. It has to extract the .tgz first, which doesn't happen until the link phase. I don't think there is a clean way to "add more packages to the install" once you are in the link phase of the workflow.

rally25rs added a commit to rally25rs/yarn that referenced this issue Jul 28, 2017
arcanis pushed a commit that referenced this issue Jul 28, 2017
* [#886] Added test for missing bundledDependencies

* Requested changes for PR 4046
@bestander
Copy link
Member

Fixed now

@Kuraisu
Copy link

Kuraisu commented Aug 4, 2017

Hi!

I have a similar problem but with deps fetched via git. A dependency package has a dependency of it's own that is listed in bundledDependencies, but it is missing, when this package is fetched via git. Yarn was failing before, but now will it just show a warning?

NPM in the same situation retrieves all deps that are listed in bundledDependencies but are missing from node_modules/, so at the end you have a working package.

I understand that this is not an ideal solution, but how should yarn behave in this case? Should it mimic npm or should it warn user and leave his app in an unworkable state?

@rally25rs
Copy link
Contributor

@Kuraisu as discussed above, we decided to get out a quick fix to prevent an error, and added a warning.

My personal opinion (don't take this a the Yarn stance on the topic) is that we shouldn't be fixing broken packages. Just like if the JS code was invalid, the package author should be responsible for that.

From a technical implementation perspective, I wasn't really sure how NPM even determines what to install because bundledDependencies do not contain any version range, but from a quick test NPM didn't give me the "latest" version either. I didn't have a lot of time to dig into their code to figure out the behavior to mimic. Also the code structure of Yarn isn't really conducive to adding new packages at the point where we detect the missing bundledDependency. I think it would have taken some re-architecting of the internal workflow to get that feature added.

I wanted to get something out quickly to at least prevent the errors. As a workaround, you can just yarn add the_missing_package and that should make the package usable again. If you would like to dig into it further, we would love to get a PR that better handles or attempts to install the missing dependencies.

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

No branches or pull requests