From 597fc19c6d7a8dc1e2470eda32a40449e864e351 Mon Sep 17 00:00:00 2001 From: Abhishek Shivanna Date: Sun, 28 Aug 2022 12:55:55 -0700 Subject: [PATCH 1/2] Make gazelle_python_manifest output deterministic Fixes 812 --- gazelle/manifest/manifest.go | 23 ++++++++++++++++++++++- gazelle/manifest/manifest_test.go | 4 ++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/gazelle/manifest/manifest.go b/gazelle/manifest/manifest.go index b92706a230..1ad25f39af 100644 --- a/gazelle/manifest/manifest.go +++ b/gazelle/manifest/manifest.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "sort" yaml "gopkg.in/yaml.v2" ) @@ -94,11 +95,31 @@ func (f *File) Decode(manifestPath string) error { return nil } +// ModulesMapping is the type used to map from importable Python modules to +// the wheel names that provide these modules +type ModulesMapping map[string]string + +// MarshalYAML makes sure that we sort the module names before marshalling +// the contents of `ModulesMapping` to a yaml file. This ensures that the +// file is deterministically generated from the map +func (m ModulesMapping) MarshalYAML() (interface{}, error) { + var mapslice yaml.MapSlice + var keys []string + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + mapslice = append(mapslice, yaml.MapItem{Key: key, Value: m[key]}) + } + return mapslice, nil +} + // Manifest represents the structure of the Gazelle manifest file. type Manifest struct { // ModulesMapping is the mapping from importable modules to which Python // wheel name provides these modules. - ModulesMapping map[string]string `yaml:"modules_mapping"` + ModulesMapping ModulesMapping `yaml:"modules_mapping"` // PipDepsRepositoryName is the name of the pip_install repository target. // DEPRECATED PipDepsRepositoryName string `yaml:"pip_deps_repository_name,omitempty"` diff --git a/gazelle/manifest/manifest_test.go b/gazelle/manifest/manifest_test.go index 40a231f2bd..3b50fd1b3e 100644 --- a/gazelle/manifest/manifest_test.go +++ b/gazelle/manifest/manifest_test.go @@ -10,7 +10,7 @@ import ( "github.com/bazelbuild/rules_python/gazelle/manifest" ) -var modulesMapping = map[string]string{ +var modulesMapping = manifest.ModulesMapping{ "arrow": "arrow", "arrow.__init__": "arrow", "arrow.api": "arrow", @@ -76,4 +76,4 @@ func TestFile(t *testing.T) { t.FailNow() } }) -} \ No newline at end of file +} From 0e2b69c9632ed2467538a28a1bb97e581bbc1cdd Mon Sep 17 00:00:00 2001 From: Abhishek Shivanna Date: Mon, 29 Aug 2022 20:15:57 -0700 Subject: [PATCH 2/2] address review comments --- gazelle/manifest/BUILD.bazel | 5 ++++- gazelle/manifest/manifest.go | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/gazelle/manifest/BUILD.bazel b/gazelle/manifest/BUILD.bazel index 2e5b6b8e99..281bcd29cf 100644 --- a/gazelle/manifest/BUILD.bazel +++ b/gazelle/manifest/BUILD.bazel @@ -5,7 +5,10 @@ go_library( srcs = ["manifest.go"], importpath = "github.com/bazelbuild/rules_python/gazelle/manifest", visibility = ["//visibility:public"], - deps = ["@in_gopkg_yaml_v2//:yaml_v2"], + deps = [ + "@com_github_emirpasic_gods//sets/treeset", + "@in_gopkg_yaml_v2//:yaml_v2", + ], ) go_test( diff --git a/gazelle/manifest/manifest.go b/gazelle/manifest/manifest.go index 1ad25f39af..e19162bd5d 100644 --- a/gazelle/manifest/manifest.go +++ b/gazelle/manifest/manifest.go @@ -5,7 +5,8 @@ import ( "fmt" "io" "os" - "sort" + + "github.com/emirpasic/gods/sets/treeset" yaml "gopkg.in/yaml.v2" ) @@ -96,21 +97,20 @@ func (f *File) Decode(manifestPath string) error { } // ModulesMapping is the type used to map from importable Python modules to -// the wheel names that provide these modules +// the wheel names that provide these modules. type ModulesMapping map[string]string -// MarshalYAML makes sure that we sort the module names before marshalling -// the contents of `ModulesMapping` to a yaml file. This ensures that the -// file is deterministically generated from the map +// MarshalYAML makes sure that we sort the module names before marshaling +// the contents of `ModulesMapping` to a YAML file. This ensures that the +// file is deterministically generated from the map. func (m ModulesMapping) MarshalYAML() (interface{}, error) { var mapslice yaml.MapSlice - var keys []string + keySet := treeset.NewWithStringComparator() for key := range m { - keys = append(keys, key) + keySet.Add(key) } - sort.Strings(keys) - for _, key := range keys { - mapslice = append(mapslice, yaml.MapItem{Key: key, Value: m[key]}) + for _, key := range keySet.Values() { + mapslice = append(mapslice, yaml.MapItem{Key: key, Value: m[key.(string)]}) } return mapslice, nil }