viirya commented on PR #6888:
URL:
https://github.com/apache/datafusion-comet/pull/6888#issuecomment-6131639588
Thanks for tightening this up. Dropping the unusable parent pom and
`comet-common` from the published set makes sense, and I confirmed locally that
the installed `comet-spark` pom (the shade dependency-reduced pom) has neither
a `<parent>` nor a `comet-common` dependency.
One thing in `dev/release/publish-to-maven.sh`: the script has no `set -e`
and doesn't check `pushd $LOCAL_REPO/org/apache/datafusion`. If `-r` is
mistyped, `pushd` fails and the new `find . -mindepth 1 -maxdepth 1 ! -name
'comet-spark-spark*' -exec rm -rf {} +` runs in the release manager's checkout,
removing `.git` and every directory. The older `find ... | xargs rm` line had
the same exposure, but only for non-jar/pom files. Could we change it to `pushd
"$LOCAL_REPO/org/apache/datafusion" || exit 1`?
Also, nothing guards the two invariants this relies on. The published pom
has no `<parent>` only because flatten feeds into shade's dependency-reduced
pom, and the snapshot run deploys only `comet-spark` only as long as nobody
re-adds the `-Dmaven.deploy.skip=false` override. The `dry_run` path runs
`install`, which ignores `maven.deploy.skip`, so it can't catch either one.
What do you think about having `dry_run` run `deploy
-DaltDeploymentRepository=dryrun::file://$RUNNER_TEMP/deploy-repo` instead,
then asserting that the repo holds only `comet-spark-spark*` directories and
that the pom has no `<parent>`? That keeps the dry run fork-safe and turns the
"check the next nightly log" step into a check that runs every time.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
