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

Incorrect timestamps for files in node_modules #5577

Closed
thomaswelton opened this issue Mar 26, 2018 · 11 comments
Closed

Incorrect timestamps for files in node_modules #5577

thomaswelton opened this issue Mar 26, 2018 · 11 comments

Comments

@thomaswelton
Copy link

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

Bug 🐛

What is the current behavior?
Timestamps for some files are being set in the 1970s

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

Installing react also includes lodash-es and a dependency. The files within the lodash-es directory are saved as having a timestamp in the 1970s

As a result when trying to create a zip file of the node_modules directory I get this error
ZIP does not support timestamps before 1980

This does not occur when I install via NPM
Here is my package.json

{
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "private": true,
  "dependencies": {
    "aws-serverless-express": "^3.1.3",
    "babel-loader": "^7.1.3",
    "bootstrap": "^4.0.0",
    "express": "^4.16.2",
    "history": "^4.7.2",
    "isomorphic-fetch": "^2.2.1",
    "json-api-normalizer": "^0.4.10",
    "loadable-components": "^1.1.1",
    "lodash-es": "4.*",
    "lodash.merge": "^4.6.1",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "react-helmet": "^5.2.0",
    "react-redux": "^5.0.7",
    "react-router": "^4.2.0",
    "react-router-dom": "^4.2.2",
    "react-router-redux": "^5.0.0-alpha.9",
    "redux": "^3.7.2",
    "redux-object": "^0.5.5",
    "redux-saga": "^0.16.0",
    "universal-webpack": "^0.6.2",
    "uuid": "^3.2.1"
  },
  "devDependencies": {
    "babel-cli": "^6.26.0",
    "babel-core": "^6.26.0",
    "babel-minify": "^0.3.0",
    "babel-plugin-transform-runtime": "^6.23.0",
    "babel-polyfill": "^6.26.0",
    "babel-preset-es2015": "^6.24.1",
    "babel-preset-flow": "^6.23.0",
    "babel-preset-react": "^6.24.1",
    "babel-preset-stage-2": "^6.24.1",
    "babili-webpack-plugin": "^0.1.2",
    "chai": "^4.1.2",
    "css-loader": "^0.28.11",
    "file-loader": "^1.1.11",
    "flow-bin": "^0.66.0",
    "jsdom": "^11.6.2",
    "mini-css-extract-plugin": "^0.2.0",
    "mocha": "^5.0.1",
    "node-sass": "^4.7.2",
    "nodemon": "^1.17.1",
    "prop-types": "^15.6.1",
    "redux-devtools-extension": "^2.13.2",
    "sass-loader": "^6.0.7",
    "sinon": "^4.4.8",
    "sinon-chai": "^2.14.0",
    "style-loader": "^0.20.3",
    "webpack": "^4.1.1",
    "webpack-bundle-analyzer": "^2.11.1",
    "webpack-cli": "^2.0.10"
  }
}

What is the expected behavior?
The loads-es directory has files created in 2018

Please mention your node.js, yarn and operating system version.
Node.js: 8.10.0
Yarn: 1.5.1
OS: 10.13.3

@ghost ghost assigned rally25rs Mar 26, 2018
@ghost ghost added the triaged label Mar 26, 2018
@rally25rs
Copy link
Contributor

The timestamps are preserved from what is in the downloaded archive from the registry.

You can download it manually from: https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.7.tgz
and check the contents:

$ tar -tvf lodash-es-4.17.7.tgz
-rw-r--r--  0 0      0         782 Dec 31  1969 package/package.json
-rw-r--r--  0 0      0         363 Dec 31  1969 package/_addMapEntry.js
-rw-r--r--  0 0      0         328 Dec 31  1969 package/_addSetEntry.js
-rw-r--r--  0 0      0         712 Dec 31  1969 package/_apply.js

npm seems to change all the timestamps to "now" which IMO seems incorrect, but what do I know 🤷‍♂️

"fixing" this to do what npm does would cause massive churn in yarn. Really all of yarn caching would have to be redesigned because we compare timestamps between the file in the yarn cache and the one in node_modules, but npm seems to update them every time you install (npm i, rm -rf node_modules, npm i would result in different time stamps)

@yarnpkg/core any thoughts?

@jdalton
Copy link
Contributor

jdalton commented Mar 26, 2018

Cross-posting from the lodash/lodash#3714:

This sounds familiar to what acorn was seeing acornjs/acorn#680 related to npm/npm#19968 and patched npm/npm@58d2aa5. It looks like the solution is to republish.

@rally25rs
Copy link
Contributor

Thanks for the links @jdalton . From the npm discussion:

For the curious: We started using node-tar's noMtime option, which set all the timestamps to 0. The reason for this was so two separate npm pack calls done from the same commit -- even on separate computers, would be able to generate hash-identical tarballs. We didn't really expect that random software out there would have... pathological issues with old timestamps. We still want to preserve the feature, though, so picking an arbitrary date more recent than 1980 seemed like the way to go. Hopefully there's no more incompatibilities!

So apparently npm decided that all packages will now be published with a hard-coded mtime, then on install it'll change it to 'now'? 😕

@bestander
Copy link
Member

There was a similar problem that file permissions were preserved during unpacking, same as timestamps here.
I remember there was some patch in Yarn that would override all permissions but then there were some side effects as well.
This thing is complex, logically Yarn should reset timestamps when copying files to a new location, why does not OS do that?

@rally25rs
Copy link
Contributor

@bestander since we use the file sizes and timestamps to determine if something needs re-copied from the cache to node_modules https://github.com/yarnpkg/yarn/blob/master/src/util/fs.js#L279

Updating all the timestamps would make yarn re-copy every file on every install/add/remove/upgrade, and rebuild all native modules (#932 ).

We already used to call fs.futimes to preserve timestamps on node <8.5, but node 8.5 added fs.copyFile which preserves timestamps on some OSs (OSX) and not others (linux).
Changes in #5470 added more logic to preserve timestamps between cache and node_modules, now calling fs.futimes when needed across node versions and OSs.

I'm guessing npm now relies exclusively on a file hash to determine if things changed, which is something yarn may need to do long-term.

@arcanis
Copy link
Member

arcanis commented Mar 27, 2018

Updating all the timestamps would make yarn re-copy every file on every install/add/remove/upgrade, and rebuild all native modules (#932 ).

We could change the timestamps when unpacking the files in the cache, right?

@bestander
Copy link
Member

Got it.
Seems like we can either "correct" file attributes during unpacking or keep them as is.
Unless there is a bug that requires correction I would prefer not doing anything.

@thomaswelton
Copy link
Author

loadash-es has been republished. If my PR to react-redux gets merged I can continue to use yarn without issue.

It seems like the root of the problems were due to a bug with npm that has now been resolved.
So it feels like any changes to yarn aren't really necessary to fix time stamps.

What could be useful is a warning when running yarn if it unpacks a dependency that contains an obscure timestamp as this would help identify which of your dependencies need republishing.
But because that is a feature request and not a bug I'm going to close this ticket.

@alec-ferguson-sunrun
Copy link

Since lots of modules out there have this issue, I use the following workaround:

# Create a file that was modified February 1, 1980 at 12:34 PM (arbitrary date, but old enough)
# Need to use this file as a comparison
touch -t 198002011234 timefile

# Find files older than timefile. Execute `touch` on them, which modifies file access time.
find node_modules/ -type f -not -newer timefile -exec touch '{}' \;

# Clean up
rm timefile

In a couple projects I've added this as a package.json script:

"scripts": {
  "postinstall": "./fix-times-in-node-modules.sh",

@arcanis
Copy link
Member

arcanis commented Aug 20, 2018

Fyi, Yarn 1.9.4 should have a fix for that (and it bumps the cache folder version, so it should start using the right mtime right off the bat). We now automatically set the mtime at unpack time (to the current time).

@alec-ferguson-sunrun
Copy link

alec-ferguson-sunrun commented Aug 20, 2018

Sweet, thanks @arcanis !

Curious though -- does modifying access time for that many files mean installs take longer? I threw in the -not -newer timefile to my script above since simply running touch on all files took too long.

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

6 participants