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

Unable to use @Option with ListProperty/SetProperty #10517

Closed
szpak opened this issue Sep 9, 2019 · 9 comments · Fixed by #25554
Closed

Unable to use @Option with ListProperty/SetProperty #10517

szpak opened this issue Sep 9, 2019 · 9 comments · Fixed by #25554
Assignees
Labels
a:bug in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty p:lazy-migration Issues covered by migration to an all-lazy API
Milestone

Comments

@szpak
Copy link
Contributor

szpak commented Sep 9, 2019

Expected Behavior

@Option should be possible to use with final ListProperty/SetProperty<> fields (as it was implemented by @adammurdoch for Property<> in #7977).

Current Behavior

@Input
@Optional
@Option(option = "pitest-features", description = "Configures PIT features to be used")
final ListProperty<String> features //it's Groovy, getter is automatically generated

gives:

OptionValidationException: No setter for Option annotated field 'features' in class 'class info.solidsoft.gradle.pitest.PitestTask'.

Context

After migration from List<> and conventionMapping to PropertyList<> (or PropertySet<>) it is no longer possible to use @Option to set/override values.

Steps to Reproduce

Try to set value with @Option for final ListProperty<>/final SetProperty<> fileds.

Your Environment

Gradle 5.6.1.

@big-guy
Copy link
Member

big-guy commented Sep 9, 2019

This looks like a good idea to me. Would you be willing to submit a PR?

@Vampire
Copy link
Contributor

Vampire commented Sep 10, 2019

Maybe it would also make sense if you could somehow specify from the commandline, whether you want to replace any value set in the build script or add to the set or list of values.

Especially for my use-case from which this issues is grown of, I really want to add to the list configured in the build script and not replace the values.

@szpak
Copy link
Contributor Author

szpak commented Sep 10, 2019

--pitest.features+=YOUR_ADDED_FEATURE? I don't know if the @Option mechanism support that.

Update. --pitest.features instead of pitest --features as I hope to have that syntax supported one day (without the need po put it right after the task).

@Vampire
Copy link
Contributor

Vampire commented Sep 10, 2019

I guess it does not yet, hence the comment here to maybe add it while adding support for ListPropery and SetProperty. :-)

Btw. I agree with @big-guy, that a "pitest" prefix for the options should not be used.
You already have to specify this option after the task the option is for, that should be enough association, no matter if a dot, dash or underscore is used to separate the prefix from the actual option name. This just adds the burden to type these characters on the command-line which is not really helpful imho. :-)

@big-guy
Copy link
Member

big-guy commented Sep 10, 2019

I think the first step would be to make this work the same way as a regular List and then add something that would append separately. It's less confusing that they just work the same for now.

@Vampire could you open an issue (if you haven't already) for allowing list options to append? Some examples for where you'd like to use it would be helpful.

@stale
Copy link

stale bot commented Oct 15, 2020

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 Oct 15, 2020
@szpak
Copy link
Contributor Author

szpak commented Oct 15, 2020

It is still relevant.

@stale stale bot removed the stale label Oct 15, 2020
@bmuskalla bmuskalla added in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty and removed in:configuration-model lazy api, domain object container labels Apr 9, 2021
@stale
Copy link

stale bot commented Apr 11, 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 11, 2022
@big-guy big-guy removed the stale label Apr 12, 2022
tricktron added a commit to tricktron/frege-gradle-plugin that referenced this issue Jul 8, 2022
BREAKING CHANGE: remove the `--compileItem` command line option.

I switched from `Property<String>` -> `ListProperty<String>` to support
multiple compile items. Unfortunately but understandably, Gradle
does not support (multiple) command line options with the `ListProperty`
(type)[gradle/gradle#10517].

In the end, the core input format of Gradle is not command
line arguments but a `build.gradle`.
tricktron added a commit to tricktron/frege-gradle-plugin that referenced this issue Jul 8, 2022
BREAKING CHANGE: remove the `--compileItem` command line option.

I switched from `Property<String>` -> `ListProperty<String>` to support
multiple compile items. Unfortunately but understandably, Gradle
does not support (multiple) command line options with the `ListProperty`
[type](gradle/gradle#10517).

In the end, the core input format of Gradle is not command
line arguments but a `build.gradle`.
tricktron added a commit to tricktron/frege-gradle-plugin that referenced this issue Jul 8, 2022
BREAKING CHANGE: remove the `--compileItem` command line option.

I switched from `Property<String>` -> `ListProperty<String>` to support
multiple compile items. Unfortunately but understandably, Gradle
does not support (multiple) command line options with the `ListProperty`
[type](gradle/gradle#10517).

In the end, the core input format of Gradle is not command
line arguments but a `build.gradle`.
tricktron added a commit to tricktron/frege-gradle-plugin that referenced this issue Jul 8, 2022
* feat!: support multiple compile items in compileFrege task

BREAKING CHANGE: remove the `--compileItem` command line option.

I switched from `Property<String>` -> `ListProperty<String>` to support
multiple compile items. Unfortunately but understandably, Gradle
does not support (multiple) command line options with the `ListProperty`
[type](gradle/gradle#10517).

In the end, the core input format of Gradle is not command
line arguments but a `build.gradle`.

* chore: update readme

* refactor: extract common logic to `filterFilesBySuffix` function

* chore: add *repl should only compile the specified repl module and its
dependencies* test

Before, the repl task compiled all frege files and failed if you had
errors in other frege files.
@jbartok jbartok added the p:lazy-migration Issues covered by migration to an all-lazy API label May 25, 2023
@lptr
Copy link
Member

lptr commented May 26, 2023

Looks like the behavior changed somewhat, but it still doesn't work as expected. From duplicate issue #25097:

Expected Behavior

It is possible to annotation a JavaBean property of type List<String> with @Option and supply it with values from the command-line. We should have feature parity for properties of type ListProperty<String> etc.

Current Behavior (optional)

Currently an error is thrown if @Option is used on ListProperty<String>:

Caused by: org.gradle.cli.CommandLineArgumentException: Command-line option '--serial' does not take an argument.
	at app//org.gradle.cli.CommandLineParser$KnownOptionParserState.onArgument(CommandLineParser.java:423)
	at app//org.gradle.cli.CommandLineParser.parse(CommandLineParser.java:93)
	at app//org.gradle.execution.commandline.CommandLineTaskConfigurer.configureTasksNow(CommandLineTaskConfigurer.java:67)
	... 123 more

Context

When migrating eager multi-valued properties from List<T> to lazy ListProperty<T> types we need to keep @Option working for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty p:lazy-migration Issues covered by migration to an all-lazy API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants