Is the change of System.getProperty for project.property really necessary as part of this?

Hey @dblock
So the right fit for our use case is to use project property, we identify the opensearch.version property, infer the plugin version based on this opensearch.version, and next time when we run ./gradlew tasks we dont have the right opensearch.version based on which the plugin version was inferred (because with existing setup, using -D we only have it during runtime). With -P passing project property, we have opensearch.version always part of the project and next time when we version increment we update this opensearch.version property (which is part of gradle.properties file.).
Note: Also project properties can be overridden with -P ./gradlew assemble --no-daemon --refresh-dependencies -Popensearch.version=$VERSION

This change is necessary (the main change that drives the version increment automation), the idea from the solution is to make sure we maintain a gradle.properties file, this file has all the project properties which can be used for now and for future automation properties as well.

More details

A gradle project property (-P)is a property that can be accessed in project’s build.gradle, in our case we use opensearch.version, build.snapshot, build.version_qualifier just to infer the plugin version which is part of build.gradle,

System properties(-D) are accessed via normal Java/Groovy system properties access methods (works only for runtime injection).
@bbarani @peterzhuamazon

Hey @dblock I understand your concern, since this -D (system property) is added at multiple places, this PR pushes multiple changes at same time, one to add gradle.properties file, two to add two different tasks, three to make change from system to project property.

Ok, I think I understand. The real reason is that ant.propertyfile is easier to use than some way of replacing a version inside build.gradle. I think you should push the property change separately, either before, or after the version increment.

If you do it before it might be easier/smaller amount of changes, especially if you need to modify files that aren't just .properties.

task setVersion {
    onlyIf { System.getProperty('newVersion') }
    // replace in build.gradle with whatever file search/replace
    doLast {
        println "Setting version to ${System.getProperty('newVersion')}."
    }
}

Ok, I think I understand. The real reason is that ant.propertyfile is easier to use than some way of replacing a version inside build.gradle. I think you should push the property change separately, either before, or after the version increment.

Yes @dblock without having property change we can still proceed to version increment, We solely rely on opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT") the default value (here its 3.0.0-SNAPSHOT) and replace with version that is out there in OpenSearch core repo
The downside with this is we will not have a fixed project property opensearch.version where we can compare the version against with.
Example version increment PR I have tested https://github.com/prudhvigodithi/sql/pull/8/files
Workflow run: https://github.com/prudhvigodithi/opensearch-build/runs/7224621950?check_suite_focus=true
Task: https://github.com/prudhvigodithi/sql/blob/main/build.gradle#L152-L172

Also @dblock adding to your point to use onlyIf, we can only skip the execution phase of the task, not its configuration phase, meaning for example when we run ./gradlew properties the task fails with output property newVersion not found, we can use onlyIf condition, but add a fail-safe way to access the project properties, whereas using if is more clean more only runs a task when has -PnewVersion
Could not get unknown property 'newVersion' for task

Hey @dblock Pushed the change in my latest commit, can you please check the task now considering the above inputs.
1st change will only be version increment, hence only added the task versionIncrement, this will use the existing

opensearch_version = System.getProperty("opensearch.version", "1.3.3-SNAPSHOT")

opensearch_version property (here from above line the default is 1.3.3-SNAPSHOT) compares with core and raises a PR.
Thank you
Sample common-utils [AUTO] PR: https://github.com/prudhvigodithi/common-utils/pull/2/files
@bbarani

Thanks @dblock for all the details, I see now why onlyIF is good use case here. I have pushed the change in my latest commit.

./gradlew versionIncrement
> Task :versionIncrement SKIPPED

./gradlew versionIncrement -DnewVersion=1.3.3-SNAPSHOT
> Task :versionIncrement
Setting version to 1.3.3-SNAPSHOT.

./gradlew build nor ./gradlew build -DnewVersion=1.3.4-SNAPSHOT
(does not run versionIncrement task)

I imagine you're renaming tasks following opensearch-project/opensearch-plugin-template-java#32?

Ye @dblock I will be shortly pushing the code renaming the task as updateVersion.

Thanks @dblock I have raised a new PR to main branch with backport labels, please advise if I can close this?
#200
Thank you

Yep, use backports.