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

npm-base: Move cache dir to persistent location #64

Closed
wants to merge 1 commit into from

Conversation

dzudek
Copy link

@dzudek dzudek commented Feb 7, 2017

Pointing NPM_CACHE_DIR_NATIVE to store cache under DL_DIR.
It wouldn't be required anymore to download all the packages after
removing TMPDIR before doing a clean build.

Pointing NPM_CACHE_DIR_NATIVE to store cache under DL_DIR.
It wouldn't required anymore to download all the packages after
removing TMPDIR before doing a clean build.
@raymanfx
Copy link
Contributor

@imyller any news on this one?
It was a pain to track down this issue on our servers.

@raymanfx
Copy link
Contributor

(same story here: imyller/meta-nodejs-contrib#5)

@bachp
Copy link
Contributor

bachp commented Feb 21, 2017

I think this makes sense. As far as I can tell the cache really only stores the downloaded tarball and the package.json.

@imyller
Copy link
Owner

imyller commented Feb 22, 2017

I'm not saying no to this, but I'd like to explain earlier reasoning for placing npm cache to workdir:

It's not quite that simple. Earlier I learned by error-and-trial that npm cache is not guaranteed to be cross-platform portable and has limited portability between npm versions (especially if switching backwards).

This caused issues in environments where packages with different Node.js version dependencies exist and/or build is made for several machine architectures using same OE environment (e.g. repository builders). Sharing common npm cache for these situations caused unexpected package build failures. Especially for npm packages containing native libraries. npm cache simply is not designed to be quite that portable.

Safe solution, and the "OE way", is to make sure that each and every package build cycle/workdir gets clean starting point without leftovers from earlier builds on another architecture or build tool version. One way to safely do this with npm is to place npm's cache to workdir. Downside is that cache lifetime is only for the package build (especially if if oe_rmwork is enabled).

To be honest, I haven't checked the situation with npm cache portability for a while, so I'm open for suggestions how to get the best of the both solutions. Maybe prefix cache dir with proper {target/build}_arch and.. maybe if required.. npm version?

Your thoughts?

@imyller
Copy link
Owner

imyller commented Apr 27, 2017

I'd like to get this in too along with #68, but issues remain with supporting multiple Node.js and NPM versions. NPM does not really handle well jumping versions back and forth with common cache dir.

With large builds having packages depending on different Node.js versions this causes very hard to debug build failures.

So far only reliable solution has been placing cache in (temporary) workdir.

Shared npm cache dir would be nice, but I'd hate to cause breakage.

Suggestions are welcome.

/cc @KurtSchluss @mdavis777

@KurtSchluss
Copy link

How does OE support different nodejs versions within one build? - Is that supported at all?
I am not familiar with the download dir structure which is created for or from npm, but it sounds to me like there has to be version and architecture dependencies introduced.

@imyller
Copy link
Owner

imyller commented Apr 27, 2017

I'm thinking we could append target arch and npm version to cache dir name? If those two remain the same, it should be safe to assume full compatibility.

@mdavis777
Copy link
Contributor

That would be my first guess on a solution. Target Arch is easy to add, but NPM version would be more difficult. I would assume you would have to have the nodejs recipe set some new global var and npm.bbclass then use it.

@imyller
Copy link
Owner

imyller commented Apr 27, 2017

Since all npm usage goes through oe_runnpm wrapper we could do version detection there with simple npm -v?

@imyller
Copy link
Owner

imyller commented Apr 27, 2017

Going further, we could also add variable like NPM_SHARED_CACHE ?= "0" (or "1") allowing bypassing this automatic cache dir prefixing.

That way you could have fully common/shared npm cache (with your own risk) or safely arch+npmver automatically prefixed cache dir.

@mdavis777
Copy link
Contributor

Did some rough work on it. Seemed to work fine.
https://github.com/mdavis777/meta-nodejs/commit/c6a034cd66017db32cc4f5b175e9a9f8a31137ae

@imyller
Copy link
Owner

imyller commented May 4, 2017

Exactly what I had in mind. I'll test this while jumping between major Node.js versions, but I do not expect any problems.

@KurtSchluss
Copy link

Looks nice. Just one question: Shouldn't npm -v be called via ${NPM}?

@imyller
Copy link
Owner

imyller commented May 6, 2017

Yes, with ${NPM}. Good catch.

Also in my testing this breaks compatibility with recipes that define their own custom cache dir by overriding NPM_CACHE_DIR. We have to check if NPM_CACHE_DIR is defined when calling oe_runnpm. If it is, use that. If not, generate version prefixed dir.

@mdavis777
Copy link
Contributor

1e68570

@KurtSchluss
Copy link

Looks good to me. How can we proceed here? May you create a pull request?

@imyller
Copy link
Owner

imyller commented May 10, 2017

Replaced by PR #70

@imyller imyller closed this May 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants