From 47d95df41b8154d020cc42623b726b913974bcbd Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Thu, 18 Jan 2024 16:28:46 -0800 Subject: [PATCH] Make integrity field optional --- CHANGELOG.md | 3 + .../.bazelversion | 2 +- .../bzlmod_build_file_generation/BUILD.bazel | 1 + .../gazelle_python.yaml | 2 +- gazelle/MODULE.bazel | 1 + gazelle/manifest/BUILD.bazel | 5 + gazelle/manifest/copy_to_source.sh | 22 ++++ gazelle/manifest/defs.bzl | 105 +++++++++++------- gazelle/manifest/generate/generate.go | 27 ++--- gazelle/manifest/manifest.go | 14 ++- gazelle/manifest/manifest_test.go | 2 +- 11 files changed, 121 insertions(+), 63 deletions(-) create mode 100755 gazelle/manifest/copy_to_source.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 66f0fc9ea4..82f9592aff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,9 @@ A brief description of the categories of changes: target for each file with `if __name__ == "__main__"` instead of just one `py_binary` for the whole module. +* (gazelle) the Gazelle manifest integrity field is now optional. + Set `use_integrity_field = False` in the `gazelle_python_manifest` to disable it. + ### Fixed * (gazelle) The gazelle plugin helper was not working with Python toolchains 3.11 diff --git a/examples/bzlmod_build_file_generation/.bazelversion b/examples/bzlmod_build_file_generation/.bazelversion index 09b254e90c..19b860c187 100644 --- a/examples/bzlmod_build_file_generation/.bazelversion +++ b/examples/bzlmod_build_file_generation/.bazelversion @@ -1 +1 @@ -6.0.0 +6.4.0 diff --git a/examples/bzlmod_build_file_generation/BUILD.bazel b/examples/bzlmod_build_file_generation/BUILD.bazel index bca3b3681b..074bb92796 100644 --- a/examples/bzlmod_build_file_generation/BUILD.bazel +++ b/examples/bzlmod_build_file_generation/BUILD.bazel @@ -52,6 +52,7 @@ gazelle_python_manifest( "//:requirements_windows.txt", ], tags = ["exclusive"], + use_integrity_field = False, ) # Our gazelle target points to the python gazelle binary. diff --git a/examples/bzlmod_build_file_generation/gazelle_python.yaml b/examples/bzlmod_build_file_generation/gazelle_python.yaml index 46a1c8b337..c402f694fb 100644 --- a/examples/bzlmod_build_file_generation/gazelle_python.yaml +++ b/examples/bzlmod_build_file_generation/gazelle_python.yaml @@ -232,6 +232,7 @@ manifest: isort.wrap: isort isort.wrap_modes: isort lazy_object_proxy: lazy_object_proxy + lazy_object_proxy.cext: lazy_object_proxy lazy_object_proxy.compat: lazy_object_proxy lazy_object_proxy.simple: lazy_object_proxy lazy_object_proxy.slots: lazy_object_proxy @@ -586,4 +587,3 @@ manifest: yamllint.rules.truthy: yamllint pip_repository: name: pip -integrity: cd25503dc6b3d9e1c5f46715ba2d0499ecc8b3d654ebcbf9f4e52f2074290e0a diff --git a/gazelle/MODULE.bazel b/gazelle/MODULE.bazel index 8c6ad19c7b..940a263953 100644 --- a/gazelle/MODULE.bazel +++ b/gazelle/MODULE.bazel @@ -4,6 +4,7 @@ module( compatibility_level = 1, ) +bazel_dep(name = "bazel_skylib", version = "1.5.0") bazel_dep(name = "rules_python", version = "0.18.0") bazel_dep(name = "rules_go", version = "0.41.0", repo_name = "io_bazel_rules_go") bazel_dep(name = "gazelle", version = "0.33.0", repo_name = "bazel_gazelle") diff --git a/gazelle/manifest/BUILD.bazel b/gazelle/manifest/BUILD.bazel index fc7fa09632..47fbee5884 100644 --- a/gazelle/manifest/BUILD.bazel +++ b/gazelle/manifest/BUILD.bazel @@ -1,5 +1,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +exports_files([ + # This gets wrapped up into a sh_binary with args inside of the gazelle_python_manifest macro. + "copy_to_source.sh", +]) + go_library( name = "manifest", srcs = ["manifest.go"], diff --git a/gazelle/manifest/copy_to_source.sh b/gazelle/manifest/copy_to_source.sh new file mode 100755 index 0000000000..19541a23df --- /dev/null +++ b/gazelle/manifest/copy_to_source.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -o errexit -o nounset -o pipefail + +# Run like: +# copy_to_source.sh path/to/generated_file path/to/source_file_to_overwrite + +GENERATED_RELATIVE_PATH=$1 +shift +TARGET_RELATIVE_PATH=$1 +shift + +# This script gets executed from the runfiles dir, so find the absolute path to the generated file based on that. +GENERATED_ABSOLUTE_PATH="$PWD/$GENERATED_RELATIVE_PATH" + +# Similarly, the target is relative to the source directory. +TARGET_ABSOLUTE_PATH="$BUILD_WORKSPACE_DIRECTORY/$TARGET_RELATIVE_PATH" + +echo "Copying $GENERATED_ABSOLUTE_PATH to $TARGET_ABSOLUTE_PATH" + +mkdir --parents "$(dirname "$TARGET_ABSOLUTE_PATH")" +cp --force "$GENERATED_ABSOLUTE_PATH" "$TARGET_ABSOLUTE_PATH" diff --git a/gazelle/manifest/defs.bzl b/gazelle/manifest/defs.bzl index ef0f275464..39ca69b826 100644 --- a/gazelle/manifest/defs.bzl +++ b/gazelle/manifest/defs.bzl @@ -16,7 +16,8 @@ for updating and testing the Gazelle manifest file. """ -load("@io_bazel_rules_go//go:def.bzl", "GoSource", "go_binary", "go_test") +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") +load("@io_bazel_rules_go//go:def.bzl", "GoSource", "go_test") def gazelle_python_manifest( name, @@ -25,6 +26,7 @@ def gazelle_python_manifest( pip_repository_name = "", pip_deps_repository_name = "", manifest = ":gazelle_python.yaml", + use_integrity_field = True, **kwargs): """A macro for defining the updating and testing targets for the Gazelle manifest file. @@ -37,8 +39,12 @@ def gazelle_python_manifest( pip_deps_repository_name: deprecated - the old pip_install target name. modules_mapping: the target for the generated modules_mapping.json file. manifest: the target for the Gazelle manifest file. - **kwargs: other bazel attributes passed to the target target generated by - this macro. + use_integrity_field: if true, an integrity field is added to the YAML manifest. + This field means we can test whether the manifest is out of date without + downloading all the pip deps, but it can also cause frequent merge conflicts + in large repos. + **kwargs: other bazel attributes passed to the generate and test targets + generated by this macro. """ if pip_deps_repository_name != "": # buildifier: disable=print @@ -52,9 +58,13 @@ def gazelle_python_manifest( # This is a temporary check while pip_deps_repository_name exists as deprecated. fail("pip_repository_name must be set in //{}:{}".format(native.package_name(), name)) + test_target = "{}.test".format(name) update_target = "{}.update".format(name) update_target_label = "//{}:{}".format(native.package_name(), update_target) + manifest_genrule = manifest.replace(":", "") + ".genrule" + generated_manifest = manifest + ".generated" + manifest_generator = Label("//manifest/generate:generate") manifest_generator_hash = Label("//manifest/generate:generate_lib_sources_hash") if type(requirements) == "list": @@ -68,56 +78,73 @@ def gazelle_python_manifest( requirements = name + "_requirements_gen" update_args = [ - "--manifest-generator-hash", - "$(rootpath {})".format(manifest_generator_hash), - "--requirements", - "$(rootpath {})".format(requirements), - "--pip-repository-name", - pip_repository_name, - "--modules-mapping", - "$(rootpath {})".format(modules_mapping), - "--output", - "$(rootpath {})".format(manifest), - "--update-target", - update_target_label, + "--manifest-generator-hash=$(location {})".format(manifest_generator_hash), + "--requirements=$(location {})".format(requirements), + "--pip-repository-name={}".format(pip_repository_name), + "--modules-mapping=$(location {})".format(modules_mapping), + "--output=$(location {})".format(generated_manifest), + "--update-target={}".format(update_target_label), + "--use-integrity-field={}".format(use_integrity_field), ] - go_binary( - name = update_target, - embed = [Label("//manifest/generate:generate_lib")], - data = [ + native.genrule( + name = manifest_genrule, + outs = [generated_manifest], + cmd = "$(location {}) {}".format(manifest_generator, " ".join(update_args)), + tools = [manifest_generator], + srcs = [ manifest, modules_mapping, requirements, manifest_generator_hash, ], - args = update_args, - visibility = ["//visibility:private"], - tags = ["manual"], ) - attrs = { - "env": { - "_TEST_MANIFEST": "$(rootpath {})".format(manifest), - "_TEST_MANIFEST_GENERATOR_HASH": "$(rootpath {})".format(manifest_generator_hash), - "_TEST_REQUIREMENTS": "$(rootpath {})".format(requirements), - }, - "size": "small", - } - go_test( - name = "{}.test".format(name), - srcs = [Label("//manifest/test:test.go")], + native.sh_binary( + name = update_target, + srcs = [Label("//manifest:copy_to_source.sh")], + args = [ + "$(location {})".format(generated_manifest), + "$(location {})".format(manifest), + ], data = [ + generated_manifest, manifest, - requirements, - manifest_generator_hash, ], - rundir = ".", - deps = [Label("//manifest")], - # kwargs could contain test-specific attributes like size or timeout - **dict(attrs, **kwargs) + **kwargs ) + if use_integrity_field: + attrs = { + "env": { + "_TEST_MANIFEST": "$(rootpath {})".format(manifest), + "_TEST_MANIFEST_GENERATOR_HASH": "$(rootpath {})".format(manifest_generator_hash), + "_TEST_REQUIREMENTS": "$(rootpath {})".format(requirements), + }, + "size": "small", + } + go_test( + name = test_target, + srcs = [Label("//manifest/test:test.go")], + data = [ + manifest, + requirements, + manifest_generator_hash, + ], + rundir = ".", + deps = [Label("//manifest")], + # kwargs could contain test-specific attributes like size or timeout + **dict(attrs, **kwargs) + ) + else: + diff_test( + name = test_target, + file1 = generated_manifest, + file2 = manifest, + failure_message = "\n\nGazelle manifest is out of date. Run 'bazel run {}' to update it.".format(native.package_relative_label(update_target)), + **kwargs + ) + native.filegroup( name = name, srcs = [manifest], diff --git a/gazelle/manifest/generate/generate.go b/gazelle/manifest/generate/generate.go index bdd0206ccb..fd57b6433b 100644 --- a/gazelle/manifest/generate/generate.go +++ b/gazelle/manifest/generate/generate.go @@ -31,12 +31,6 @@ import ( "github.com/bazelbuild/rules_python/gazelle/manifest" ) -func init() { - if os.Getenv("BUILD_WORKSPACE_DIRECTORY") == "" { - log.Fatalln("ERROR: this program must run under Bazel") - } -} - func main() { var ( manifestGeneratorHashPath string @@ -45,6 +39,7 @@ func main() { modulesMappingPath string outputPath string updateTarget string + useIntegrityField bool ) flag.StringVar( &manifestGeneratorHashPath, @@ -77,6 +72,11 @@ func main() { "update-target", "", "The Bazel target to update the YAML manifest file.") + flag.BoolVar( + &useIntegrityField, + "use-integrity-field", + true, + "Whether to add an integrity field to the YAML manifest file.") flag.Parse() if requirementsPath == "" { @@ -102,12 +102,12 @@ func main() { header := generateHeader(updateTarget) repository := manifest.PipRepository{ - Name: pipRepositoryName, + Name: pipRepositoryName, } manifestFile := manifest.NewFile(&manifest.Manifest{ ModulesMapping: modulesMapping, - PipRepository: &repository, + PipRepository: &repository, }) if err := writeOutput( outputPath, @@ -115,6 +115,7 @@ func main() { manifestFile, manifestGeneratorHashPath, requirementsPath, + useIntegrityField, ); err != nil { log.Fatalf("ERROR: %v\n", err) } @@ -154,13 +155,9 @@ func writeOutput( manifestFile *manifest.File, manifestGeneratorHashPath string, requirementsPath string, + useIntegrityField bool, ) error { - stat, err := os.Stat(outputPath) - if err != nil { - return fmt.Errorf("failed to write output: %w", err) - } - - outputFile, err := os.OpenFile(outputPath, os.O_WRONLY|os.O_TRUNC, stat.Mode()) + outputFile, err := os.OpenFile(outputPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644) if err != nil { return fmt.Errorf("failed to write output: %w", err) } @@ -182,7 +179,7 @@ func writeOutput( } defer requirements.Close() - if err := manifestFile.Encode(outputFile, manifestGeneratorHash, requirements); err != nil { + if err := manifestFile.Encode(outputFile, manifestGeneratorHash, requirements, useIntegrityField); err != nil { return fmt.Errorf("failed to write output: %w", err) } diff --git a/gazelle/manifest/manifest.go b/gazelle/manifest/manifest.go index fb146f9439..bb805bfe3b 100644 --- a/gazelle/manifest/manifest.go +++ b/gazelle/manifest/manifest.go @@ -31,7 +31,7 @@ type File struct { // Integrity is the hash of the requirements.txt file and the Manifest for // ensuring the integrity of the entire gazelle_python.yaml file. This // controls the testing to keep the gazelle_python.yaml file up-to-date. - Integrity string `yaml:"integrity"` + Integrity string `yaml:"integrity,omitempty"` } // NewFile creates a new File with a given Manifest. @@ -40,12 +40,14 @@ func NewFile(manifest *Manifest) *File { } // Encode encodes the manifest file to the given writer. -func (f *File) Encode(w io.Writer, manifestGeneratorHashFile, requirements io.Reader) error { - integrityBytes, err := f.calculateIntegrity(manifestGeneratorHashFile, requirements) - if err != nil { - return fmt.Errorf("failed to encode manifest file: %w", err) +func (f *File) Encode(w io.Writer, manifestGeneratorHashFile, requirements io.Reader, useIntegrityField bool) error { + if useIntegrityField { + integrityBytes, err := f.calculateIntegrity(manifestGeneratorHashFile, requirements) + if err != nil { + return fmt.Errorf("failed to encode manifest file: %w", err) + } + f.Integrity = fmt.Sprintf("%x", integrityBytes) } - f.Integrity = fmt.Sprintf("%x", integrityBytes) encoder := yaml.NewEncoder(w) defer encoder.Close() if err := encoder.Encode(f); err != nil { diff --git a/gazelle/manifest/manifest_test.go b/gazelle/manifest/manifest_test.go index 43c4099aa1..39613d9c11 100644 --- a/gazelle/manifest/manifest_test.go +++ b/gazelle/manifest/manifest_test.go @@ -53,7 +53,7 @@ func TestFile(t *testing.T) { t.FailNow() } defer requirements.Close() - if err := f.Encode(&b, manifestGeneratorHashFile, requirements); err != nil { + if err := f.Encode(&b, manifestGeneratorHashFile, requirements, true); err != nil { log.Println(err) t.FailNow() }