diff --git a/ui/build/config.go b/ui/build/config.go index 5cd5c658d..7837cc4e0 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -83,6 +83,8 @@ type configImpl struct { // Set by multiproduct_kati emptyNinjaFile bool + + metricsUploader string } const srcDirFileCheck = "build/soong/root.bp" @@ -237,7 +239,8 @@ func NewConfig(ctx Context, args ...string) Config { // Precondition: the current directory is the top of the source tree checkTopDir(ctx) - if srcDir := absPath(ctx, "."); strings.ContainsRune(srcDir, ' ') { + srcDir := absPath(ctx, ".") + if strings.ContainsRune(srcDir, ' ') { ctx.Println("You are building in a directory whose absolute path contains a space character:") ctx.Println() ctx.Printf("%q\n", srcDir) @@ -245,6 +248,8 @@ func NewConfig(ctx Context, args ...string) Config { ctx.Fatalln("Directory names containing spaces are not supported") } + ret.metricsUploader = GetMetricsUploader(srcDir, ret.environ) + if outDir := ret.OutDir(); strings.ContainsRune(outDir, ' ') { ctx.Println("The absolute path of your output directory ($OUT_DIR) contains a space character:") ctx.Println() @@ -1246,10 +1251,7 @@ func (c *configImpl) BuildDateTime() string { } func (c *configImpl) MetricsUploaderApp() string { - if p, ok := c.environ.Get("ANDROID_ENABLE_METRICS_UPLOAD"); ok { - return p - } - return "" + return c.metricsUploader } // LogsDir returns the logs directory where build log and metrics @@ -1277,3 +1279,14 @@ func (c *configImpl) SetEmptyNinjaFile(v bool) { func (c *configImpl) EmptyNinjaFile() bool { return c.emptyNinjaFile } + +func GetMetricsUploader(topDir string, env *Environment) string { + if p, ok := env.Get("METRICS_UPLOADER"); ok { + metricsUploader := filepath.Join(topDir, p) + if _, err := os.Stat(metricsUploader); err == nil { + return metricsUploader + } + } + + return "" +} diff --git a/ui/build/config_test.go b/ui/build/config_test.go index 03d304e43..e29327572 100644 --- a/ui/build/config_test.go +++ b/ui/build/config_test.go @@ -1122,3 +1122,65 @@ func TestBuildConfig(t *testing.T) { }) } } + +func TestGetMetricsUploaderApp(t *testing.T) { + + metricsUploaderDir := "metrics_uploader_dir" + metricsUploaderBinary := "metrics_uploader_binary" + metricsUploaderPath := filepath.Join(metricsUploaderDir, metricsUploaderBinary) + tests := []struct { + description string + environ Environment + createFiles bool + expected string + }{{ + description: "Uploader binary exist", + environ: Environment{"METRICS_UPLOADER=" + metricsUploaderPath}, + createFiles: true, + expected: metricsUploaderPath, + }, { + description: "Uploader binary not exist", + environ: Environment{"METRICS_UPLOADER=" + metricsUploaderPath}, + createFiles: false, + expected: "", + }, { + description: "Uploader binary variable not set", + createFiles: true, + expected: "", + }} + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + defer logger.Recover(func(err error) { + t.Fatalf("got unexpected error: %v", err) + }) + + // Create the root source tree. + topDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(topDir) + + expected := tt.expected + if len(expected) > 0 { + expected = filepath.Join(topDir, expected) + } + + if tt.createFiles { + if err := os.MkdirAll(filepath.Join(topDir, metricsUploaderDir), 0755); err != nil { + t.Errorf("failed to create %s directory: %v", metricsUploaderDir, err) + } + if err := ioutil.WriteFile(filepath.Join(topDir, metricsUploaderPath), []byte{}, 0644); err != nil { + t.Errorf("failed to create file %s: %v", expected, err) + } + } + + actual := GetMetricsUploader(topDir, &tt.environ) + + if actual != expected { + t.Errorf("expecting: %s, actual: %s", expected, actual) + } + }) + } +} diff --git a/ui/build/upload.go b/ui/build/upload.go index 55ada335b..687f51977 100644 --- a/ui/build/upload.go +++ b/ui/build/upload.go @@ -70,12 +70,11 @@ func pruneMetricsFiles(paths []string) []string { return metricsFiles } -// UploadMetrics uploads a set of metrics files to a server for analysis. An -// uploader full path is specified in ANDROID_ENABLE_METRICS_UPLOAD environment -// variable in order to upload the set of metrics files. The metrics files are -// first copied to a temporary directory and the uploader is then executed in -// the background to allow the user/system to continue working. Soong communicates -// to the uploader through the upload_proto raw protobuf file. +// UploadMetrics uploads a set of metrics files to a server for analysis. +// The metrics files are first copied to a temporary directory +// and the uploader is then executed in the background to allow the user/system +// to continue working. Soong communicates to the uploader through the +// upload_proto raw protobuf file. func UploadMetrics(ctx Context, config Config, simpleOutput bool, buildStarted time.Time, paths ...string) { ctx.BeginTrace(metrics.RunSetupTool, "upload_metrics") defer ctx.EndTrace() diff --git a/ui/build/upload_test.go b/ui/build/upload_test.go index b740c1120..764a1e127 100644 --- a/ui/build/upload_test.go +++ b/ui/build/upload_test.go @@ -80,13 +80,10 @@ func TestUploadMetrics(t *testing.T) { createFiles bool files []string }{{ - description: "ANDROID_ENABLE_METRICS_UPLOAD not set", - }, { - description: "no metrics files to upload", - uploader: "fake", + description: "no metrics uploader", }, { description: "non-existent metrics files no upload", - uploader: "fake", + uploader: "echo", files: []string{"metrics_file_1", "metrics_file_2", "metrics_file_3"}, }, { description: "trigger upload", @@ -137,9 +134,9 @@ func TestUploadMetrics(t *testing.T) { config := Config{&configImpl{ environ: &Environment{ "OUT_DIR=" + outDir, - "ANDROID_ENABLE_METRICS_UPLOAD=" + tt.uploader, }, - buildDateTime: strconv.FormatInt(time.Now().UnixNano()/int64(time.Millisecond), 10), + buildDateTime: strconv.FormatInt(time.Now().UnixNano()/int64(time.Millisecond), 10), + metricsUploader: tt.uploader, }} UploadMetrics(ctx, config, false, time.Now(), metricsFiles...) @@ -192,9 +189,10 @@ func TestUploadMetricsErrors(t *testing.T) { config := Config{&configImpl{ environ: &Environment{ - "ANDROID_ENABLE_METRICS_UPLOAD=fake", "OUT_DIR=/bad", - }}} + }, + metricsUploader: "echo", + }} UploadMetrics(ctx, config, true, time.Now(), metricsFile) t.Errorf("got nil, expecting %q as a failure", tt.expectedErr) diff --git a/ui/metrics/metrics.go b/ui/metrics/metrics.go index ccf9bd8c4..f1bb862bd 100644 --- a/ui/metrics/metrics.go +++ b/ui/metrics/metrics.go @@ -25,16 +25,11 @@ package metrics // that captures the metrics and is them added as a perfInfo into the set // of the collected metrics. Finally, when soong_ui has finished the build, // the defer Dump function is invoked to store the collected metrics to the -// raw protobuf file in the $OUT directory. -// -// There is one additional step that occurs after the raw protobuf file is written. -// If the configuration environment variable ANDROID_ENABLE_METRICS_UPLOAD is -// set with the path, the raw protobuf file is uploaded to the destination. See -// ui/build/upload.go for more details. The filename of the raw protobuf file -// and the list of files to be uploaded is defined in cmd/soong_ui/main.go. -// -// See ui/metrics/event.go for the explanation of what an event is and how -// the metrics system is a stack based system. +// raw protobuf file in the $OUT directory and this raw protobuf file will be +// uploaded to the destination. See ui/build/upload.go for more details. The +// filename of the raw protobuf file and the list of files to be uploaded is +// defined in cmd/soong_ui/main.go. See ui/metrics/event.go for the explanation +// of what an event is and how the metrics system is a stack based system. import ( "io/ioutil"