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

Allow specifying cache outputs outside of package folder #861

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

jaredpalmer
Copy link
Contributor

@jaredpalmer jaredpalmer commented Mar 10, 2022

This allows people to specify outputs outside of the package folder from which the command originated, which is especially useful for code-generation tools and polyglot repos. To enable this, we store artifacts in the cache relative to the root of the monorepo (see screenshot) below for before (top) and after (bottom).

CleanShot 2022-03-10 at 12 58 21@2x

Since we are changing how cache is structured, I updated the global cache key in turbo in order to prevent prior old artifacts from ever ever being cache hits with after this change

The only downside to this change, which might not be a downside at all, I guess, is possibility that artifacts are now tied to the directory structure of outputs relative to the root of the monorepo.

@vercel
Copy link

vercel bot commented Mar 10, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/FVU58zPukLKnbi6aH6HAGvuXk7vK
✅ Preview: https://turbo-site-git-feat-cache-files-outside-of-folder.vercel.sh

taskName: string
): string {
return path.join(cacheDir, ".turbo", `turbo-${taskName}.log`);
return path.join(cacheDir, pathToPackage, ".turbo", `turbo-${taskName}.log`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsoltis is this PR and change going to work us into a weird hole in the future? im thinking that we lose flexibility by storing cache items with their directory structure, but maybe not

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess moving a package will cause a cache miss. That seems fine.

I think we'll always know the path to the package, there's no notion of running turbo in a subtree of the repo without knowing anything about the root of the repo.

We should probably validate that the outputs are inside the monorepo if they are no longer going to be inside the corresponding tasks's package. I don't think we would want a future cache hit to make changes outside of the monorepo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but do we really care about outside of monorepo use case enough to warn against it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose "just don't do that" is ok.

@weyert
Copy link
Contributor

weyert commented Mar 10, 2022

Would this mean when I have a package package1 which part of its build or deploy uploads files to a CDN like Google Cloud Storage which final url would depended on my package2 I could save the url to a file and the package2-package directory and have the cache recover the file when a cache hit occurs?

@jaredpalmer
Copy link
Contributor Author

@weyert it means you can do this: outputs: ['../../../some/where/in/my/monorepo/**/*'] in your pipeline

@kodiakhq kodiakhq bot merged commit 42d7447 into main Mar 10, 2022
@kodiakhq kodiakhq bot deleted the feat/cache-files-outside-of-folder branch March 10, 2022 21:30
@TxHawks
Copy link

TxHawks commented Mar 12, 2022

Could this change also be used in the future to enable adding files outside the package dir to a specific task's dependencies (see #706)?

I realize this isn't it, but does this change make it easier?

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.

4 participants