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

[rush] Move cached folder list to a per-project configuration file. #2403

Merged
merged 4 commits into from
Dec 29, 2020

Conversation

iclanton
Copy link
Member

This is one of the TODO items listed in #2393 (comment)

This change:

  • Removes the "projectOutputFolderNames" property from the /common/config/rush/build-cache.json file
  • Removes the "additionalProjectOutputFolderNames" property option from the <project root>/config/rush/build-cache.json file
  • Adds rig and "extends" support to the <project root>/config/rush/build-cache.json file

Open questions:

  • Should a warning be printed if a project doesn't have a build-cache.json in a repo that has build cache support enabled?

This change can be tested in this branch: https://github.com/iclanton/rushstack/tree/ianc/build-cache-example-per-project-config

@iclanton iclanton changed the title [rush-lib] Move cache folders into a per-project configuration file. [rush-lib] Move cached folder list to a per-project configuration file. Dec 22, 2020
@iclanton iclanton changed the title [rush-lib] Move cached folder list to a per-project configuration file. [rush] Move cached folder list to a per-project configuration file. Dec 22, 2020
terminal.writeVerboseLine(
'Project does not have a build-cache.json configuration file, or one provided by a rig, ' +
'so it does not support caching.'
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's a couple interesting edge cases:

  • What if a project has no build script? ("build": "" in package.json) Will the cache ignore it? It should have no impact on downstream projects.
  • What if a project DOES have a build script, but the script does not produce any output. For example, we have an NPM package that contains a bunch of .json files. The "build" script tests them against a JSON schema, and will report an error if they fail, but it does not create any new files.

Copy link
Member Author

Choose a reason for hiding this comment

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

What if a project has no build script? ("build": "" in package.json) Will the cache ignore it? It should have no impact on downstream projects.

When a project has no build script, we short-circuit out of this function early here. This causes the code that creates cache entries to get skipped. So when this function executes, it'll first query the cache, find nothing (because another run of this function won't create a cache entry), and then return at that short-circuit.

This is sub-optimal, but I've been waiting to do a major refactor of how all of this works as part of the multi-phase work, and I want to do that after we've made a release with this feature (including these follow-up PRs). So - not great, but I want to improve it soon.

What if a project DOES have a build script, but the script does not produce any output. For example, we have an NPM package that contains a bunch of .json files. The "build" script tests them against a JSON schema, and will report an error if they fail, but it does not create any new files.

If caching is configured, it'll create a cache entry containing no output assets (i.e. - an empty tarball). That seems like the expected behavior.

@iclanton iclanton force-pushed the ianc/rig-project-output-folders branch from 6c8235d to 09800ed Compare December 25, 2020 02:45
@iclanton iclanton force-pushed the ianc/rig-project-output-folders branch from 09800ed to e6ecb65 Compare December 29, 2020 01:49
@iclanton iclanton merged commit f7d5b6e into microsoft:master Dec 29, 2020
@iclanton iclanton deleted the ianc/rig-project-output-folders branch December 29, 2020 02:31
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.

2 participants