Skip to content

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

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
51 changes: 48 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"
"strconv"

"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 @@ -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++
Copy link
Member

Choose a reason for hiding this comment

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

I find easier to follow the loop when not mutating the index.

Suggested change
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++
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]
}

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], ".", ""))
Copy link
Member

Choose a reason for hiding this comment

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

There is a low risk of overflow here. Would it be straighter to check that all characters are either digits or dots instead?

// 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
Expand Down
7 changes: 7 additions & 0 deletions gazelle_cabal/dependency_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down