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

[turborepo] Newly-cloned git cloned monorepo reports erroneous "cache hit" for a common monorepo lib package #5025

Closed
benduran opened this issue May 19, 2023 · 7 comments · Fixed by #5038
Labels
kind: bug Something isn't working needs: triage New issues get this label. Remove it after triage

Comments

@benduran
Copy link

benduran commented May 19, 2023

What version of Turborepo are you using?

1.9.8

What package manager are you using / does the bug impact?

npm

What operating system are you using?

Mac

Describe the Bug

I have one package that is a shared utility library, which is imported by a number of other packages. This package is declared as a devDependency to get the NPM Monorepo link, but to keep it out of our consumer's node_modules when they run npm install.

Here is how one of the consuming packages that uses this shared utility library is declaring the monorepo dep:

"devDependencies": {
    "@internal/devspaces-flowify-js-utils": "^0.0.0",
    "app-root-path": "^3.1.0",
    "open": "^7"
  },

And here is the shared utility library's package.json file:

{
  "private": true,
  "name": "@internal/devspaces-flowify-js-utils",
  "description": "A collection of shared utilities that are useful for any VSCode plugin or its compiled core",
  "main": "dist/index.js",
  "typings": "dist/index.d.ts",
  "types": "dist/index.d.ts",
  "version": "0.0.0",
  "scripts": {
    "build": "run-s clean typecheck bundle",
    "bundle": "node ./scripts/bundle.js",
    "clean": "rm -rf ./dist",
    "lint": "eslint './src/**/*.{tsx,ts,jsx,js}' --no-error-on-unmatched-pattern",
    "lint:fix": "eslint './src/**/*.{tsx,ts,jsx,js}' --no-error-on-unmatched-pattern --fix",
    "prepublishOnly": "npm run --prefix ../.. build -- --filter @flowify/devspaces-flowify-js-utils",
    "typecheck": "tsc --project ./tsconfig.json --noEmit"
  },
  "devDependencies": {
    "@types/command-exists": "^1.2.0",
    "@types/fs-extra": "^11.0.1",
    "@types/unzipper": "^0.10.6",
    "@types/tar": "^6.1.5"
  },
  "dependencies": {
    "@antongolub/git-root": "^1.5.10",
    "chalk": "^4",
    "command-exists": "^1.2.9",
    "fast-glob": "^3.2.12",
    "fs-extra": "^11.1.1",
    "got": "^11",
    "unzipper": "^0.10.14",
    "tar": "^6.1.15"
  }
}

This utility package keeps reporting a cache hit, even after I've blasted everything away in the repo withgit clean -xdf, which should wiping out the Turbo cache (see images below for behavior):

Top-level project folder structure

top-level-structure

Monorepo packages folder structure

package-structure

Erroneous Cache Hit with no node_modules folder

CleanShot 2023-05-18 at 17 31 45

Even after a fresh repository clone, the package reports a cache hit, which leads me to believe that there's either a bug with the Caching mechanism, or Turbo's Cache location is erroneously reported as node_modules/.cache/turbo in the official documentation.

Downgrading clear back to 1.6 has corrected this issue, so I am unblocked for now, but I am concerned there's a lingering bug that is causing this issue.

Expected Behavior

I would expect there to not be a cache hit, since I blasted away all non-SCM files. I would also not expect a cache hit if I re-cloned the git repository, even if the repository was cloned at some point in the past, but was removed before re-cloning.

To Reproduce

Please refer to screenshots in the bug description. The shared utility library is compiled to plain JS (with *.d.ts files) using tsup.

Reproduction Repo

An internal, enterprise repository

@benduran benduran added area: turborepo kind: bug Something isn't working needs: triage New issues get this label. Remove it after triage labels May 19, 2023
@benduran
Copy link
Author

I realized I shared the turbo.json in a screenshot in the
description, but not the actual code snippet itself. Here it is:

{
  "pipeline": {
    "build": {
      "dependsOn": ["^build"],
      "outputs": ["dist/**", "build/**"]
    },
    "lint": {
      "outputs": []
    },
    "lint:fix": {
      "outputs": []
    },
    "start": {
      "dependsOn": []
    },
    "test": {
      "dependsOn": [],
      "inputs": ["src/**/*.tsx", "src/**/*.ts"],
      "outputs": []
    },
    "typecheck": {
      "outputs": []
    }
  }
}

@tknickman
Copy link
Member

😄 Thanks for that, I saw that screenshot as soon as I posted the comment! We're looking into this one now.

One of the theories is that this is related to a bug the Daemon and our file watching. We do file watching to monitor files that are changed to ensure we only actually restore when things have changed that impact the outputs. That means that you can have remote caching disabled, delete your local cache directory, and still get a cache hit as long as your outputs still exist, and you haven't changed anything related to your inputs.

However, a fresh clone and build should definitely not get a cache hit (since outputs should definitely not already exist in this case).

@benduran
Copy link
Author

benduran commented May 19, 2023 via email

@tknickman
Copy link
Member

Did some investigation here, this does look like an issue with the daemon, we're working on a fix, but in the meantime can you verify that running with --no-daemon works as expected?

@benduran
Copy link
Author

confirmed --no-daemon doesn't experience the issue, but experienced it immediately when running with the default settings

chris-olszewski added a commit that referenced this issue May 22, 2023
### Description

Fixes #5025

In Go code we would shut down the daemon whenever the repository root
was removed. This PR adds that logic back by watching the root and
exiting the daemon if the root is removed.

### Testing Instructions

Added unit test. Follow instructions in reproduction repository in
`uncurated-tests/turborepo-netflix-cache-repro`

---------

Co-authored-by: Alexander Lyon <arlyon@me.com>
Co-authored-by: Chris Olszewski <Chris Olszewski>
gsoltis pushed a commit that referenced this issue May 22, 2023
### Description

Fixes #5025

In Go code we would shut down the daemon whenever the repository root
was removed. This PR adds that logic back by watching the root and
exiting the daemon if the root is removed.

### Testing Instructions

Added unit test. Follow instructions in reproduction repository in
`uncurated-tests/turborepo-netflix-cache-repro`

---------

Co-authored-by: Alexander Lyon <arlyon@me.com>
Co-authored-by: Chris Olszewski <Chris Olszewski>
gsoltis pushed a commit that referenced this issue May 22, 2023
### Description

Fixes #5025

In Go code we would shut down the daemon whenever the repository root
was removed. This PR adds that logic back by watching the root and
exiting the daemon if the root is removed.

### Testing Instructions

Added unit test. Follow instructions in reproduction repository in
`uncurated-tests/turborepo-netflix-cache-repro`

---------

Co-authored-by: Alexander Lyon <arlyon@me.com>
Co-authored-by: Chris Olszewski <Chris Olszewski>
gsoltis pushed a commit that referenced this issue May 22, 2023
### Description

Fixes #5025

In Go code we would shut down the daemon whenever the repository root
was removed. This PR adds that logic back by watching the root and
exiting the daemon if the root is removed.

### Testing Instructions

Added unit test. Follow instructions in reproduction repository in
`uncurated-tests/turborepo-netflix-cache-repro`

---------

Co-authored-by: Alexander Lyon <arlyon@me.com>
Co-authored-by: Chris Olszewski <Chris Olszewski>
@gsoltis
Copy link
Contributor

gsoltis commented May 23, 2023

@benduran Can you give turbo@1.9.9 a try and confirm that the default configuration is working for you?

@benduran
Copy link
Author

@gsoltis 👋 tried out this morning and I can confirm it's working as expected. Thank you for the swift patch 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working needs: triage New issues get this label. Remove it after triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants