diff --git a/build/rewrite.go b/build/rewrite.go index 721765292..924c3bd4f 100644 --- a/build/rewrite.go +++ b/build/rewrite.go @@ -126,12 +126,18 @@ func doNotSort(x Expr) bool { return hasComment(x, "do not sort") } -// keepSorted reports whether x is marked with a comment containing +// forceKeepSorted reports whether x is marked with a comment containing // "keep sorted", case-insensitive. -func keepSorted(x Expr) bool { +func forceKeepSorted(x Expr) bool { return hasComment(x, "keep sorted") } +// keepSorted checks whether lists are to be sorted by default, and respects comments as overrides of the default table +// value +func keepSorted(x Expr) bool { + return !doNotSort(x) && (tables.SortListsByDefault || forceKeepSorted(x)) +} + // fixLabels rewrites labels into a canonical form. // // First, it joins labels written as string addition, turning @@ -450,7 +456,7 @@ func sortStringList(x Expr, context string) { return } - forceSort := keepSorted(list) || keepSorted(list.List[0]) + forceSort := forceKeepSorted(list) || forceKeepSorted(list.List[0]) // TODO(bazel-team): Decide how to recognize lists that cannot // be sorted. Avoiding all lists with comments avoids sorting diff --git a/tables/jsonparser.go b/tables/jsonparser.go index ca2bc4443..7d68ac406 100644 --- a/tables/jsonparser.go +++ b/tables/jsonparser.go @@ -30,6 +30,7 @@ type Definitions struct { SortableWhitelist map[string]bool NamePriority map[string]int StripLabelLeadingSlashes bool + SortListsByDefault bool ShortenAbsoluteLabelsToRelative bool } @@ -55,9 +56,9 @@ func ParseAndUpdateJSONDefinitions(file string, merge bool) error { } if merge { - MergeTables(definitions.IsLabelArg, definitions.LabelBlacklist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableBlacklist, definitions.SortableWhitelist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) + MergeTables(definitions.IsLabelArg, definitions.LabelBlacklist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableBlacklist, definitions.SortableWhitelist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.SortListsByDefault, definitions.ShortenAbsoluteLabelsToRelative) } else { - OverrideTables(definitions.IsLabelArg, definitions.LabelBlacklist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableBlacklist, definitions.SortableWhitelist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) + OverrideTables(definitions.IsLabelArg, definitions.LabelBlacklist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableBlacklist, definitions.SortableWhitelist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.SortListsByDefault, definitions.ShortenAbsoluteLabelsToRelative) } return nil } diff --git a/tables/jsonparser_test.go b/tables/jsonparser_test.go index 3bd854ec5..7bd651646 100644 --- a/tables/jsonparser_test.go +++ b/tables/jsonparser_test.go @@ -37,6 +37,7 @@ func TestParseJSONDefinitions(t *testing.T) { SortableWhitelist: map[string]bool{}, NamePriority: map[string]int{"name": -1}, StripLabelLeadingSlashes: true, + SortListsByDefault: true, } if !reflect.DeepEqual(expected, definitions) { t.Errorf("ParseJSONDefinitions() = %v; want %v", definitions, expected) diff --git a/tables/tables.go b/tables/tables.go index ab90043ad..14c03517a 100644 --- a/tables/tables.go +++ b/tables/tables.go @@ -192,6 +192,8 @@ var NamePriority = map[string]int{ var StripLabelLeadingSlashes = false +var SortListsByDefault = false + var ShortenAbsoluteLabelsToRelative = false // AndroidNativeRules lists all Android rules that are being migrated from Native to Starlark. @@ -271,7 +273,7 @@ var ProtoNativeSymbols = []string{ var ProtoLoadPath = "@rules_proto//proto:defs.bzl" // OverrideTables allows a user of the build package to override the special-case rules. The user-provided tables replace the built-in tables. -func OverrideTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, sortWhitelist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { +func OverrideTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, sortWhitelist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, sortListsByDefault, shortenAbsoluteLabelsToRelative bool) { IsLabelArg = labelArg LabelBlacklist = blacklist IsListArg = listArg @@ -280,11 +282,12 @@ func OverrideTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist SortableWhitelist = sortWhitelist NamePriority = namePriority StripLabelLeadingSlashes = stripLabelLeadingSlashes + SortListsByDefault = sortListsByDefault ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative } // MergeTables allows a user of the build package to override the special-case rules. The user-provided tables are merged into the built-in tables. -func MergeTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, sortWhitelist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { +func MergeTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, sortWhitelist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, sortListsByDefault, shortenAbsoluteLabelsToRelative bool) { for k, v := range labelArg { IsLabelArg[k] = v } @@ -307,5 +310,6 @@ func MergeTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, s NamePriority[k] = v } StripLabelLeadingSlashes = stripLabelLeadingSlashes || StripLabelLeadingSlashes + SortListsByDefault = sortListsByDefault || SortListsByDefault ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative || ShortenAbsoluteLabelsToRelative } diff --git a/tables/testdata/simple_tables.json b/tables/testdata/simple_tables.json index ae72bd02a..dd2dc70db 100644 --- a/tables/testdata/simple_tables.json +++ b/tables/testdata/simple_tables.json @@ -19,5 +19,6 @@ "NamePriority": { "name": -1 }, - "StripLabelLeadingSlashes": true + "StripLabelLeadingSlashes": true, + "SortListsByDefault": true }