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

Option to limit Dagger factory generation to specific source sets #727

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

matejdro
Copy link

@matejdro matejdro commented Jul 21, 2023

Fixes #718

PR adds extra command line argument to the compiler that receives whitelist of absolute paths within which factory generation can occur. Then gradle plugin generates list of those paths from list of source sets that user specified.

PR now checks every set of Kotlin compiler run against user's allowlist of source sets. If source set is not allowed, it disables factory generation for this run.

@matejdro matejdro changed the title Dagger factories source sets Option to limit Dagger factory generation to specific source sets Jul 21, 2023
Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

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

Three high level comments

  • Absolute paths are likely to cause gradle caching issues as they cannot be cached across machines or even different repo installations
  • With KSP, this doesn't become possible I think?
  • Instead of whitelist/blacklist, prefer allowlist/blocklist as both more descriptive names and more inclusive naming.

@matejdro
Copy link
Author

matejdro commented Jul 27, 2023

Huh you are right about build cache, I totally forgot about that. I just noticed that there is a FilesSubpluginOption (which is already used for build dir argument, that I totally missed). I think switching to that should resolve this issue?

To be honest, I have no idea how KSP works, so I'm not sure if it would work or not. Does KSP not give us a list of files to work with?

I can change the PR to allowlist/blocklist, if desired.

@matejdro
Copy link
Author

Doh, I have managed to find much simpler solution that does not even touch the compiler and thus sidestepping the absolute path issue entirely (possibly also sidesteps KSP problem?).

I have also renamed all whitelist references to allowlist.

# Conflicts:
#	gradle-plugin/src/main/java/com/squareup/anvil/plugin/VariantFilter.kt
@JoelWilcox
Copy link
Member

With KSP, this doesn't become possible I think?

@ZacSweers Are you able to provide more details on this? Is it related to us no longer manually managing the output dir/location?

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.

Add option to disable generating dagger factories for specific source sets
3 participants