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

Use flags-as-proto to determine command line schema #8057

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
* text=auto

*.b64 binary
Comment on lines +1 to +3
Copy link
Member

Choose a reason for hiding this comment

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

The b64 file is a text-format (ASCII) representation of binary content, so I think this change should be reverted

Copy link
Contributor Author

@tempoz tempoz Dec 19, 2024

Choose a reason for hiding this comment

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

This setting tells git not to do cleanup of new lines on the file (adding a terminating new line, doing CRLF conversion) or to try to display diffs of it as if it is a normal file (which it can't do effectively, as it's all on one line anyway).

See the Identifying Binary Files section of https://git-scm.com/book/ms/v2/Customizing-Git-Git-Attributes , which states:

Some files look like text files but for all intents and purposes are to be treated as binary data.

and then goes on to describe an example of a text file that is not mergeable and where diffs aren't "generally helpful", advising that in these cases the files in question should be labeled binary in .gitattributes

It later describes how to get git to display diffs of "binary" files that have been converted to human-readable text files, which we could do, but that requires a git config setting, which would have to be set for every user who clones the repo, so it's not really practical.

2 changes: 2 additions & 0 deletions cli/parser/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ go_library(
"//cli/log",
"//cli/storage",
"//cli/workspace",
"//proto:bazel_flags_go_proto",
"//proto:remote_execution_go_proto",
"//server/remote_cache/digest",
"//server/util/disk",
"//server/util/proto",
"@com_github_google_shlex//:shlex",
],
)
Expand Down
82 changes: 73 additions & 9 deletions cli/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package parser
import (
"bufio"
"bytes"
"encoding/base64"
"fmt"
"io"
"os"
Expand All @@ -21,8 +22,10 @@ import (
"github.com/buildbuddy-io/buildbuddy/cli/workspace"
"github.com/buildbuddy-io/buildbuddy/server/remote_cache/digest"
"github.com/buildbuddy-io/buildbuddy/server/util/disk"
"github.com/buildbuddy-io/buildbuddy/server/util/proto"
"github.com/google/shlex"

bfpb "github.com/buildbuddy-io/buildbuddy/proto/bazel_flags"
repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
)

Expand Down Expand Up @@ -350,15 +353,72 @@ func (s *CommandLineSchema) CommandSupportsOpt(opt string) bool {
return false
}

func GetOptionSetsfromProto(args []string, flagCollection *bfpb.FlagCollection, onlyStartupOptions bool) (map[string]*OptionSet, error) {
sets := make(map[string]*OptionSet)
for _, info := range flagCollection.FlagInfos {
if info.GetName() == "bazelrc" {
// `bazel help flags-as-proto` incorrectly reports `bazelrc` as not
// allowing multiple values.
// See https://github.com/bazelbuild/bazel/issues/24730 for more info.
*info.AllowsMultiple = true
}
o := &Option{
Name: info.GetName(),
ShortName: info.GetAbbreviation(),
Multi: info.GetAllowsMultiple(),
BoolLike: info.GetHasNegativeFlag(),
}
for _, cmd := range info.GetCommands() {
var set *OptionSet
var ok bool
if set, ok = sets[cmd]; !ok {
set = &OptionSet{
All: []*Option{},
ByName: make(map[string]*Option),
ByShortName: make(map[string]*Option),
}
sets[cmd] = set
}
set.All = append(set.All, o)
set.ByName[o.Name] = o
if o.ShortName != "" {
set.ByShortName[o.ShortName] = o
}
}
}
return sets, nil
}

// GetCommandLineSchema returns the effective CommandLineSchemas for the given
// command line.
func getCommandLineSchema(args []string, bazelHelp BazelHelpFunc, onlyStartupOptions bool) (*CommandLineSchema, error) {
startupHelp, err := bazelHelp("startup_options")
if err != nil {
return nil, err
var optionSets map[string]*OptionSet
// try flags-as-proto first; fall back to parsing help if bazel version does not support it.
if protoHelp, err := bazelHelp("flags-as-proto"); err == nil {
protoHelp = strings.TrimSpace(protoHelp)
b, err := base64.StdEncoding.DecodeString(protoHelp)
if err != nil {
return nil, err
}
flagCollection := &bfpb.FlagCollection{}
if err := proto.Unmarshal(b, flagCollection); err != nil {
return nil, err
}
sets, err := GetOptionSetsfromProto(args, flagCollection, onlyStartupOptions)
if err != nil {
return nil, err
}
optionSets = sets
}
schema := &CommandLineSchema{
StartupOptions: parseBazelHelp(startupHelp, "startup_options"),
schema := &CommandLineSchema{}
if startupOptions, ok := optionSets["startup"]; ok {
schema.StartupOptions = startupOptions
} else {
startupHelp, err := bazelHelp("startup_options")
if err != nil {
return nil, err
}
schema.StartupOptions = parseBazelHelp(startupHelp, "startup_options")
}
bazelCommands, err := BazelCommands()
if err != nil {
Expand Down Expand Up @@ -394,11 +454,15 @@ func getCommandLineSchema(args []string, bazelHelp BazelHelpFunc, onlyStartupOpt
if schema.Command == "" {
return schema, nil
}
commandHelp, err := bazelHelp(schema.Command)
if err != nil {
return nil, err
if commandOptions, ok := optionSets[schema.Command]; ok {
schema.CommandOptions = commandOptions
} else {
commandHelp, err := bazelHelp(schema.Command)
if err != nil {
return nil, err
}
schema.CommandOptions = parseBazelHelp(commandHelp, schema.Command)
}
schema.CommandOptions = parseBazelHelp(commandHelp, schema.Command)
return schema, nil
}

Expand Down
Loading
Loading