Skip to content

[SPARK-57393][BUILD][FOLLOWUP] Clean out old PySpark sdists#56996

Open
nchammas wants to merge 3 commits into
apache:masterfrom
nchammas:SPARK-57393-clean-old-sdists
Open

[SPARK-57393][BUILD][FOLLOWUP] Clean out old PySpark sdists#56996
nchammas wants to merge 3 commits into
apache:masterfrom
nchammas:SPARK-57393-clean-old-sdists

Conversation

@nchammas

@nchammas nchammas commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Clear stale PySpark sdist tarballs from python/dist/ before building new ones in dev/make-distribution.sh.

Why are the changes needed?

#56453 added a post-build validation that checks every dist/pyspark*.tar.gz for top-level LICENSE and NOTICE files. However, if python/dist/ already contains tarballs from a previous build, the glob picks them up and the validation fails with:

ERROR: dist/pyspark-4.0.0.dev0.tar.gz is missing LICENSE at the package root

The script already cleans pyspark.egg-info to avoid stale caches. This change also cleans out old sdists.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

I reproduced the failure locally with a stale pyspark-4.0.0.dev0.tar.gz in python/dist/, applied the fix, and confirmed make-distribution.sh --pip completes successfully.

Was this patch authored or co-authored using generative AI tooling?

Co-authored with GitHub Copilot.

@nchammas

nchammas commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

cc @huaxingao since you authored the linked PR.

Btw I didn't include the phrase Generated-by: because it seems too strong of a wording to use when an LLM was involved but didn't exactly generate the whole patch.

Comment thread dev/make-distribution.sh Outdated
rm -rf pyspark.egg-info || echo "No existing egg info file, skipping deletion"
# Remove stale sdists so the validation glob below does not trip on tarballs
# left over from earlier builds with a different version.
rm -rf dist/pyspark*.tar.gz

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just do -f?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @nchammas, I left just one minor comment.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-57393] Build: Clean out old PySpark sdists [SPARK-57393][BUILD] Clean out old PySpark sdists Jul 10, 2026
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-57393][BUILD] Clean out old PySpark sdists [SPARK-57393][BUILD][FOLLOWUP] Clean out old PySpark sdists Jul 10, 2026

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

BTW, @nchammas , it changed the PR title for you according to the Apache Spark community convention. It would be greatly helpful if you follow the convention.

- [SPARK-57393] Build: Clean out old PySpark sdists
+ [SPARK-57393][BUILD][FOLLOWUP] Clean out old PySpark sdists

@nchammas

Copy link
Copy Markdown
Contributor Author

I wrote "Build:" to mirror #56453, but yes thank you for the correction. This is indeed a follow-up PR. I will update the contributing guide to note the convention regarding follow-up PRs. It is currently not documented, even though it is an old convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants