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

Plugin adds the generated folders from other sourceset to the main sourceset #334

Closed
AliLozano opened this issue Sep 7, 2019 · 6 comments
Labels

Comments

@AliLozano
Copy link

AliLozano commented Sep 7, 2019

Hi, I'm creating another sourceSet to integration test module and the ide handle it wrong, someone can help me?.

Version: 0.8.10 con kotlin

error proto

My configuration:

sourceSets {
    main {
        java {
            srcDir("src/main/protoGen")
        }
    }
}


val SourceSet.kotlin: SourceDirectorySet
    get() = (this as HasConvention).convention.getPlugin(KotlinSourceSet::class.java).kotlin

sourceSets {
    create("it") {
        kotlin.srcDir("src/it/kotlin")
        resources.srcDir("src/it/resources")
        compileClasspath += sourceSets.main.get().output + configurations["testRuntimeClasspath"]
        runtimeClasspath += output + compileClasspath + sourceSets.test.get().runtimeClasspath
    }
}


val generatedJava = file("${protobuf.protobuf.generatedFilesBaseDir}/main/java")
val generatedGrpc = file("${protobuf.protobuf.generatedFilesBaseDir}/main/grpc")


idea {
    module {
        excludeDirs = excludeDirs.plus(file("gradle"))
        excludeDirs = excludeDirs.plus(file(".idea"))

        generatedSourceDirs = generatedSourceDirs.plus(generatedJava)
        generatedSourceDirs = generatedSourceDirs.plus(generatedGrpc)

        sourceDirs = sourceDirs.minus(project.sourceSets.getAt("it").kotlin.srcDirs)

        testSourceDirs.plusAssign(project.sourceSets.getAt("it").kotlin.srcDirs)
        testResourceDirs.plusAssign(project.sourceSets.getAt("it").resources.srcDirs)

    }
}
@bric3
Copy link

bric3 commented Oct 18, 2019

Hi,

I just stiumbled on this issue, I'm affected by this issue as I don't use it the same way. But I noticed the gradle config uses the idea plugin. I believe JetBrains is phasing out how IntelliJ IDEA uses the *.iml *.ipr files, so this gradle plugin has no effects.
You may have more chances to play with JetBrains's own plugin https://github.com/JetBrains/gradle-idea-ext-plugin.

HTH

@laurenkt
Copy link

laurenkt commented Dec 9, 2020

Not sure if either of you found an answer to this issue.

Will summarize my investigation:

From what I've been able to gather the issue is in ProtobufPlugin#addSourcesToIde.

You can see for both the proto sources and the generated code dirs, it uses a util method Utils.addToIdeSources.

  static void addToIdeSources(Project project, boolean isTest, File f) {
    IdeaModel model = project.getExtensions().findByType(IdeaModel)
    if (model != null) {
      // TODO(zpencer): switch to model.module.generatedSourceDirs when that API becomes stable
      // For now, just hint to the IDE that it's a source dir or a test source dir.
      if (isTest) {
        model.module.testSourceDirs += f
      } else {
        model.module.sourceDirs += f // <-- here
      }

This seems to depend on the idea plug-in -- and as far as I can figure out from looking at the docs from that, there's no support for multiple modules. See gradle/gradle#9681

I had a look at the idea-ext plug-in you mentioned @bric3 - but there is a corresponding issue there (JetBrains/gradle-idea-ext-plugin#22) so it doesn't look like that project supports this either.

Given the outstanding issues on both of those projects I don't think there's likely to be a solution there soon, but I'm wondering if we can raise a PR on this one to at least prevent it from marking it as a source dir using the idea plug-in. Then I think the generated sources could just be added as src dirs for each source set?

@voidzcy
Copy link
Collaborator

voidzcy commented Dec 9, 2020

For main and test source sets, everything should be clean and correct. But for custom source sets, like @AliLozano posted in the original issue, it may contain protos and generated code for main source set as well.

The problem is that the IDEA plugin doesn't provide APIs for adding srcDirs to a specific custom source set, all it has are sourceDirs and testSourceDirs, adding source to sourceDirs will just add to all source sets except test. One thing we could do is to only add protos and generated code of main and test source sets and nothing else. But that would make protos and generated code for other source sets unreferenced (you won't even be able to navigate code in Intellij), which is worse than what they currently have (having more than it should vs. having nothing).

Realistically, this should only affect people compile with Intellij's compiler. But since you are delegating to Gradle, it shouldn't cause any compilation trouble right? If you really don't want to link sources not belonging to the specific source set (e.g., navigating code in custom source set jumps to code in main), it's easy to clean them up manually in Intellij. I don't think there is anything in this plugin that can make it better than how it currently works.

@laurenkt
Copy link

laurenkt commented Dec 9, 2020

You're correct that it doesn't cause any compilation trouble. But it does break IntelliJ features like code navigation (every reference to a generated class in a different sourceSet will appear as if it cannot be resolved, will be red in the editor, and won't support code-sense, navigation, analysis etc).

I think I agree there's not much this plug-in could do to improve things without changes in the IDEA plug-in or IntelliJ gradle support. I think the best we can hope for is just a way to prevent this plug-in from interacting with IDEA plug-in entirely, so we can fix the paths ourselves. Would you be receptive to a PR for ability to disable that?

@voidzcy
Copy link
Collaborator

voidzcy commented Dec 9, 2020

Yeah, you can send us a PR if you can find a better solution. If there is a way to improve the user experience, we are happy to see it. But I don't think we want to completely disable IDEA integrations. I mean working in some extent is better than nothing. Most users do not create custom source sets, so they should be able to see the integration work nicely.

@AliLozano
Copy link
Author

AliLozano commented Dec 10, 2020

Hi!.

In the end I solved my problem in another way, I don't know if it could apply for you... but here it goes..

Since I didn't require integration tests for protos then I created a separated module only for protos with only two sourcesets main/test as it is supported, and others modules for protos consumers with any sourceset that I need, look the image:

image

And If you need to separate your proto in many sourcesets, you could separate it in many modules.

I hope it helps you.

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

No branches or pull requests

4 participants