Give good error if an optional parameter is not set

[jjohannes]

An optional build parameter is of typ Provider<> and returns a 'standard' Gradle provider implementation. If Gradle calls .get() on it when the value is set, you always get the following (very useless) error:

> Cannot query the value of this provider because it has no value available.

In the case if the Build Parameters plugin, we have all the information which property is not set (property name and env variable name). So we should be able to construct providers that automatically give a better error.

Example

Would be nice to write a task like this and automatically get a good error if CATALINA_HOME is not set:

tasks.register<Copy>("deployWebApp") {
    from(tasks.war) { into("webapps") }
    into(buildParameters.catalina.home)
}

Right now I have to add a custom error through .orElse(provider { ... which is a bit ugly and verbose.

tasks.register<Copy>("deployWebApp") {
    from(tasks.war) { into("webapps") }
    into(buildParameters.catalina.home.orElse(provider {
        throw RuntimeException("CATALINA_HOME environment variable not set")
    }))
}

[jjohannes]

As far as I can see, there is no way to implement this with Gradle Providers. My approach was to use orElse and then throw an exception. Which I tried out in this PR: #30

However, the errors we see in the tests on the PR show that this approach will always throw the error if you use orElse or present later.

[jjohannes]

There is also this perspective to look at this:

We do not only have two kinds of parameters, but three:

1. Parameters with defaults
2. Parameters that are truly optional in the sense that 'is not present' is a valid value that is handled when the paramter is accessed
3. Parameters that are not optional ('is not present' is not a valid value) but also do not have a default. That is, such a parameters always needs to be set, when a part of the build is used that needs it.

Right now, we support 1 and 2, but 3 is not explicitly supported. I think that 3 might actually be more common than 2. E.g. the example used in our docs – credentials.password – is of this kind.
If we would explicitly support 3 – e.g. by adding a configuration like failIfNotSet = true – we could change the code generation for this kind. It would then return the actual type (not a Provider) like it is the case for 1. But instead of having a default value as convention, it would have throwing the error that I prepared in #30 as convention.

[britter]

Team decision

We realized that we can implement this by adding a throws clause to the getter implementation of build parameters. This is controlled by an additional boolean configuration property on BuildParameter called mandatory. If it's set to true parameters without default value will have a getter generated that returns the actual type and not a Provider of the type. If the value is not set when the getter is called, an exception is thrown. For parameters with default value the behavior doesn't change no matter the configuration of mandatory.

[britter]

Unfortunately after thinking about it some more, I realized what we discussed doesn't work. Let's say I have a build parameter that models the password to a maven repository for publishing:

buildParameters {
    string("repositoryPassword") {
        mandatory.set(true)
    }
}

I want this to be present when it's needed, e.g. when the build publishes some artifacts on CI, but I don't want developers having to set this every time, because developers simply don't have the credentials to deploy to the production repository. Now when I configure the maven publication it will look like this:

publications {
    repositories {
        maven {
            password = buildParameters.repositoryPassword
        }
    }
}

This code will fail at configuration time no matter whether a publication task was requested or not. First I thought this is a problem with maven-publish not using lazy API but even if it would I would have to do something like this in order to prevent errors at configuration times:

publications {
    repositories {
        maven {
            password.set(provider { buildParameters.repositoryPassword })
        }
    }
}

So I would always have to wrap this value into a provider because otherwise the code of the generated getter would be executed at configuration time.

I think we can solve this be creating a new Provider instance using ObjectFactory inside the getter and then throwing the exception from a map but I'm not sure if that really works. I think we need to revisit this.

[jjohannes]

Yes you are right. The second point is important. You may not want to access the parameter at configuration time to not get an error for build runs where you do not need that parameter. For me this means, that the type of a mandatory parameter (e.g. of type string) would still be Provider<String> instead of String. (This actually makes it easier to implement on top of what we already have I think.)

Then, if you only need the value for a certain task, it would only be accessed during task execution, or when the inputs of a task are calculated if the parameter provider is (part of) a task input.

Unfortunately, the repository password is a bad example then, because it's not sufficiently lazy in Gradle itself - although it should be! This is a problem in Gradle itself. I can open an issue for that (this looks related gradle/gradle#20925). I think there are workarounds to do this lazily. I will see if I can get it working and then we can document it as this is a quite common thing you may want to do.

For the documentation of mandatory parameters we should look for a clean example though.