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] Allow project config to specify non-build-related files #2618

Closed
elliot-nelson opened this issue Apr 17, 2021 · 8 comments
Closed

[rush] Allow project config to specify non-build-related files #2618

elliot-nelson opened this issue Apr 17, 2021 · 8 comments
Labels
enhancement The issue is asking for a new feature or design change

Comments

@elliot-nelson
Copy link
Collaborator

Summary

Often, projects contain files that will never impact the build (for example, .psd files designers check in as reference material). If a project could specify, in rush-project.json, that *.psd was excluded for build, then we could ignore matching files when computing the hash for that project. In turn, this means that the build cache feature would automatically ignore those files when deciding whether it needed to build a project.

The end result would be that (in this case) designers checking in a .psd file into a project that ignores *.psd would have a lightning-fast PR build, since rush build would automatically conclude that nothing had to be built.

For example:

{
    "projectOutputFolderNames": ["lib", "dist"],
    "ignoredBuildFiles": [
        "*.psd",
        "docs/*"
    ]
}

Details

  • My gut is that rush-project.json is the right place to put this, since setting universal settings across the monorepo could get tricky (one project's .md files might just be documentation, another's might build a static website, for example). If you wanted repo-wide settings, I imagine this new option could be rig'd?

  • I imagine this option as an array of file globs to ignore. It could potentially be more powerful if it was instead the array of file globs to include in the hash, with ['**'] being the default value, and my example above being an override of ['**', '!*.psd']. I'm not sure if that would conflict with other ways to ignore files during hashing (like various .dotfiles).

  • I'm not sure if this setting really has any impact outside of the new build cache feature. It could possibly be put inside the "buildCacheOptions" key in rush-project.json, unless the package dep hashing might be used in other features in the future that aren't build-cache-related.

  • This new feature is similar to [rush] exclude files from rush change diff calculation #2263, but for computing hash instead of calculating rush diff changes. I'm still waffling on whether adding files to this list should also exclude them from rush diff changes... I think the answer is no, because that feature already gives you a way to do that, and if a file should never impact build hash or rush diff, you can just add it in both places.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version? 5.43.0
rushVersion from rush.json? 5.43.0
useWorkspaces from rush.json? true
Operating system? Mac
Would you consider contributing a PR? Yes
Node.js version (node -v)? 12.17.0
@iclanton
Copy link
Member

I like this idea. I think an ignore list is safer/less likely to be used incorrectly than an include list.

Seems like rush-project.json is the right place to put this. @elliot-nelson - would you be interested in putting together a PR to add this feature?

@elliot-nelson
Copy link
Collaborator Author

I'll take a stab at it this week!

@octogonz octogonz added the enhancement The issue is asking for a new feature or design change label Apr 20, 2021
@octogonz
Copy link
Collaborator

It's interesting to compare this design with other ignore files commonly found in a Rush repo:

Repo-level:

  • .gitignore for files that should not be tracked by Git
  • .prettierignore for Prettier

Project-specific:

None of these files are riggable. The .gitignore file can be inherited from parent folders, which works because of blazingly fast C++ code. ESLint tried to do the same thing with .eslintignore and discovered that it is performance problem for Node.js, so they introduced a "root": true setting to disable that.

The above does somewhat suggest that your proposed "ignoredBuildFiles" setting should be its own file .buildignore. But I agree that we probably want it to be riggable, since how stuff gets built is intimately associated with your rig. I can't think of an intuitive way to put an ignore file in a rig. (Any ideas?)

A different viewpoint would be to keep "ignoredBuildFiles", and recommend to convert .changeignore (whose PR has not been merged yet) to a setting in rush-project.json also. In his PR @mmkal's motivating example was rush change ... --ignore '**/*.test.ts' --ignore '**/*.md which definitely seems like something that could go in a rig, to avoid copying a .changeignore file into every project folder.

@octogonz
Copy link
Collaborator

  • I'm not sure if this setting really has any impact outside of the new build cache feature. It could possibly be put inside the "buildCacheOptions" key in rush-project.json, unless the package dep hashing might be used in other features in the future that aren't build-cache-related.

The cloud build cache has somewhat different logic from the classic rush build incremental calculation. @iclanton speculated that eventually the classic rush build can be completely eliminated. Since they are solving the exact same problem, ideally "ignoredBuildFiles" should apply to both implementations. If that is expensive to implement, we can document that "ignoredBuildFiles" requires the cache.

@octogonz
Copy link
Collaborator

octogonz commented Apr 20, 2021

  • I imagine this option as an array of file globs to ignore. It could potentially be more powerful if it was instead the array of file globs to include in the hash, with ['**'] being the default value, and my example above being an override of ['**', '!*.psd']. I'm not sure if that would conflict with other ways to ignore files during hashing (like various .dotfiles).

Ignore files are fairly tricky:

  • Typically evaluating globs is CPU intensive and disk intensive, especially when developers string together pattern-matcher API calls rather than delegating the whole problem to a heavily optimized matching engine.
  • When rules override other rules, the logic gets confusing really fast, especially if overrides can be inherited and merged via "extends"
  • Debugging problems ("why didn't this particular file get published?!") ideally requires a diagnostic report that can explain which rule was applied for each file, but nobody ever implements that. The scenario of "Why didn't this particular file change trigger an incremental build?" seems particularly bewildering.

@mmkal
Copy link
Contributor

mmkal commented Apr 21, 2021

Having thought a bit more about the .changeignore proposal I made, and this request, I think rush could provide most value by using just .npmignore. .npmignore defines what's published/not published in an npm package, so it's the correct way to make these kinds of determinations. If you add *.psd to .npmignore then rush could use that to make various decisions about what to do when a psd file changes (in this case - nothing. It could reuse the cached output/skip rebuilding for the project and all its dependents)

@elliot-nelson
Copy link
Collaborator Author

elliot-nelson commented Apr 22, 2021

🤔 I like the idea of reusing an existing paradigm, but it doesn't seem like it works for the dev dependencies. That is, you generally don't publish your unit tests (for example), but if you touch one you still want to rebuild that project (to result in a build and test).

(I'm referring to computing project state hash; for the rush change situation maybe npmignore is a good substitute.)

@octogonz
Copy link
Collaborator

PR 2643 was released with Rush 5.47.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change
Projects
Archived in project
Development

No branches or pull requests

4 participants