Skip to content

Add generateFileManifest function for enhanced pattern support#1178

Closed
BrianLang wants to merge 1 commit intorstudio:mainfrom
BrianLang:main
Closed

Add generateFileManifest function for enhanced pattern support#1178
BrianLang wants to merge 1 commit intorstudio:mainfrom
BrianLang:main

Conversation

@BrianLang
Copy link

@BrianLang BrianLang commented Jun 15, 2025

Rather than directly suggesting an update to current implementation of .rscignore files, I'd like to propose the addition of a new exported function that does have more flexible matching than the current .rscignore file approach.

Happy for feedback and a discussion on how we can facilitate this.

Without an appetite for inclusion I'd be willing to just make a small package instead.

  • Introduced generateFileManifest() to process enhanced ignore patterns from .rscignore-patterns files, allowing for advanced file filtering during deployment.
  • Added support for exact filenames, wildcard patterns, and directory exclusions.
  • Implemented a helper function parseRscignoreFile() to handle pattern file parsing.
  • Created comprehensive tests for various pattern matching scenarios.
  • Updated documentation and added a vignette for usage examples and integration with deployApp().
  • Exported the new function in NAMESPACE.

#1118
#774 @ColinFay could this be a path forward? (yes, a very old issue of yours)

- Introduced `generateFileManifest()` to process enhanced ignore patterns from `.rscignore-patterns` files, allowing for advanced file filtering during deployment.
- Added support for exact filenames, wildcard patterns, and directory exclusions.
- Implemented a helper function `parseRscignoreFile()` to handle pattern file parsing.
- Created comprehensive tests for various pattern matching scenarios.
- Updated documentation and added a vignette for usage examples and integration with `deployApp()`.
- Exported the new function in NAMESPACE.
@aronatkins
Copy link
Contributor

Hi, @BrianLang - Thanks for the suggestion and contribution. Seeing your PR has caused us to consider revisiting and expanding the .rscignore rules. We're trying to convince ourselves that this path doesn't break backwards compatibility, but hope to report back soon.

Would richer patterns in .rscignore suffice? What were your reasons for suggesting an alternative?

@BrianLang
Copy link
Author

BrianLang commented Jun 17, 2025

Hi @aronatkins, thanks for the response. I am happy to see this move forward in either case, whether it is an enhancement of the .rscignore processing or if someone makes available a nice helper for creating a file manifest with a similar level of flexibility.

My main consideration was mostly avoiding digging too deep into your codebase. I see that there is a recursive calculation of bundle size, which is facilitated by the fact that you are only ever considering the root-level files and directories with the .rscignore. To change the logic and allow for deeper ignoring would add a lot of complexity to keep that recursive calculation.

But at the same time, I'm unconvinced of the necessity of that computation, couldn't you just do the filtration for the bundle and call sum(file.info(keptFiles)$size)? My ignorance as to why we have such a finessed calculation of size kept me away from wanting to propose a solution directly with the .rscignore. :-)

Thanks!

@karawoo
Copy link
Contributor

karawoo commented Jun 23, 2025

I think we'd like to bring .rscignore in line with .renvignore/.gitignore behavior without adding a separate file. This would also involve continuing to support .rscignore files placed in subdirectories within the file hierarchy. Additionally, we'd have to ensure that the correct precedence of .rscignore files is respected. Right now this is not an issue because .rscignore files can only reference files/folders within their immediate directory. If we expand the rules, then an .rscignore in the root directory could contradict entries in an .rscignore in a child directory; in these cases the child directory's .rscignore needs to take precedence.

It is a fairly big effort so I definitely see why you opted for a separate file first. However I think the best user experience will be for .rscignore to behave just like .renvignore/.gitignore.

Would you be interested in taking a pass at updating this PR? Totally understand if it's more than you want to take on. If that's the case then we'll start work on an implementation when we're able (but no guarantees as to when that will be).

@BrianLang
Copy link
Author

Hi, thanks for this context. I somehow didn't realize it's possible to have multiple .rscignore files to ignore specific files/directories in subdirectories. Is this why the current implementation depends on recursion?

From what you've written, I understand that you'd be interested in a refactor of how .rscignore files are processed such that

  1. their rules are interpreted in the same way as .gitignore files. https://git-scm.com/docs/gitignore
  2. subdirectory .rscignore should be treated with precedence.

The main place I see a possibility for root level .rscignore conflicting with subdir/.rscignore is

  1. if the user puts negation in their root level .rscignore that implies keeping files that subdir/.rscignore removes.
  2. the user ignores a file or directory from their root level .rscignore that the subdirectory/.rscignore negates.

I would propose that we do not support negation. In this way the union of all .rscignore would form the final exclusion list. Is there something I'm missing here?
Thanks!

@karawoo
Copy link
Contributor

karawoo commented Jul 14, 2025

Is this why the current implementation depends on recursion?

Yes, currently the recursive approach allows us to ignore files in subdirectories if there's a .rscignore file present in the subdirectory.

I'd be ok with broadening our rscignore syntax incrementally and not supporting negation yet, but I do think we'd want to implement it eventually, as it's a useful feature that users may expect if they are already familiar with .gitignore.

Another option is to vendor renv and reuse its (unexported) parsing of .renvignore files. That is a little confusing since rsconnect already depends on renv too. But it might be the quickest way to get support for the full syntax.

If we're not implementing the full .renvignore syntax from scratch right at this moment then I think I lean toward the latter option (vendoring renv). But I'd be willing to review a PR that takes either approach as long as it moves us in the direction we want to go with these files, without removing functionality that currently exists.

@BrianLang
Copy link
Author

closing as this will be addressed on a different branch and PR #1185

@BrianLang BrianLang closed this Jul 26, 2025
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.

3 participants