gradle / gradle-native

The home of Gradle's support for natively compiled languages
https://blog.gradle.org/introducing-the-new-cpp-plugins
Apache License 2.0
92 stars 8 forks source link

Publish and consume headers-only dependency transitively #901

Closed amz-shahji closed 5 years ago

amz-shahji commented 5 years ago

Multiple public distributions (like boost) include headers only. Currently, there isn't an easy way generate or consume such dependencies. Plus, no way to make it transitive even with known workarounds.

Expected Behavior

A new linkage type, say Linkage.HEADERS, which would get propagated transitively .

Current Behavior

No way to declare a header-only dependency. Both 'api' and 'implementation' dependencies expect a library and fail at link time. The workaround at the moment is to explicitly add the dependency like the following at every level in the project hierarchy. Maintenance nightmare.

afterEvaluate { dependencies { cppCompileDebugShared "$group:boost:+" cppCompileReleaseShared "$group:boost:+" cppCompileDebugStatic "$group:boost:+" cppCompileReleaseStatic "$group:boost:+" } }

Context

We use a number of projects that are headers only and are used at very low level. Some we even publish as part of our own development.

Steps to Reproduce (for bugs)

Your Environment

lacasseio commented 5 years ago

An alternative to the workaround is to generate a single source file with unique symbols that isn't used anywhere. Given the linker flag to remove dead code is passed this result into a no-op and the header will be propagated correctly.

lacasseio commented 5 years ago

Currently, the dependencies should be transitive as long as the dependency is declared as api in the transitive project.

If you are referring to publishing a Zip containing all the transitive headers, then it should be handled the same way the shadow plugin do it in Java which creates a new publishing artifact which contains all the header you want to publish.

lacasseio commented 5 years ago

As for the Linkage.HEADERS, I'm not sure it's the right approach. What would library.linkage = [Linkage.STATIC, Linkage.HEADERS] means. It sounds like a hack on what we already have. A header-only library is a design choice where no compilation or linking should happen (for the main component). I would maybe see it as a 3rd plugin cpp-header-library. The issue with that approach is the multiplication of plugins. However, we could tailor a library extension for that exact purpose, aka. no compile task, link task, source, private headers, implementation dependency, etc.

amz-shahji commented 5 years ago

Currently, the dependencies should be transitive as long as the dependency is declared as api in the transitive project.

The issue is that the dependency cannot be declared as api because then a shared linkage expects a lib to link which doesn't exist.

I would maybe see it as a 3rd plugin cpp-header-library.

I like the idea of having an explicit plugin for this. @big-guy suggested the Linkage approach and seemed like relatively less work. But I agree that combination of Linkage options become ambiguous, unless Linkage.Static and Linkage.Shared stop meaning "publish the headers as well". This will be a breaking change though. @big-guy might be able to better defend his suggestion.

Nitpick - I would call this cpp-headers and not misuse the term 'library'.

amz-shahji commented 5 years ago

@lacasseio An alternative to the workaround is to generate a single source file with unique symbols that isn't used anywhere.

Not an option for projects that we don't own because it adds additional undesired maintenance for the team and an upgrade nightmare.

@lacasseio However, we could tailor a library extension for that exact purpose, aka. no compile task, link task, source, private headers, implementation dependency, etc.

Alternatively, an extension that generates this dummy/placeholder source file and let cpp-library do its usual stuff.

@lacasseio @big-guy Is the extension approach an acceptable long term solution?

lacasseio commented 5 years ago

The proposal here isn't to ask the developer to add and maintain source files but have Gradle generate a source file with a deterministic symbol prior to building the code. Gradle will build this code and most likely never compile it again. The source file is not checked inside the source control or anything. It's just generated.

We could patch it that way but when we will move toward a cpp-header-library it will be properly modeled. The plugin is the long-term solution. The short-term is to generate a single source file directly in Gradle. It can be packaged as a custom plugin so it can be reuse internally.

amz-shahji commented 5 years ago

@lacasseio That sounds good. Can you provide some implementation specific details so we can move forward on this task. Is there an example that I can follow to guide me thru' the process of writing an extension? Any other documentation link would help also.

I "hope" moving from this short-term solution to long term solution doesn't mean having to make significant changes to the DSL. I don't see needing big changes but something to strive for when implementing the longer term solution.

amz-shahji commented 5 years ago

Duplicate of #447

amz-shahji commented 5 years ago

Duplicate of #739

amz-shahji commented 5 years ago

@lacasseio @big-guy Here's my attempt to implement this as an external plugin. Running into an issue where applying the cpp-library plugin from within my custom plugin-in doesn't take the inputs from DSL.

Plugin implementation -

open class CppHeadersPlugin : Plugin<Project> {
    override fun apply(project: Project): Unit = project.run {
//        pluginManager.apply(CppLibraryPlugin::class.java)
        project.plugins.apply(CppLibraryPlugin::class.java)

        // Force "Static" linkage only since "Shared" doesn't really make sense for this use case
        // Note that this possibly overrides user intention, which would be wrong anyways
        val cppLibraryExtension = extensions.getByType(CppLibrary::class.java)
        cppLibraryExtension.linkage.set(setOf(Linkage.STATIC))

        with(tasks) {
            val generateCppHeadersSourceTask = create("generateCppHeadersSource", CppHeadersGenerateSourceTask::class.java)

            afterEvaluate {
                withType(CppCompile::class.java).forEach { compileTask ->
                    compileTask.dependsOn(generateCppHeadersSourceTask)
                }
            }
        }
    }
}

Task implementation -

open class CppHeadersGenerateSourceTask : DefaultTask() {
    companion object {
        const val FILENAME_PREFIX = "_cpp_headers_"
        const val FILENAME_SUFFIX = "_.cpp"
    }

    private var outputFile: File

    init {
        var outputDir = File(project.buildDir, "generated${File.separator}cpp-headers")
        outputFile = File(outputDir, "$FILENAME_PREFIX${project.name}$FILENAME_SUFFIX")

        project.afterEvaluate {
            // Do it again here, DSL could have overridden "buildDir"
            outputDir = File(project.buildDir, "generated${File.separator}cpp-headers")
            outputDir.mkdirs()

            outputFile = File(outputDir, "$FILENAME_PREFIX${project.name}$FILENAME_SUFFIX")
            outputs.file(outputFile) // Add the outputFile to the task outputs to insure we are properly monitoring

            extensions.getByType(CppComponent::class.java).source.from(outputDir)
        }
    }

    @Suppress("unused")
    @TaskAction
    fun runTask() {
        outputFile.writeText("void _cpp_headers_${project.name}_() {}")
    }
}

Build script -

apply plugin: 'cpp-headers'

version = 'dev'

library {
    publicHeaders.from project.file('.')    // This input is getting ignored
}

Since the library block is being ignored, the ZipTask fails (no output file) and so publish fails. Note that the build succeeds but the publish fails.

Please advise!

amz-shahji commented 5 years ago

@lacasseio @big-guy Can I get some input on this please. Thanks!

amz-shahji commented 5 years ago

@lacasseio @big-guy Ping!!

lacasseio commented 5 years ago

I wrote a native sample to demonstrate how you can currently do this with Gradle.

amz-shahji commented 5 years ago

@lacasseio That's wonderful. Very much appreciate the effort.

As it turns out the implementation above what I shared works as expected. The problem is this code snippet in the DSL.

afterEvaluate {
    cppHeaders.includeEmptyDirs = false

    cppHeaders.include 'abc.inl'
    cppHeaders.include 'def.hpp'
}

Unfortunately, your implementation also fails with this in the DSL. Swapping cppHeaders.include with cppHeaders.from resolved the issue.

That, obviously, raises the question why doesn't cppHeaders.include work? Is there a convention on when to use include vs. from? An informative error message would have helped here.

amz-shahji commented 5 years ago

Also, in context to #922, is there a way to not use cppHeaders task name and still achieve the same result?

lacasseio commented 5 years ago

I suggest you read the documentation for include vs from. Basically, include is a pattern of files to include vs from is a set of source to use.

As for cppHeaders, there isn't any way aside from depending on the task name. We won't move toward exposing the task in the DSL but instead expose what needs to be exported.

amz-shahji commented 5 years ago

I misunderstood the include or was rather using it wrong. I saw filter method in AbstractCopyTask and so assumed include was to append to existing collection. Reading the referenced doc, include without a from doesn't make sense, I wasn't providing one and so those statements ended up filtering out all files from the collection. All makes sense now.

Thank you!