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

Make built-in Gradle tasks and java.lang.Object accepting methods more Provider<> friendly #4224

Closed
mkobit opened this issue Jan 31, 2018 · 7 comments
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container stale

Comments

@mkobit
Copy link
Contributor

mkobit commented Jan 31, 2018

Gradle recently introduced the Provider<> types to make configuration more lazy and functional. These types are great to use in custom plugins and when composing functional pieces together. Passing around functions that produce values is a much easier way to reason about the dependency graph when you only care about inputs and outputs and not as much about the underlying dependencies. You only have to find the value you want, possibly transform it (executed lazily), and then Gradle will take care of the ordering and precedence to produce that value.

Wiring tasks together that have inputs or outputs of Provider<>/Property is mostly straightfoward. Taking outputs from one task that are not a Provider<> type and wrapping them to be a Provider<> is also easy to do (although input provenance and inferred task dependencies might not be present). However, trying to use tasks that have outputs of Provider<> and using them in tasks that do not accept them is painful and removes the benefits of lazy/calculated values.

Expected Behavior

Gradle task types should be more aligned with the Provider<> type and functional/lazy model.

Current Behavior

You have to call .get() to greedily evaluate during configuration time or extract the container value in a doFirst block. Getting correct up-to-date checks in this manner is tricky, as build script authors may have to register dynamic task inputs in hopes of getting correct up-to-date checks. Some tasks inputs will work as under the hood they convert them with project.file, but not all tasks accept or produce file-like inputs and outputs. (Sort of on-topic, I'd like to see non-file type outputs be treated as close to the same as file-type outputs). Some tasks like Exec that have methods like args(Object object...) seem to work in some cases but not others. They also don't provide strong typing or clues for users who write build scripts in the kotlin-dsl if they will work with Provider types or not.

Context

This happens anytime you want to wire a Provider<> type into a task that does not take in a Provider<>.

Here are a few examples I've ran into:

  • updating the Manifest of a Jar task with an output derived from another task or calculated value (say for example, a Git commit hash)

  • Test task system properties are generated by another task

  • Test task environment variables are generated by another task

  • Wrapper input URL and other properties

  • Provider of some object in for commandLine (and I'm guessing other Object accepting methods that coerce to String)

    tasks.create('myExec', Exec) {
      commandLine(provider { 'ls' })
    }
    // ...
    > A problem occurred starting process 'command 'value: ls'' 
  • Set the description on any task based on a Provider<> value

  • Add values/links/tags to Build Scan plugin

Steps to Reproduce (for bugs)

N/A

Your Environment

Gradle 4.5

@ldaley
Copy link
Member

ldaley commented Jan 31, 2018

@mkobit great issue report. 👏

Lot's of explanation of the problem in practical terms and not just a proposed solution implementation.

@mkobit
Copy link
Contributor Author

mkobit commented Sep 22, 2018

Maybe breaking this down into separate issues would help it be incrementally fixed or improved? I listed a couple tasks and areas above that had issues, but it is not exhaustive. This might make it easier to pick a few areas where I can get some guidance and contribute a fix in that area.

For example, the Exec task and project.exec(Action<? super ExecSpec> action) are areas I have run into most frequently.

ExecSpec has several different APIs that could possibly be adapted to be used with the Provider type.

  • setCommandLine(Object... args)
  • commandLine(Object... args)
  • executable(Object executable)
  • args(Object... args)
  • setArgs(List<String> args)
  • executable(Object executable)
  • setExecutable(Object executable) (setExecutable(String executable) is a String overload)
  • setWorkingDir(Object dir) (setWorkingDir(File dir) overload)
  • workingDir(Object dir)
  • setErrorOutput(OutputStream outputStream)
  • setStandardInput(InputStream inputStream)
  • setStandardOutput(OutputStream outputStream)

Lots of these take in Object and convert underneath, so they could potentially be changed to lazily evaluate Provider<> types. Calling getArgs() (or other related getters could realize the underlying Provider<>, which is something to consider. All of the getters here return "immediate" values like List<String>, File, etc., and changing those is a breaking API change, so I would think any PR that changed those types would (rightfully) reject that kind of change so I think this needs to be another area for consideration.

There is, however, another thing that makes moving this forward ambiguous. The List<CommandLineArgumentProvider> getArgumentProviders() seems to imply it is for the lazy APIs, but I don't see any additive configuration methods like argumentProvider(CommandLineArgumentProvider provider) or overwriting configuration like setArgumentProvider(List<? extends CommandLineArgumentProvider> providers). There isn't really any documentation on it at all, so I don't even know how I would use it. I'm not sure why it doesn't use the existing ListProperty type rather than a specialized type. It doesn't seem desirable to create an additional type (e.g. like ExecutableProvider or WorkingDirProvider) and an overload for every one of those new types on each and every Gradle task, but I'm not the one to make that decision. I think discussion around them would be useful, though.

I'm sure there is similar discussion that could be had in other areas like JavaExec, CopySpec, Jar, so I think separating this into a bunch of issues might be a way to start progress.

@tbroyer
Copy link
Contributor

tbroyer commented Oct 13, 2018

Wrt List<CommandLineArgumentProvider>, the returned list is mutable, so you would add() your own CommandLineArgumentProvider implementation to the list (see https://github.com/tbroyer/gradle-errorprone-plugin/blob/v0.6/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt#L74-L76 for an example, albeit on a JavaCompile task).
See also #4685 for a discussion about replacing it with ListProvider.

@mkobit mkobit changed the title Make built-in Gradle tasks more Provider<> friendly Make built-in Gradle tasks and java.lang.Object accepting methods more Provider<> friendly Feb 5, 2019
@bmuskalla bmuskalla added this to the Unscheduled milestone Jun 4, 2020
@fcurts
Copy link

fcurts commented Aug 25, 2020

What's the plan for fixing such a fundamental issue? This is causing tremendous pain.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

@stale stale bot added the stale label Apr 16, 2022
@stale
Copy link

stale bot commented Jun 19, 2022

This issue has been automatically closed due to inactivity. If you can reproduce this on a recent version of Gradle or if you have a good use case for this feature, please feel free to to let know so we can reopen the issue. Please try to provide steps to reproduce, a quick explanation of your use case or a high-quality pull request.

@stale stale bot closed this as completed Jun 19, 2022
@wolfs wolfs closed this as not planned Won't fix, can't repro, duplicate, stale Sep 16, 2022
@mkobit
Copy link
Contributor Author

mkobit commented Dec 26, 2022

I know this is closed as unplanned, but in case others arrive here it looks like this is eventually planned to be addressed with https://github.com/orgs/gradle/projects/31/views/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container stale
Projects
None yet
Development

No branches or pull requests

9 participants