Skip to content

chore: change deleted package flag updater to put one per line #3368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

chore: change deleted package flag updater to put one per line #3368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9119e3e
chore: make update deleted package script put one entry per line
rickeylev Oct 17, 2025
aa06873
cleanup balzerc
rickeylev Oct 17, 2025
2178184
fix sort order
rickeylev Oct 17, 2025
a56a5a0
udpate bazelrc, precommit
rickeylev Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 137 additions & 6 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,142 @@
# For bazel-in-bazel testing
# Trick bazel into treating BUILD files under examples/* as being regular files
# For bazel-in-bazel testing and ignoring sub-workspaces for ... patterns.
#
# Trick Bazel into treating BUILD files under examples/* as being regular files
# This lets us glob() up all the files inside the examples to make them inputs to tests
# (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it)
# To update these lines, execute
# `bazel run @rules_bazel_integration_test//tools:update_deleted_packages`
build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,rules_python-repro,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/another_module,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2,tests/modules/other/with_external_data,tests/whl_with_build_files/testdata,tests/whl_with_build_files/testdata/somepkg,tests/whl_with_build_files/testdata/somepkg-1.0.dist-info,tests/whl_with_build_files/testdata/somepkg/subpkg
query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,rules_python-repro,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/another_module,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2,tests/modules/other/with_external_data,tests/whl_with_build_files/testdata,tests/whl_with_build_files/testdata/somepkg,tests/whl_with_build_files/testdata/somepkg-1.0.dist-info,tests/whl_with_build_files/testdata/somepkg/subpkg
# `tools/update_deleted_packages.sh`
# GENERATED_DELETED_PACKAGES_START
common --deleted_packages=examples/build_file_generation
Comment on lines +7 to +8
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

The comment # GENERATED_DELETED_PACKAGES_START should be placed on the line immediately before the first common --deleted_packages entry to clearly demarcate the start of the generated section.

common --deleted_packages=examples/build_file_generation/random_number_generator
common --deleted_packages=examples/bzlmod
common --deleted_packages=examples/bzlmod/entry_points
common --deleted_packages=examples/bzlmod/entry_points/tests
common --deleted_packages=examples/bzlmod/libs/my_lib
common --deleted_packages=examples/bzlmod/other_module
common --deleted_packages=examples/bzlmod/other_module/other_module/pkg
common --deleted_packages=examples/bzlmod/patches
common --deleted_packages=examples/bzlmod/py_proto_library
common --deleted_packages=examples/bzlmod/py_proto_library/example.com/another_proto
common --deleted_packages=examples/bzlmod/py_proto_library/example.com/proto
common --deleted_packages=examples/bzlmod/py_proto_library/foo_external
common --deleted_packages=examples/bzlmod/runfiles
common --deleted_packages=examples/bzlmod/tests
common --deleted_packages=examples/bzlmod/tests/other_module
common --deleted_packages=examples/bzlmod/whl_mods
common --deleted_packages=examples/bzlmod_build_file_generation
common --deleted_packages=examples/bzlmod_build_file_generation/other_module
common --deleted_packages=examples/bzlmod_build_file_generation/other_module/other_module/pkg
common --deleted_packages=examples/bzlmod_build_file_generation/runfiles
common --deleted_packages=examples/multi_python_versions
common --deleted_packages=examples/multi_python_versions/libs/my_lib
common --deleted_packages=examples/multi_python_versions/requirements
common --deleted_packages=examples/multi_python_versions/tests
common --deleted_packages=examples/pip_parse
common --deleted_packages=examples/pip_parse_vendored
common --deleted_packages=examples/pip_repository_annotations
common --deleted_packages=examples/py_proto_library
common --deleted_packages=examples/py_proto_library/example.com/another_proto
common --deleted_packages=examples/py_proto_library/example.com/proto
common --deleted_packages=gazelle
common --deleted_packages=gazelle/docs
common --deleted_packages=gazelle/manifest
common --deleted_packages=gazelle/manifest/generate
common --deleted_packages=gazelle/manifest/hasher
common --deleted_packages=gazelle/manifest/test
common --deleted_packages=gazelle/modules_mapping
common --deleted_packages=gazelle/python
common --deleted_packages=gazelle/python/private
common --deleted_packages=gazelle/python/testdata/add_type_stub_packages
common --deleted_packages=gazelle/python/testdata/annotation_include_dep
common --deleted_packages=gazelle/python/testdata/annotation_include_pytest_conftest
common --deleted_packages=gazelle/python/testdata/binary_without_entrypoint
common --deleted_packages=gazelle/python/testdata/binary_without_entrypoint_per_file_generation
common --deleted_packages=gazelle/python/testdata/binary_without_entrypoint_per_file_generation_partial_update
common --deleted_packages=gazelle/python/testdata/clear_out_deps
common --deleted_packages=gazelle/python/testdata/dependency_resolution_order
common --deleted_packages=gazelle/python/testdata/different_packages_in_same_namespace
common --deleted_packages=gazelle/python/testdata/directive_python_default_visibility
common --deleted_packages=gazelle/python/testdata/directive_python_generate_proto
common --deleted_packages=gazelle/python/testdata/directive_python_generate_proto_bzlmod_protobuf
common --deleted_packages=gazelle/python/testdata/directive_python_generate_proto_bzlmod_protobuf_renamed
common --deleted_packages=gazelle/python/testdata/directive_python_label_convention
common --deleted_packages=gazelle/python/testdata/directive_python_label_normalization
common --deleted_packages=gazelle/python/testdata/directive_python_proto_naming_convention
common --deleted_packages=gazelle/python/testdata/directive_python_test_file_pattern
common --deleted_packages=gazelle/python/testdata/directive_python_test_file_pattern_bad_glob
common --deleted_packages=gazelle/python/testdata/directive_python_test_file_pattern_no_value
common --deleted_packages=gazelle/python/testdata/directive_python_visibility
common --deleted_packages=gazelle/python/testdata/disable_import_statements_validation
common --deleted_packages=gazelle/python/testdata/dont_ignore_setup
common --deleted_packages=gazelle/python/testdata/dont_rename_target
common --deleted_packages=gazelle/python/testdata/file_name_matches_import_statement
common --deleted_packages=gazelle/python/testdata/first_party_dependencies
common --deleted_packages=gazelle/python/testdata/first_party_file_and_directory_modules
common --deleted_packages=gazelle/python/testdata/from_imports
common --deleted_packages=gazelle/python/testdata/generated_test_entrypoint
common --deleted_packages=gazelle/python/testdata/ignored_invalid_imported_module
common --deleted_packages=gazelle/python/testdata/invalid_annotation_exclude
common --deleted_packages=gazelle/python/testdata/invalid_annotation_include_dep
common --deleted_packages=gazelle/python/testdata/invalid_imported_module
common --deleted_packages=gazelle/python/testdata/monorepo
common --deleted_packages=gazelle/python/testdata/multiple_tests
common --deleted_packages=gazelle/python/testdata/naming_convention
common --deleted_packages=gazelle/python/testdata/naming_convention_binary_fail
common --deleted_packages=gazelle/python/testdata/naming_convention_library_fail
common --deleted_packages=gazelle/python/testdata/naming_convention_test_fail
common --deleted_packages=gazelle/python/testdata/per_file
common --deleted_packages=gazelle/python/testdata/per_file_non_empty_init
common --deleted_packages=gazelle/python/testdata/per_file_subdirs
common --deleted_packages=gazelle/python/testdata/per_package_test_target_without_entry_point
common --deleted_packages=gazelle/python/testdata/project_generation_mode
common --deleted_packages=gazelle/python/testdata/project_generation_mode_with_test_entrypoint
common --deleted_packages=gazelle/python/testdata/project_generation_mode_with_tests
common --deleted_packages=gazelle/python/testdata/py312_syntax
common --deleted_packages=gazelle/python/testdata/python_ignore_dependencies_directive
common --deleted_packages=gazelle/python/testdata/python_ignore_files_directive
common --deleted_packages=gazelle/python/testdata/python_target_with_test_in_name
common --deleted_packages=gazelle/python/testdata/relative_imports_package_mode
common --deleted_packages=gazelle/python/testdata/relative_imports_project_mode
common --deleted_packages=gazelle/python/testdata/remove_invalid_library
common --deleted_packages=gazelle/python/testdata/respect_kind_mapping
common --deleted_packages=gazelle/python/testdata/sibling_imports
common --deleted_packages=gazelle/python/testdata/sibling_imports_disabled
common --deleted_packages=gazelle/python/testdata/sibling_imports_disabled_file_mode
common --deleted_packages=gazelle/python/testdata/simple_binary
common --deleted_packages=gazelle/python/testdata/simple_binary_with_library
common --deleted_packages=gazelle/python/testdata/simple_library
common --deleted_packages=gazelle/python/testdata/simple_library_without_init
common --deleted_packages=gazelle/python/testdata/simple_test
common --deleted_packages=gazelle/python/testdata/simple_test_with_conftest
common --deleted_packages=gazelle/python/testdata/simple_test_with_conftest_sibling_imports_disabled
common --deleted_packages=gazelle/python/testdata/subdir_sources
common --deleted_packages=gazelle/python/testdata/type_checking_imports
common --deleted_packages=gazelle/python/testdata/type_checking_imports_across_packages
common --deleted_packages=gazelle/python/testdata/type_checking_imports_disabled
common --deleted_packages=gazelle/python/testdata/type_checking_imports_package
common --deleted_packages=gazelle/python/testdata/type_checking_imports_project
common --deleted_packages=gazelle/python/testdata/with_nested_import_statements
common --deleted_packages=gazelle/python/testdata/with_std_requirements
common --deleted_packages=gazelle/python/testdata/with_third_party_requirements
common --deleted_packages=gazelle/python/testdata/with_third_party_requirements_from_imports
common --deleted_packages=gazelle/pythonconfig
common --deleted_packages=tests/integration/compile_pip_requirements
common --deleted_packages=tests/integration/compile_pip_requirements_test_from_external_repo
common --deleted_packages=tests/integration/custom_commands
common --deleted_packages=tests/integration/ignore_root_user_error
common --deleted_packages=tests/integration/ignore_root_user_error/submodule
common --deleted_packages=tests/integration/local_toolchains
common --deleted_packages=tests/integration/pip_parse
common --deleted_packages=tests/integration/pip_parse/empty
common --deleted_packages=tests/integration/py_cc_toolchain_registered
common --deleted_packages=tests/modules/another_module
common --deleted_packages=tests/modules/other
common --deleted_packages=tests/modules/other/nspkg_delta
common --deleted_packages=tests/modules/other/nspkg_gamma
common --deleted_packages=tests/modules/other/nspkg_single
common --deleted_packages=tests/modules/other/simple_v1
common --deleted_packages=tests/modules/other/simple_v2
common --deleted_packages=tests/modules/other/with_external_data
# GENERATED_DELETED_PACKAGES_END
Comment on lines +138 to +139
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

The comment # GENERATED_DELETED_PACKAGES_END should be placed on the line immediately after the last common --deleted_packages entry to clearly demarcate the end of the generated section.


test --test_output=errors

Expand Down
4 changes: 1 addition & 3 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ repos:
- id: update-deleted-packages
name: Update deleted packages
language: system
# 7.x is necessary until https://github.com/bazel-contrib/rules_bazel_integration_test/pull/414
# is merged and released
entry: env USE_BAZEL_VERSION=7.x bazel run @rules_bazel_integration_test//tools:update_deleted_packages
entry: tools/update_deleted_packages.sh
files: ^((examples|tests)/.*/(MODULE.bazel|WORKSPACE|WORKSPACE.bzlmod|BUILD.bazel)|.bazelrc)$
pass_filenames: false
78 changes: 54 additions & 24 deletions tools/update_deleted_packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,57 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# For integration tests, we want to be able to glob() up the sources inside a nested package
# See explanation in .bazelrc
#
# This script ensures that we only delete subtrees that have something a file
# signifying a new bazel workspace, whether it be bzlmod or classic. Generic
# algorithm:
# 1. Get all directories where a WORKSPACE or MODULE.bazel exists.
# 2. For each of the directories, get all directories that contains a BUILD.bazel file.
# 3. Sort and remove duplicates.

set -euxo pipefail

DIR="$(dirname $0)/.."
cd $DIR

# The sed -i.bak pattern is compatible between macos and linux
sed -i.bak "/^[^#].*--deleted_packages/s#=.*#=$(\
find examples/*/* tests/*/* \( -name WORKSPACE -or -name MODULE.bazel \) |
xargs -n 1 dirname |
xargs -n 1 -I{} find {} \( -name BUILD -or -name BUILD.bazel \) |
xargs -n 1 dirname |
sort -u |
paste -sd, -\
)#" $DIR/.bazelrc && rm .bazelrc.bak
# This script finds Bazel sub-workspaces and marks them, and all their BUILD
# containing directories, as ignored by the root level bazel project, via the
# `--deleted_packages` flag.

set -euo pipefail

DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
ROOT_DIR="$DIR/.."
BAZELRC="$ROOT_DIR/.bazelrc"

# Find all packages in sub-workspaces
found_packages() {
cd "$ROOT_DIR"
(
find . -mindepth 2 \( -name WORKSPACE -o -name MODULE.bazel \) |
while read -r marker; do
workspace_dir="$(dirname "$marker")"
echo "$workspace_dir"
find "$workspace_dir" \( -name BUILD -o -name BUILD.bazel \) -exec dirname {} \;
done
) | sed 's#^\./##'
Comment on lines +30 to +36
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

The find command within the found_packages function could be simplified by combining the -name options into a single expression using -o (OR operator) and grouping them with parentheses. This improves readability and potentially performance.

Also, the sed command at the end of the function can be simplified by using the s#^ ./## pattern instead of s#^./## to remove the leading ./ from the paths.

Suggested change
find . -mindepth 2 \( -name WORKSPACE -o -name MODULE.bazel \) |
while read -r marker; do
workspace_dir="$(dirname "$marker")"
echo "$workspace_dir"
find "$workspace_dir" \( -name BUILD -o -name BUILD.bazel \) -exec dirname {} \;
done
) | sed 's#^\./##'
find . -mindepth 2 \( -name WORKSPACE -o -name MODULE.bazel \) |
while read -r marker; do
workspace_dir="$(dirname "$marker")"
echo "$workspace_dir"
find "$workspace_dir" \( -name BUILD -o -name BUILD.bazel \) -exec dirname {} \;
done
) | sed 's#^./##'

}

# Update .bazelrc
update_bazelrc() {
local packages
# Set LC_COLLATE so that "/" sorts before "_", which makes
# for slightly nicer ordering with file paths
packages=$(found_packages | LC_COLLATE=C sort | uniq)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

The variable packages is assigned the output of found_packages | LC_COLLATE=C sort | uniq. If found_packages returns an error, this error will not be propagated because it's part of a pipeline. To ensure that errors are caught, it's better to store the output of found_packages in a temporary variable and check its exit code before proceeding.

Also, consider using tr '\n' ' ' instead of a loop to concatenate the packages into a single string, as it's more efficient.


local start_marker="# GENERATED_DELETED_PACKAGES_START"
local end_marker="# GENERATED_DELETED_PACKAGES_END"

# Create a temporary file
local tmpfile
tmpfile=$(mktemp)

# Write the content before the start marker
sed "/$start_marker/q" "$BAZELRC" > "$tmpfile"
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

The sed command uses the q option to quit after finding the start marker. This is efficient, but it might be more robust to use a different approach that ensures the entire file is processed, especially if the start marker is not found. Consider using a different sed command that doesn't rely on quitting mid-file.


# Write the generated packages
for pkg in $packages; do
echo "common --deleted_packages=$pkg" >> "$tmpfile"
done
Comment on lines +57 to +59
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

medium

Using a while loop to iterate through the packages is less efficient than using printf to generate all the lines at once and then appending them to the file. This reduces the number of write operations and improves performance. Also, consider adding a check to ensure that the package variable is not empty before appending it to the file.

Suggested change
for pkg in $packages; do
echo "common --deleted_packages=$pkg" >> "$tmpfile"
done
printf "common --deleted_packages=%s\n" "$packages" >> "$tmpfile"

echo "$end_marker" >> "$tmpfile"

# Write the content after the end marker
sed "1,/$end_marker/d" "$BAZELRC" >> "$tmpfile"
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 17, 2025

Choose a reason for hiding this comment

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

high

The sed command uses 1,/$end_marker/d to delete lines from the beginning of the file up to and including the end marker. This could be problematic if the end marker is not found, as it would delete the entire file. Consider adding a check to ensure that the end marker exists before running this command, or use a more robust approach that doesn't rely on deleting lines based on a marker.


# Replace the original file
mv "$tmpfile" "$BAZELRC"
}

update_bazelrc