ningsh7 commented on code in PR #11180:
URL: https://github.com/apache/gravitino/pull/11180#discussion_r3314845735
##########
build.gradle.kts:
##########
@@ -778,6 +780,11 @@ jacoco {
tasks {
val projectDir = layout.projectDirectory
val outputDir = projectDir.dir("distribution")
+ val cleanDistributionPackage by registering(Delete::class) {
+ group = "gravitino distribution"
+ delete(distributionPackageDir, distributionPackageAllDir)
Review Comment:
> ```java
> val compileDistribution by registering {
> // Add clean task
> dependsOn("clean")
> val dependencies = ...
> ```
>
> I mean, we can only add a single line above to solve the problem, can you
try to verify it?
@yuqi1129 Sure! I have verified your suggestion. Adding only
`dependsOn("clean")` to `compileDistribution` looks fine in dry-run:
- `./gradlew compileDistribution -PskipWeb=true -x test --dry-run`
- `:cleanDistribution` and `:clean` were listed before the copy/jar tasks
- No dependency cycle was reported
However, the actual execution order was different:
- `./gradlew compileDistribution -PskipWeb=true -x test`
- Some copy tasks were executed before `clean`, for example:
- `:copySubprojectDependencies`
- `:copyCatalogLibAndConfigs`
- `:lance:lance-rest-server:copyLibs`
- Then Gradle executed:
- `:cleanDistribution`
- `:clean`
This deleted files that had already been copied into `distribution/package`.
The build still ended with `BUILD SUCCESSFUL`, but the generated distribution
package was incomplete.
I checked the Lance REST server jars afterwards:
- `find distribution/package distribution/package-all -name
'lance-core-2.0.1.jar' -print`
- Result: no output
- `find distribution/package -maxdepth 3 -type d -path
'*/lance-rest-server/libs' -print`
- Result: no output
So adding only `dependsOn("clean")` does not solve the issue. It has a
similar ordering risk to deleting files in `compileDistribution.doFirst`: the
clean task may run after some dependent
copy tasks and remove files that were already copied.
I restored the current implementation and verified it again:
- `./gradlew compileDistribution -PskipWeb=true -x test`
- `find distribution/package distribution/package-all -name
'lance-core-2.0.1.jar' -print`
The result included:
- `distribution/package/libs/lance-core-2.0.1.jar`
- `distribution/package/lance-rest-server/libs/lance-core-2.0.1.jar`
- `distribution/package-all/libs/lance-core-2.0.1.jar`
- `distribution/package-all/lance-rest-server/libs/lance-core-2.0.1.jar`
Therefore, I do not think the single-line `dependsOn("clean")` approach is
safe here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
