BuildKonfig icon indicating copy to clipboard operation
BuildKonfig copied to clipboard

Support config inheritance

Open yshrsmz opened this issue 3 years ago • 9 comments

#61

copied from https://github.com/yshrsmz/BuildKonfig/pull/61#issuecomment-947685297, by @TobiasPr


Why did you decide that the hierarchical higher source set always has precedence? I think this is counter intuitive. I think about as inheritance in which lower hierarchical source set inherits the props from the higher source set, but it should still be able to set a more specific value right now we have a sourceset hierarchy like this:

commonMain/
├─ jvmMain/
│├─ androidMain/
├─ iosMain/

for androidMain the buildkonfig is not generated - only for the jvmMain

yshrsmz avatar Oct 21 '21 12:10 yshrsmz

copied from https://github.com/yshrsmz/BuildKonfig/pull/61#issuecomment-947790589


That's a good question.

That's because you can only have one actual declaration in sourceset hierarchy.

If you want targetConfig in androidMain, then you can't have it in jvmMain. 2 actual declarations in a hierarchy simply throws a compile error.

Which we should prefer is something I couldn't find a good answer.

What is your desired behavior? I think I can change it, but then what jvmMain can see is fields defined in defaultConfigs.

yshrsmz avatar Oct 21 '21 12:10 yshrsmz

copied from https://github.com/yshrsmz/BuildKonfig/pull/61#issuecomment-948557106, by @TobiasPr


Thanks for replying so quickly :)

In our case we build our application for jvmMain, androidMain (which is currently 100% identical source sets wise but we would like to set some differnent value on buildkonfig) and iosMain. Here is a part of our build.gradle.kts:

sourceSets {
        val commonMain by getting {
            dependencies {
                implementation("common.dependency:1.2.3")
            }
        }

        val jvmMain by getting {
            dependsOn(commonMain)
            dependencies {
                api("some.dependency:1.2.3")
                ...
            }
        }

        val androidMain by getting {
            dependsOn(jvmMain)
        }
...
}

val versionName = "1.2.3"
buildkonfig {
    packageName = "com.mypackage"

    // default config is required
    defaultConfigs {
        buildConfigField(STRING, "semanticVersionName", versionName)
        buildConfigField(STRING, "osName", "common")
    }

    targetConfigs {

        create("jvm") {
            buildConfigField(STRING, "osName", "jvm")
        }

        create("android") {
            buildConfigField(STRING, "osName", "android")
        }

        create("ios") {
            buildConfigField(STRING, "osName", "ios")
        }
    }
}

if we build for android the generated buildkonfig is the same as for jvm and it would be cool to have them differently. I am not that experienced in KMM so I am not sure if it is just a use case of ours and whether we actually need to make the distinction between jvm and android. I was just thinking whether this inheritance like behavior could also solve the limitations mentioned in the ticket https://github.com/yshrsmz/BuildKonfig/issues/38 for Desktop systems, which seem a similar use case - but maybe I did not understand the technical limitations correctly

yshrsmz avatar Oct 21 '21 12:10 yshrsmz

Your jvmMain is an intermediate SourceSet, but also a "terminal" SourceSet. Actually, I didn't consider that usecase.

Assuming that your jvmMain is something like a library, the current suggestion is:

  • create a jvmCommonMain SourceSet. The SourceSet hierarchy should be like below.
  • create targetConfigs for jvmMain and androidMain
- commonMain
  - jvmCommonMain  // ← does not know anything about BuildKonfig(or knows only about default fields)
    - jvmMain           // ← knows about jvmMain specific fields in BuildKonfig
    - androidMain  // ← knows about androidMain specific fields in BuildKonfig
  • common implementation is in jvmCommonMain, and library-specific interface/implementation in jvmMain
    • common implementation doesn't know about BuildKonfig, just receive constants as arguments
  • pass BuildKonfig's value to common implementation from jvmMain/androidMain

yshrsmz avatar Oct 21 '21 13:10 yshrsmz

Whoops, timing.

Generally, BuildKonfig currently assumes that "intermediate" SourceSet is just "intermediate", as Kotlin does not allow having multiple actual declarations for one expect declaration in the SourceSet hierarchy.

yshrsmz avatar Oct 21 '21 13:10 yshrsmz

I tried that, but my build is failing. I think it is because we did not see that it is currently not supported/ we are not supopsed to share code between android and jvm according to the documentation:

Kotlin doesn’t currently support sharing a source set for these combinations:

  • Several JVM targets
  • JVM + Android targets
  • Several JS targets

Unfortunately I cannot really say something if creating a common source set works, because when I tried I ran into compilation errors which are probably caused by missing support

TobiasPr avatar Oct 21 '21 13:10 TobiasPr

Yeah, above suggestion is before-your-previous-comment, so I didn't know about the current KMP limitation you said.

So it's a general suggestion rather than your usecase specific, unfortunately :(

yshrsmz avatar Oct 21 '21 13:10 yshrsmz

Okay, thank you for taking the time :)

TobiasPr avatar Oct 21 '21 13:10 TobiasPr

@yshrsmz

I am having a sub-module(:shared:core) in shared module. I want to define buildConfigFields variables in core module and access them in my shared module like in multimodular Android project .

But here i was not able use generated references of core module in shared-module.

package com.example.core

import kotlin.Boolean
import kotlin.String

internal expect object BuildKonfig {
  public val BUILD_TYPE: String?

  public val FLAVOUR: String?

  public val DEBUG: Boolean

  public val BUILD_VARIENT: String?
}

The generated class is internal class. Can't this be a public class? So that it's visible to all dependent modules.

farhazulmullick-pw avatar Nov 22 '23 06:11 farhazulmullick-pw

@farhazulmullick-pw please create a separate issue. This issue is for config inheritance, not about config visibility.

yshrsmz avatar Nov 22 '23 06:11 yshrsmz