From 4729a488236ffcc2763e1c3368b48c44f055f8db Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 19 Sep 2022 12:32:39 +0200 Subject: [PATCH 1/7] Do not duplicate packages when running gazelle-update-repos --- gazelle_cabal/dependency_resolution.go | 51 +++++++++++++++++++-- gazelle_cabal/dependency_resolution_test.go | 7 +++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/gazelle_cabal/dependency_resolution.go b/gazelle_cabal/dependency_resolution.go index 39b9e2b..d8f36cd 100644 --- a/gazelle_cabal/dependency_resolution.go +++ b/gazelle_cabal/dependency_resolution.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "strconv" "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/label" @@ -155,7 +156,7 @@ func rel(lbl label.Label, from label.Label) label.Label { // haskell_library are assumed to be local. There rest of the // dependencies are assumed to come from packageRepo. func setDepsAndPluginsAttributes( - extraLibraries []label.Label, + extraLibraries []label.Label, packageRepo string, ix *resolve.RuleIndex, r *rule.Rule, @@ -253,7 +254,7 @@ func getRuleIndexKeys(depName string, from label.Label, cabalPkgName string) []s // colon prefix detected packagePrefix := splitted[0] libraryName := splitted[1] - if (packagePrefix == cabalPkgName) { + if packagePrefix == cabalPkgName { // the package prefix leads to a sub-library of the same package return []string{ fmt.Sprintf(format, privatePrefix, packagePrefix, libraryName), @@ -408,7 +409,51 @@ func mapSortedStringKeys(m map[string]bool) []string { i++ } sort.Strings(ss) - return ss + s_result := removeEquivalent(samePackage, ss) + return s_result +} + +// This function assumes that equivalent elements are bundled and only keeps the last element of each equivalence class. +func removeEquivalent(equiv func(string, string) bool, s []string) []string { + if len(s) < 1 { + return s + } + + first_free_position := 1 + for curr := 1; curr < len(s); curr++ { + if equiv(s[first_free_position-1], s[curr]) { + // If the last added element is equivalent is considered one, + // then we move the cursor back so that the currently considered element + // replace the previously added one. + // This function is only keeping the last representative of the equivalence class + // because it is used to deal with packages having or not a version number, + // and in case both option are available, the version number should be kept. + first_free_position-- + } + s[first_free_position] = s[curr] + first_free_position++ + } + + return s[:first_free_position] +} + +func samePackage(s1 string, s2 string) bool { + ss1 := chopVersionNumber(s1) + ss2 := chopVersionNumber(s2) + return ss1 == ss2 +} + +func chopVersionNumber(s string) string { + l := strings.Split(s, "-") + n := len(l) + // strconv.Atoi converts a string to an integer, + // and potentially outputs an error message. + _, err := strconv.Atoi(strings.ReplaceAll(l[n-1], ".", "")) + // If the conversion succeeded, the error is 'nil'. + if err == nil { + return strings.Join(l[:n-1], "-") + } + return s } // label.Parse chokes on hyphenated repo names with diff --git a/gazelle_cabal/dependency_resolution_test.go b/gazelle_cabal/dependency_resolution_test.go index 51ea734..b245df4 100644 --- a/gazelle_cabal/dependency_resolution_test.go +++ b/gazelle_cabal/dependency_resolution_test.go @@ -141,6 +141,13 @@ func TestMapSortedStringKeys(t *testing.T) { t.Errorf("got %v, wanted %v", got, wanted) } + m = map[string]bool{"a": true, "b": false, "b-1.0": false} + got = mapSortedStringKeys(m) + wanted = []string{"a", "b-1.0"} + if !reflect.DeepEqual(got, wanted) { + t.Errorf("got %v, wanted %v", got, wanted) + } + m = map[string]bool{} got = mapSortedStringKeys(m) wanted = []string{} From 124ad22d840daf08b60625030b99638b5fcb3f73 Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Tue, 20 Sep 2022 12:05:26 +0200 Subject: [PATCH 2/7] Facundo's comments --- gazelle_cabal/dependency_resolution.go | 36 ++++++++++++--------- gazelle_cabal/dependency_resolution_test.go | 18 +++++++++-- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/gazelle_cabal/dependency_resolution.go b/gazelle_cabal/dependency_resolution.go index d8f36cd..19e189c 100644 --- a/gazelle_cabal/dependency_resolution.go +++ b/gazelle_cabal/dependency_resolution.go @@ -5,7 +5,7 @@ import ( "errors" "flag" "fmt" - "strconv" + "regexp" "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/label" @@ -413,7 +413,15 @@ func mapSortedStringKeys(m map[string]bool) []string { return s_result } -// This function assumes that equivalent elements are bundled and only keeps the last element of each equivalence class. +// This function assumes that equivalent elements are bundled. +// Hence, the input list should be preprocessed with a function ensuring this property. +// +// In our use-case, it means that +// the input list must be sorted. +// +// This function is only keeping the last representative of the equivalence class. +// The rationale is it is used to deal with packages having or not a version number, +// and in case both option are available, the version number should be kept. func removeEquivalent(equiv func(string, string) bool, s []string) []string { if len(s) < 1 { return s @@ -422,35 +430,31 @@ func removeEquivalent(equiv func(string, string) bool, s []string) []string { first_free_position := 1 for curr := 1; curr < len(s); curr++ { if equiv(s[first_free_position-1], s[curr]) { - // If the last added element is equivalent is considered one, - // then we move the cursor back so that the currently considered element - // replace the previously added one. - // This function is only keeping the last representative of the equivalence class - // because it is used to deal with packages having or not a version number, - // and in case both option are available, the version number should be kept. - first_free_position-- + s[first_free_position-1] = s[curr] + } else { + s[first_free_position] = s[curr] + first_free_position++ } - s[first_free_position] = s[curr] - first_free_position++ } return s[:first_free_position] } +// The same package should not appear twice in a 'stack_snapshot' rule. +// See https://github.com/tweag/gazelle_cabal/issue/46 func samePackage(s1 string, s2 string) bool { ss1 := chopVersionNumber(s1) ss2 := chopVersionNumber(s2) return ss1 == ss2 } +// Strips a suffix "-digit+[.digit+]*" from the given string if present. +// Otherwise returns the string unmodified. func chopVersionNumber(s string) string { l := strings.Split(s, "-") n := len(l) - // strconv.Atoi converts a string to an integer, - // and potentially outputs an error message. - _, err := strconv.Atoi(strings.ReplaceAll(l[n-1], ".", "")) - // If the conversion succeeded, the error is 'nil'. - if err == nil { + matched, _ := regexp.MatchString(`^[0-9]+(\.[0-9]+)*$`, l[n-1]) + if matched { return strings.Join(l[:n-1], "-") } return s diff --git a/gazelle_cabal/dependency_resolution_test.go b/gazelle_cabal/dependency_resolution_test.go index b245df4..86871f4 100644 --- a/gazelle_cabal/dependency_resolution_test.go +++ b/gazelle_cabal/dependency_resolution_test.go @@ -141,9 +141,23 @@ func TestMapSortedStringKeys(t *testing.T) { t.Errorf("got %v, wanted %v", got, wanted) } - m = map[string]bool{"a": true, "b": false, "b-1.0": false} + m = map[string]bool{"a": true, "b": false, "b-.1.02": false} got = mapSortedStringKeys(m) - wanted = []string{"a", "b-1.0"} + wanted = []string{"a", "b", "b-.1.02"} + if !reflect.DeepEqual(got, wanted) { + t.Errorf("got %v, wanted %v", got, wanted) + } + + m = map[string]bool{"a": true, "b": false, "b-1.02.": false} + got = mapSortedStringKeys(m) + wanted = []string{"a", "b", "b-1.02."} + if !reflect.DeepEqual(got, wanted) { + t.Errorf("got %v, wanted %v", got, wanted) + } + + m = map[string]bool{"a": true, "b": false, "b-1.02": false} + got = mapSortedStringKeys(m) + wanted = []string{"a", "b-1.02"} if !reflect.DeepEqual(got, wanted) { t.Errorf("got %v, wanted %v", got, wanted) } From 966b08249a92ca00c7141ed71d81993c6ad910e9 Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 3 Oct 2022 17:44:23 +0200 Subject: [PATCH 3/7] Attempt to modify Fix function --- gazelle_cabal/dependency_resolution.go | 19 ++++++++++++------- gazelle_cabal/lang.go | 25 +++++++++++++++---------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/gazelle_cabal/dependency_resolution.go b/gazelle_cabal/dependency_resolution.go index 19e189c..de673dc 100644 --- a/gazelle_cabal/dependency_resolution.go +++ b/gazelle_cabal/dependency_resolution.go @@ -408,6 +408,10 @@ func mapSortedStringKeys(m map[string]bool) []string { ss[i] = s i++ } + listSortedStringKeys(ss) +} + +func listSortedStringKeys(ss []string) []string { sort.Strings(ss) s_result := removeEquivalent(samePackage, ss) return s_result @@ -450,14 +454,15 @@ func samePackage(s1 string, s2 string) bool { // Strips a suffix "-digit+[.digit+]*" from the given string if present. // Otherwise returns the string unmodified. +regexp.Regexp chopVersionNumberRegexp = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`) + func chopVersionNumber(s string) string { - l := strings.Split(s, "-") - n := len(l) - matched, _ := regexp.MatchString(`^[0-9]+(\.[0-9]+)*$`, l[n-1]) - if matched { - return strings.Join(l[:n-1], "-") - } - return s + loc := chopVersionNumberRegexp.FindStringIndex(s) + if loc == nil { + return s + } else { + return s[:loc[0]] + } } // label.Parse chokes on hyphenated repo names with diff --git a/gazelle_cabal/lang.go b/gazelle_cabal/lang.go index 65ec323..472d9ea 100644 --- a/gazelle_cabal/lang.go +++ b/gazelle_cabal/lang.go @@ -80,14 +80,14 @@ var haskellAttrInfo = rule.KindInfo{ MatchAttrs: []string{}, NonEmptyAttrs: map[string]bool{}, ResolveAttrs: map[string]bool{ - "ghcopts": true, - "data": true, - "deps": true, - "main_file": true, - "plugins": true, - "srcs": true, - "tools": true, - "version": true, + "ghcopts": true, + "data": true, + "deps": true, + "main_file": true, + "plugins": true, + "srcs": true, + "tools": true, + "version": true, }, } @@ -130,7 +130,7 @@ func (*gazelleCabalLang) Imports(c *config.Config, r *rule.Rule, f *rule.File) [ case "haskell_library": visibility := r.PrivateAttr("visibility") pkgName := r.PrivateAttr("pkgName") - if visibility == "private" { + if visibility == "private" { prefix = fmt.Sprintf("private_library:%s:", pkgName) } else { prefix = fmt.Sprintf("public_library:%s:", pkgName) @@ -169,7 +169,7 @@ func (*gazelleCabalLang) GenerateRules(args language.GenerateArgs) language.Gene generateResult := infoToRules(args.Config.RepoRoot, ruleInfos) - if (args.File != nil) { + if args.File != nil { copyPrivateAttrs(generateResult.Gen, args.File.Rules) } @@ -210,6 +210,11 @@ func (*gazelleCabalLang) Fix(c *config.Config, f *rule.File) { !hasRuleInfo(ruleInfos, r.Kind(), r.Name()) { r.Delete() } + + if !r.ShouldKeep() && + r.Kind() == "stack_snapshot" { + r.Attrs["packages"] = listSortedStringKeys(r.Attrs["packages"]) + } } } From 362e862c9fa1b922f92814cefd0360ea3fb24554 Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 3 Oct 2022 17:52:18 +0200 Subject: [PATCH 4/7] Syntax --- gazelle_cabal/dependency_resolution.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gazelle_cabal/dependency_resolution.go b/gazelle_cabal/dependency_resolution.go index de673dc..915a102 100644 --- a/gazelle_cabal/dependency_resolution.go +++ b/gazelle_cabal/dependency_resolution.go @@ -454,15 +454,15 @@ func samePackage(s1 string, s2 string) bool { // Strips a suffix "-digit+[.digit+]*" from the given string if present. // Otherwise returns the string unmodified. -regexp.Regexp chopVersionNumberRegexp = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`) +const chopVersionNumberRegexp regexp.Regexp = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`) func chopVersionNumber(s string) string { - loc := chopVersionNumberRegexp.FindStringIndex(s) - if loc == nil { - return s - } else { - return s[:loc[0]] - } + loc := chopVersionNumberRegexp.FindStringIndex(s) + if loc == nil { + return s + } else { + return s[:loc[0]] + } } // label.Parse chokes on hyphenated repo names with From 7388c7e39d2605ebba0afd804616c769ece9e41b Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 3 Oct 2022 19:10:10 +0200 Subject: [PATCH 5/7] Ugly type annotations --- gazelle_cabal/BUILD.bazel | 1 + gazelle_cabal/dependency_resolution.go | 4 ++-- gazelle_cabal/lang.go | 18 +++++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/gazelle_cabal/BUILD.bazel b/gazelle_cabal/BUILD.bazel index 5a17a4a..e343046 100644 --- a/gazelle_cabal/BUILD.bazel +++ b/gazelle_cabal/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "@bazel_gazelle//resolve:go_default_library", "@bazel_gazelle//rule:go_default_library", "@bazel_gazelle//walk:go_default_library", + "@com_github_bazelbuild_buildtools//build:go_default_library", "@io_bazel_rules_go//go/tools/bazel:go_default_library", ], ) diff --git a/gazelle_cabal/dependency_resolution.go b/gazelle_cabal/dependency_resolution.go index 915a102..412b483 100644 --- a/gazelle_cabal/dependency_resolution.go +++ b/gazelle_cabal/dependency_resolution.go @@ -408,7 +408,7 @@ func mapSortedStringKeys(m map[string]bool) []string { ss[i] = s i++ } - listSortedStringKeys(ss) + return listSortedStringKeys(ss) } func listSortedStringKeys(ss []string) []string { @@ -454,7 +454,7 @@ func samePackage(s1 string, s2 string) bool { // Strips a suffix "-digit+[.digit+]*" from the given string if present. // Otherwise returns the string unmodified. -const chopVersionNumberRegexp regexp.Regexp = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`) +var chopVersionNumberRegexp, _ = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`) func chopVersionNumber(s string) string { loc := chopVersionNumberRegexp.FindStringIndex(s) diff --git a/gazelle_cabal/lang.go b/gazelle_cabal/lang.go index 472d9ea..37e01fc 100644 --- a/gazelle_cabal/lang.go +++ b/gazelle_cabal/lang.go @@ -15,6 +15,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/repo" "github.com/bazelbuild/bazel-gazelle/resolve" "github.com/bazelbuild/bazel-gazelle/rule" + bzl "github.com/bazelbuild/buildtools/build" "github.com/bazelbuild/rules_go/go/tools/bazel" "os" @@ -213,7 +214,22 @@ func (*gazelleCabalLang) Fix(c *config.Config, f *rule.File) { if !r.ShouldKeep() && r.Kind() == "stack_snapshot" { - r.Attrs["packages"] = listSortedStringKeys(r.Attrs["packages"]) + var list []string + pack := r.Attr("packages") + switch expr := pack.(type) { + case *bzl.ListExpr: + for _, elem := range expr.List { + switch exprElem := elem.(type) { + case *bzl.StringExpr: + list = append(list, exprElem.Value) + default: + panic("Elements of packages should be string!") + } + } + default: + panic("Packages should be a list!") + } + r.SetAttr("packages", listSortedStringKeys(list)) } } } From fed76e2720112b66bd5075362d88a164bd818475 Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 3 Oct 2022 19:12:52 +0200 Subject: [PATCH 6/7] Add an ugly print --- gazelle_cabal/lang.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gazelle_cabal/lang.go b/gazelle_cabal/lang.go index 37e01fc..07f07f4 100644 --- a/gazelle_cabal/lang.go +++ b/gazelle_cabal/lang.go @@ -229,6 +229,7 @@ func (*gazelleCabalLang) Fix(c *config.Config, f *rule.File) { default: panic("Packages should be a list!") } + print(list) r.SetAttr("packages", listSortedStringKeys(list)) } } From 999e16ac79dc5ea3567c2fd4a2158ef1c3798dfa Mon Sep 17 00:00:00 2001 From: GuillaumeGen Date: Mon, 3 Oct 2022 19:15:19 +0200 Subject: [PATCH 7/7] Debugging --- gazelle_cabal/lang.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gazelle_cabal/lang.go b/gazelle_cabal/lang.go index 07f07f4..bd5d659 100644 --- a/gazelle_cabal/lang.go +++ b/gazelle_cabal/lang.go @@ -206,6 +206,8 @@ func (*gazelleCabalLang) Fix(c *config.Config, f *rule.File) { ruleInfos := cabalToRuleInfos(cabalFiles) for _, r := range f.Rules { + print(r) + if !r.ShouldKeep() && isHaskellRule(r.Kind()) && !hasRuleInfo(ruleInfos, r.Kind(), r.Name()) { @@ -214,6 +216,7 @@ func (*gazelleCabalLang) Fix(c *config.Config, f *rule.File) { if !r.ShouldKeep() && r.Kind() == "stack_snapshot" { + print("Case accessed") var list []string pack := r.Attr("packages") switch expr := pack.(type) {