[SPARK-56338][INFRA][FOLLOWUP] Support MAVEN_MIRROR_URL in SBT launcher bootstrap#55238
[SPARK-56338][INFRA][FOLLOWUP] Support MAVEN_MIRROR_URL in SBT launcher bootstrap#55238cloud-fan wants to merge 2 commits intoapache:masterfrom
Conversation
This is a follow-up to SPARK-56338 which added `MAVEN_MIRROR_URL` support for Maven and SBT project builds. The SBT launcher script (`build/sbt-launch-lib.bash`) was not covered — both the launcher JAR download and the SBT boot phase (resolving SBT itself and Scala) still used hardcoded default repositories. This patch makes the SBT launcher script respect `MAVEN_MIRROR_URL`: - The launcher JAR download falls back to `MAVEN_MIRROR_URL` when `DEFAULT_ARTIFACT_REPOSITORY` is not set. - When `MAVEN_MIRROR_URL` is set, a temporary SBT repositories config is generated and passed via `-Dsbt.repository.config` so the boot phase also uses the mirror. Co-authored-by: Isaac
cloud-fan
changed the title
| local | ||
| maven-mirror: ${MAVEN_MIRROR_URL} | ||
| EOF | ||
| addJava "-Dsbt.repository.config=$sbt_repo_config" |
There was a problem hiding this comment.
If -Dsbt.repository.config is already set by the users, what would happen? We still want that to work right? Also maybe we should cleanup the temp dir after the script? Even if it's in /tmp
There was a problem hiding this comment.
Good points. I'll add a guard to skip if -Dsbt.repository.config is already in SBT_OPTS, and add a trap to clean up the temp file.
| # If MAVEN_MIRROR_URL is set, generate a repositories config so the SBT launcher | ||
| # resolves SBT and Scala through the mirror during the boot phase. | ||
| if [[ -n "$MAVEN_MIRROR_URL" ]]; then | ||
| local sbt_repo_config="$(mktemp /tmp/sbt-repositories.XXXXXX)" |
There was a problem hiding this comment.
it's a little bit hacky, users may want to configure more than one repo sometimes
in Apache Celeborn, we allow users to have a build/sbt-config/repositories-local (ignored by Git), and provide several common templates
https://github.com/apache/celeborn/blob/main/build/sbt-config/
There was a problem hiding this comment.
The SBT bootstrap phase only downloads SBT itself and Scala — a single mirror should be sufficient for that. This is just to make MAVEN_MIRROR_URL work end-to-end consistently with the existing support in pom.xml and SparkBuild.scala.
For more complex multi-repo setups, users can still configure ~/.sbt/repositories or -Dsbt.repository.config directly.
…, clean up temp file Co-authored-by: Isaac
|
the docker test failure is unrelated, thanks for review, merging to master! |
5 participants
What changes were proposed in this pull request?
This is a follow-up to #55168. The SBT launcher script (
build/sbt-launch-lib.bash) did not respectMAVEN_MIRROR_URL. This patch adds support for it in two places:MAVEN_MIRROR_URLis used as the default forDEFAULT_ARTIFACT_REPOSITORYwhen neither is explicitly set.MAVEN_MIRROR_URLis set, a temporary SBT repositories config is generated and passed via-Dsbt.repository.configso SBT resolves itself and Scala through the mirror.Why are the changes needed?
SPARK-56338 added
MAVEN_MIRROR_URLsupport for Maven (pom.xml) and SBT project builds (SparkBuild.scala), but the SBT launcher script was not covered. In environments where default Maven repositories are unreachable, the SBT launcher JAR download and boot phase still fail without manual configuration (e.g.~/.sbt/repositories).Does this PR introduce any user-facing change?
No.
How was this patch tested?
Manually tested by setting
MAVEN_MIRROR_URLand building Spark with SBT from scratch (launcher JAR removed, no~/.sbt/repositories).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code