build: remove eachdist.py#5381
Draft
ocelotl wants to merge 24 commits into
Draft
Conversation
eachdist.py duplicated functionality tox already provides (install/test/lint/format per package), which is the core complaint in open-telemetry#1462. Its remaining real responsibilities are split into two small, single-purpose scripts instead of one multi-command tool: - scripts/repo_targets.py: stdlib-only distribution discovery, used by scripts/griffe_check.py for the public API breaking-change check. - scripts/release.py: version bumping used by the release workflows and .github/scripts/update-version*.sh, reading the renamed repo.ini (formerly eachdist.ini). Also removes the coverage tox env and scripts/coverage.sh: it isn't in tox's envlist, isn't run by any workflow, and already referenced instrumentation/* and exporter-datadog paths that no longer exist in this repo since the split to opentelemetry-python-contrib.
release.py repeated the exact pattern open-telemetry#1462 asked to remove, just at smaller scale: one script dispatching to 3 subcommands, each of which only ever has a single caller. Splits it into version.py, update_version.py, and update_patch_version.py, with the shared regex/file-update helpers factored into version_files.py so the two version-bump scripts don't duplicate that logic.
Matches the convention already used elsewhere in scripts/ (e.g. public_symbols_checker.py, add_required_checks.py): from x import y instead of import x, unless there's a good reason not to.
Both .github/scripts/update-version*.sh existed only to sed the new version into repo.ini before invoking the corresponding Python script, a holdover from eachdist.py never accepting a target version as a CLI argument. update_version.py and update_patch_version.py now take the new versions directly and patch repo.ini themselves via update_repo_ini_version() (an in-place text substitution equivalent to the old sed command, not a ConfigParser round-trip, so comments and formatting in repo.ini are preserved). The 3 call sites in prepare-release-branch.yml and prepare-patch-release.yml now invoke the Python scripts directly.
Gives packages/sortfirst native array support instead of ConfigParser's multi-line-string convention, removing the hand-rolled getlistcfg list parser, and matches how every other config in this repo is written (pyproject.toml, tox-uv.toml). version_files.py already depends on the toml package for reading pyproject.toml, so no new dependency. update_repo_toml_version() does a plain load/mutate/dump instead of the previous ini-specific regex substitution; this collapses repo.toml's comments and one-item-per-line array formatting on the next version bump, a known and accepted trade-off for simpler code.
Reads (repo_targets.py, version.py, version_files.py's pyproject.toml lookup) now use stdlib tomllib instead of the third-party toml package. tomllib has no writer, so update_repo_toml_version keeps writing via a surgical regex substitution on the version line instead of a full load/dump round-trip, which also preserves repo.toml's comments and array formatting. Removes the now-unneeded `pip install toml` steps from the release workflows.
…heck tomlkit reads and writes TOML while preserving comments and array formatting (verified via round-trip test), and supports Python >=3.9, which fits this repo's >=3.10 floor better than stdlib tomllib's 3.11+ requirement. Replaces the tomllib-for-reads / regex-for-write split with a single dependency that handles both, and drops the need for update_repo_toml_version's regex substitution. Also fixes a latent bug from the earlier eachdist.py removal commit: the public-symbols-check tox env's deps had the toml package dropped without replacement, even though scripts/griffe_check.py depends on it transitively through repo_targets.py.
version.py, update_version.py, update_patch_version.py, and version_files.py are only ever used for cutting a release. Group them under scripts/release/ to make that boundary explicit. repo_targets.py and repo.toml stay put: repo_targets.py is also imported by scripts/griffe_check.py for the public-symbols-check tox env (unrelated to releases), so repo.toml isn't exclusively a release file. The 3 moved entry points import repo_targets from the parent scripts/ directory, which requires an explicit sys.path insertion since Python only auto-adds a directly-run script's own directory to sys.path. Added matching E402 per-file-ignores in pyproject.toml, mirroring the existing exception for opentelemetry-sdk/tests/_configuration/test_models.py.
Replaces import sys with from sys import exit, path to stay consistent with this repo's from-import convention.
update_dependencies had exactly one call site (in update_version.py's loop) so it's inlined there directly. update_patch_dependencies has two call sites, both in update_patch_version.py, so it stays a function but moves out of version_files.py into the file that actually uses it. version_files.py keeps only what's genuinely shared between both entry points.
Neither wrapper was doing real work here: main() never returned a value (exit(main()) always exited 0 regardless), and nothing imports these files, so the __main__ guard had no import-safety to protect. parse_args() had exactly one call site. Flattened all three scripts to plain top-level code.
Both had exactly one call site (update_files and update_version_files respectively). filter_packages inlines to a simple any() filter condition; find inlines as a nested os.walk loop with the same first-match-wins behavior.
--mode accepted any string but every call site only ever passed
"stable" or "prerelease" ("DEFAULT", the default, was never actually
used). Replaced with two specific, mutually exclusive, required flags
matching the --stable_version/--unstable_version vocabulary already
used by update_version.py and update_patch_version.py.
Matches the verb-based naming every other executable script here uses (update_version.py, check_license_header.py, griffe_check.py, ...). "version.py" read like it might hold version constants or utilities; its only job is printing the current version to stdout.
print_version.py and update_version.py have no function definitions after the earlier main()/parse_args() removal, so there's nothing to annotate there. find_projectroot() is now typed to return Path instead of Path | None: none of its 4 call sites ever checked for None, so the Optional was never actually handled -- it now raises FileNotFoundError if no project root is found, which is both more honest about the actual invariant and gives a non-nullable return type everywhere it's used.
cfg (repo_targets.find_targets, print_version.py), section (print_version.py), search (version_files.update_version_files, a constant), and pyproject (version_files.update_version_files) were each referenced exactly once; inlined at their point of use. update_version.py's per-package search/replace regex variables were also single-use (unlike update_patch_version.py's, which are read twice -- once by a debug print, once by update_files -- so those stay named). Left cfg in update_version.py/update_patch_version.py alone: it's loaded once outside a loop and read inside the loop body, so inlining would reload/reparse repo.toml on every iteration.
One-line docstrings on every function in repo_targets.py, version_files.py, and update_patch_version.py, matching the style already used in scripts/check_for_valid_readme.py. print_version.py and update_version.py have no function definitions to document -- both are plain top-level scripts since main() was removed earlier.
"targets" didn't say what it held (a list of package directory Paths) and read confusingly close to the unrelated "packages" (a list of package name strings). Renamed throughout, including find_targets -> find_package_dirs and find_targets_unordered -> find_package_dirs_unordered in repo_targets.py, which forced a matching update in scripts/griffe_check.py, the only other caller. Replaced print() with logging in update_version.py, update_patch_version.py, and version_files.py: each entry point calls basicConfig(level=INFO, format="%(message)s") to keep today's plain CI output, and version_files.py (a shared, non-entry-point module) just grabs its own logger. The one debug-style print in update_patch_dependencies (dumping the generated regex/replacement per package) is now logger.debug(), so it's available on demand without cluttering normal runs. print_version.py is untouched: its single print is the script's actual output, captured via $(...) in the workflows, not a log message.
Explains what each of repo_targets.py, print_version.py, version_files.py, update_version.py, and update_patch_version.py does. update_version.py and update_patch_version.py's docstrings include concrete before/after snippets of repo.toml, a package's pyproject.toml, and its version/__init__.py, since those are the files each script actually rewrites and their .dev-suffix-vs-exact- previous-version distinction isn't obvious from the code alone.
--stable and --unstable were plain store_true flags, so getting the
repo.toml section name required a manual "stable" if args.stable else
"prerelease" translation after parsing. Switched both to
store_const with a shared dest="section", so args.section directly
holds the exact string ("stable"/"prerelease") each flag represents --
no ternary needed. Mutual exclusivity and required-ness (exactly one
of the two must be given) are unchanged, still enforced by the
mutually exclusive group.
Inlined two more single-use variables found on a fresh sweep: args in print_version.py (used exactly once, as args.section) and OPERATORS in version_files.py (used exactly once, to build OPERATORS_PATTERN). Renamed version_files.py to edit.py, and its three functions (update_version_files, update_files, update_repo_toml_version) to edit_version_files, edit_files, edit_repo_toml_version, updating the imports and call sites in update_version.py and update_patch_version.py.
Adds a header comment block to each of release.yml, prepare-release-branch.yml, and prepare-patch-release.yml explaining its overall purpose and how it fits into the 3-workflow release pipeline (prepare-release-branch -> release -> optionally prepare-patch-release -> release again), plus inline comments walking through what each step and non-trivial shell command does and why. Purely additive: every non-comment line is unchanged (verified by diffing each file's content with comments/blank lines stripped against its pre-change version).
package_directory_path(s), rootpath to root_path, inline unique() unique() had exactly one call site (find_package_dirs' return list( unique(package_dirs))); replaced with the standard order-preserving dict.fromkeys() idiom instead of reimplementing its seen-set loop inline. find_package_dirs is renamed to find_package_dirs_ordered, to read as the counterpart of the existing find_package_dirs_unordered rather than an unlabeled default. Renamed repo_targets.py to find.py to match what it actually does. package_dir/package_dirs renamed to package_directory_path/package_directory_paths, and rootpath to root_path, everywhere they appear (find.py, edit.py, print_version.py, update_version.py, update_patch_version.py, and the forced update to griffe_check.py, the only other importer of find.py).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes
scripts/eachdist.py. Its overlap with tox (per-package install/test/lint/format) is exactly the duplication called out in #1462; every one of those subcommands has zero remaining callers. Its two still-needed responsibilities move to small, single-purpose, documented, typed scripts instead of one multi-command tool:scripts/find.py— locates this repo's root and its package directories (find_projectroot,find_package_dirs_ordered,find_package_dirs_unordered). Shared by the release scripts below andscripts/griffe_check.py's public-API breaking-change check, so it stays at the top level ofscripts/, not underscripts/release/.scripts/release/print_version.py— printsrepo.toml's current--stable/--unstableversion to stdout, for the release workflows to capture.scripts/release/update_version.py— bumpsrepo.toml, every package'spyproject.tomldependency pins currently ending in.dev, and every package's__version__, to a new version. Used both to finalize a release branch's version and to advance main to the next dev version.scripts/release/update_patch_version.py— the patch-release equivalent: same three targets, but anchored on the exact previous version rather than a.devsuffix, since a release branch's versions are already finalized.scripts/release/edit.py— the file-editing helpers genuinely shared between the two scripts above (edit_version_files,edit_files,edit_repo_toml_version,OPERATORS_PATTERN).eachdist.iniis renamed torepo.toml(via an intermediaterepo.ini): native TOML arrays forpackages/sortfirstremove the hand-rolledgetlistcfglist parser, matching how every other config in this repo is written. It's read/written viatomlkit(round-trips without collapsing comments/array formatting, supports Python>=3.9matching this repo's>=3.10floor — stdlibtomllibwas rejected: needs 3.11+ and has no writer). It stays at the repo root for the same reasonscripts/find.pydoes: it's read by the public-symbols-check path too, not just releases.While auditing where
tomlkitneeds installing, found and fixed a latent bug: thepublic-symbols-checktox env'sdepshad losttoml(needed transitively viagriffe_check.py→scripts/find.py) in the commit that removedeachdist.py, with no replacement — confirmed this actually breaks (ModuleNotFoundError) without the fix.Removes the
.github/scripts/update-version.sh/update-version-patch.shwrappers (existed only to patch the version into the file before invoking a script that immediately re-read it — a holdover fromeachdist.pynever taking a target version as a CLI argument); the 3 release workflows now call the Python scripts directly.Removes the
coveragetox env andscripts/coverage.sh: not in tox'senvlist, not invoked anywhere, already referencing paths that don't exist in this repo since the split to opentelemetry-python-contrib.Also cleaned up along the way, applying this repo's own conventions consistently:
print_version.py's old--mode <string>accepted any string but every caller only ever passed"stable"/"prerelease"; replaced with mutually-exclusive, required--stable/--unstableflags usingaction="store_const"soargs.sectiondirectly holds the string needed, no manual translation.main()/parse_args()wrappers and several single-caller helpers (update_dependencies,find,filter_packages,unique), inlined at their call site (or, for helpers with 2+ call sites, moved next to their one real caller); inlined every genuinely single-use local variable found across multiple passes (args,cfg,OPERATORS,search/replace, ...).repo.toml/pyproject.toml/__init__.pyinupdate_version.py/update_patch_version.py, since the.dev-suffix-vs-exact-previous-version distinction between them isn't obvious from the code alone).print()withloggingin the two entry points andedit.py;print_version.py's singleprintis untouched since it's the script's actual output, captured via$(...)in the workflows.targets/package_dir(s)(a list ofPaths, easily confused with the unrelatedpackageslist of name strings) →package_directory_path(s);rootpath→root_path;find_targets/find_targets_unordered→find_package_dirs_ordered/find_package_dirs_unordered;repo_targets.py→find.py;version_files.py→edit.pywith itsupdate_*functions renamed toedit_*.Finally, added detailed header + inline documentation to all 3 release workflows (
release.yml,prepare-release-branch.yml,prepare-patch-release.yml), explaining each workflow's overall purpose, how the 3 fit together as a pipeline, and what each non-trivial step/shell command does. Purely additive — every functional line is unchanged.Closes #1462.
Test plan
./scripts/release/print_version.py --stable/--unstableprint the same values asrepo.toml; confirmed the flags are mutually exclusive and one is required (argparse errors, exit code 2, in both violation cases).update_version.pyandupdate_patch_version.pyend-to-end (as subprocesses) against scratch fixture repos at every stage of this branch's refactors, confirming dependency pins,__version__files, andrepo.toml's version fields update identically to the originaleachdist.pylogic throughout.tomlkit-basededit_repo_toml_versionwrite preservesrepo.toml's comments and array formatting exactly (only the targetedversion = "..."lines change).find.find_package_dirs_orderedreturns the same sorted, de-duplicated distribution list (26 targets, same order) aseachdist.find_targets("DEFAULT", ...)used to.public-symbols-checktox env with only its declareddepsinstalled and confirmedgriffe_check.pyruns cleanly; confirmed it fails withouttomlkit, proving the dropped-dependency bug was real.basedpyright) across every file inscripts//scripts/release/touched by this PR, plusgriffe_check.py: 0 errors, 0 warnings, at every stage of the refactor.ruff check/ruff format --checkclean on every edited Python file throughout.grep -rn eachdist,grep -rn "release\.py",grep -rn "repo\.ini", andgrep -rn "repo_targets\|version_files\|find_targets\b\|rootpath\|package_dir\b\|package_dirs\b\|--mode"(old paths/names) across the tracked tree all return no results.