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

Packages with build artifacts missing, but listed in .yarn-integrity, should be rebuilt. #3781

Open
CrabDude opened this issue Jun 30, 2017 · 5 comments

Comments

@CrabDude
Copy link
Contributor

CrabDude commented Jun 30, 2017

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

What is the current behavior?

Packages are not rebuilt if all non-build artifacts already exist at the destination. This results in errors like:

module.js:327
    throw err;
    ^

Error: Cannot find module '../build/Release/hash'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/.../node_modules/xxhash/lib/xxhash.js:4:13)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)

If the current behavior is a bug, please provide the steps to reproduce.

$ yarn add xxhash
$ rm node_modules/xxhash/build/Release/hash.node
// Bring in an updated yarn.lock (i.e., via git)
$ yarn 
// xxhash does not get rebuilt

What is the expected behavior?

If a file contained in .yarn-integrity is missing (or maybe even the wrong size or mtime), the package should be rebuilt.

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

[email protected]
yarn@master:HEAD (69574f6)
Docker ubuntu12.04

Related:
#231 #1955 (comment)
#1955 (comment)

@bestander
Copy link
Member

Good find

@bestander
Copy link
Member

Send a PR please :)

@CrabDude
Copy link
Contributor Author

CrabDude commented Jul 5, 2017

For posterity, this was exacerbated by previous issues (seemingly fixed, mostly by --check-files I believe) that purged build artifacts, which caused previous failures (now fixed) to not get fixed since the dependency in question was cached and unchanged. (CAVEAT I think)

@CrabDude
Copy link
Contributor Author

This was largely addressed by #3752, which ensures that interrupting install does not result in unbuilt packages / package artifacts getting removed. However, should artifacts be touched inappropriately in the future by an offender other than #3752, it would still be nice to have this check in place.

@rally25rs
Copy link
Contributor

I think I can rig up a PR for this (I came up with it while researching another bug) but I think we should consider the balance of speed vs checking everything.

What are the chances that a previously installed package will be missing build artifacts? We can check all packages for their artifacts every time, but it'll be another loop added to the code to fs.lstat each artifact file to see if it exists. The filesystem isn't notoriously fast.

Do you think this is worth adding more filesystem i/o to check what is likely not a widespread issue? If so, I'll put up a PR. @CrabDude @bestander

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

4 participants