Skip to content
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.

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

Do not duplicate packages when running gazelle-update-repos #47

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions gazelle_cabal/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand Down
60 changes: 57 additions & 3 deletions gazelle_cabal/dependency_resolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"flag"
"fmt"
"regexp"

"github.com/bazelbuild/bazel-gazelle/config"
"github.com/bazelbuild/bazel-gazelle/label"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -407,8 +408,61 @@ func mapSortedStringKeys(m map[string]bool) []string {
ss[i] = s
i++
}
return listSortedStringKeys(ss)
}

func listSortedStringKeys(ss []string) []string {
sort.Strings(ss)
return ss
s_result := removeEquivalent(samePackage, ss)
return s_result
}

// 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 {
facundominguez marked this conversation as resolved.
Show resolved Hide resolved
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]) {
s[first_free_position-1] = s[curr]
} else {
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 {
facundominguez marked this conversation as resolved.
Show resolved Hide resolved
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.
var chopVersionNumberRegexp, _ = regexp.Compile(`-[0-9]+(\.[0-9]+)*$`)

func chopVersionNumber(s string) string {
facundominguez marked this conversation as resolved.
Show resolved Hide resolved
loc := chopVersionNumberRegexp.FindStringIndex(s)
if loc == nil {
return s
} else {
return s[:loc[0]]
}
}

// label.Parse chokes on hyphenated repo names with
Expand Down
21 changes: 21 additions & 0 deletions gazelle_cabal/dependency_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,27 @@ func TestMapSortedStringKeys(t *testing.T) {
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", "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)
}

m = map[string]bool{}
got = mapSortedStringKeys(m)
wanted = []string{}
Expand Down
45 changes: 35 additions & 10 deletions gazelle_cabal/lang.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -80,14 +81,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,
},
}

Expand Down Expand Up @@ -130,7 +131,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)
Expand Down Expand Up @@ -169,7 +170,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)
}

Expand Down Expand Up @@ -205,11 +206,35 @@ 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()) {
r.Delete()
}

if !r.ShouldKeep() &&
r.Kind() == "stack_snapshot" {
print("Case accessed")
var list []string
pack := r.Attr("packages")
switch expr := pack.(type) {
Copy link
Member

Choose a reason for hiding this comment

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

This code is probably better put in a function that extracts an array of strings from an attribute value. gazelle_haskell_modules has such a function.

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!")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
panic("Elements of packages should be string!")
panic("Elements of packages should be strings!")

}
}
default:
panic("Packages should be a list!")
}
print(list)
Copy link
Member

Choose a reason for hiding this comment

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

There are a few print statements that should be removed.

r.SetAttr("packages", listSortedStringKeys(list))
Copy link
Member

Choose a reason for hiding this comment

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

Is reordering the strings necessary? If so, perhaps it should be mentioned in the README.

}
}
}

Expand Down