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

Idea: create per-sourceset task plus the default one for files outside of sourcesets #18

Open
vlsi opened this issue Sep 4, 2020 · 5 comments

Comments

@vlsi
Copy link
Contributor

vlsi commented Sep 4, 2020

The current approach with a single task is not very build-cache friendly: it has to re-compute all the files no matter what has changed.

What if the plugin generated multiple tasks (e.g. per-source set)?

Note: there are files outside of sourcesets, so there should be a task that verifies them as well. I wonder if there's Gradle out-of-the-box way to create such a task that would verify all files in the root project, yet it won't dig into subproject's files?

Full RAT for Apache Calcite takes ~17sec

Build scan: https://scans.gradle.com/s/mwgkq3a7ozuzm

@eskatos
Copy link
Owner

eskatos commented Sep 13, 2020

Hi Vladimir,

This is a good suggestion. But, as you wrote, there wouldn't be a straight forward way to know the "set of files not included in source sets" without reaching across projects or doing some other gymnastic. We could try to figure out an elegant way to do it but it won't be trivial and will impose constraints. From the top of my head I can think of aggregating the set of files covered by source sets tasks and then using that set as excludes on the "not in source sets". It would mean that the "not in source sets" task would have to wait for all "source set tasks" to be run. There might be other ways.

That being said, a plugin that sets up one task per source set could be enough for some users that only want to check the actual sources. This is the first time this need is expressed. But if I understand correctly you got to this idea because of the performance aspect, not because of the pure use case point of view.

Performance here is two fold. There's the time it takes to snapshot everything and there's the fact that given all files are in a single bucket the task gets invalidated as soon as one file changes.

Making the rat task incremental could also be another way to make its execution faster. I'm not sure this is possible given the current RAT api, maybe it is, I didn't investigate that yet.

@vlsi
Copy link
Contributor Author

vlsi commented Sep 13, 2020

It would mean that the "not in source sets" task would have to wait for all "source set tasks" to be run

AFAIK, sourceSet is always defined as a set of directories.
Then, catch all task could have the sourceset like fileTree(projectDir) { exclude(sourceSetDir1); exclude(subproject1dir); ...}
It does not really need to wait for other tasks to start or complete. It could just ignore the relevant directories.

From the top of my head I can think of aggregating the set of files covered by source sets tasks and then using that set as excludes on the "not in source sets

Well, recently there was a discussion on cross-project configuration, and it looks like "code analysis" tools need a way to split the files as follows:
a) per-source set. It seems to be possible, and it is usable
b) "all other files, except files from other projects". For instance, I want to validate $rootDir/build.gradle.kts, however, I do not want "catch all" task from the root project to dig into subproject/lib/... files
c) Exclude .gitignore files by default. Sourceset-based tasks do not suffer from garbage content, however, as one tries to process something like $rootDir/**.java, then they immediately face folders like build/... and others. Of course, it is not clear if Gradle should be gitignore-aware, however, I know Gradle does inherit certain ignore patterns from Ant, so it would probably make sense to support vcs-driven exclude patterns.

I have the same use-case in https://github.com/autostyle/autostyle, and my current workaround is a lazy extra in the root project:

        extra[PROJECT_DIR_MAP] = lazy {
            allprojects
                .asSequence()
                .flatMap { sequenceOf(it.projectDir, it.buildDir) }
                .plus(File(rootDir, "buildSrc"))
                .plus(File(rootDir, ".gradle"))
                .plus(File(rootDir, ".idea"))
                .mapTo(TreeSet()) { it.absolutePath + File.separatorChar }
        }

Making the rat task incremental could also be another way to make its execution faster

I don't think Gradle supports "incremental from build cache" execution mode yet.
In other words, making the task incremental would help for the local development (which is nice), however, it won't improve CI build times.

@eskatos
Copy link
Owner

eskatos commented Sep 13, 2020

Your idea uses allprojects and I was trying to think of a solution that doesn't reach across projects. Each project could publish the set of checked files so that the "not in source sets" task can consume that and use it as excludes.

making the task incremental would help for the local development (which is nice), however, it won't improve CI build times.

That's right.

@vlsi
Copy link
Contributor Author

vlsi commented Sep 13, 2020

Your idea uses allprojects and I was trying to think of a solution that doesn't reach across projects.

:)

Ah, you've caught me :)
I just thought you have a way to avoid allprojects.

@eskatos
Copy link
Owner

eskatos commented Sep 13, 2020

:)

I just thought you have a way to avoid allprojects.

Yep, publications. But it's quite involved and I would like to take the time to think about / discuss how a solution would look before committing to implementing this. There might be simpler alternatives.

A naïve "workaround" could be adding a **/src/** exclude to the catch all task. Not perfect but it could cover most of the use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants