From d9108d2d343117278ca647f8abe3e40fccb7119d Mon Sep 17 00:00:00 2001 From: Yaowen Mei Date: Thu, 29 Aug 2024 16:56:32 +0000 Subject: [PATCH] Fix a bug in the log directory cleanup logic. This CL fix the bug that shouldCleanupRBELogsDir() never return true. The way how shouldCleanupRBELogsDir() use to work is it will return true only if `RBE_proxy_log_dir` flag is not set. But CI build always got this flag from gcl file, and developer build always got this flag set from ui/build/rbe.go (http://shortn/_K604iWNYkd). So there is never a case this shouldCleanupRBELogsDir() return true. That is to say, we never clean up RBELogsDir by ourselves. The reason why this is not a concern is that soong will automatically delete the out/soong/.temp folder every time when user run `m` or `lunch`, and the default RBELogsDir use to be `out/soong/.temp/rbe`. So soong is helping us to clean the logs before. Previously, I merged this CL: https://r.android.com/3192211 to support running `lunch` in two terminals. In that CL, I moved the auto created RBELogsDir one level up to `out/soong/rbe`. This is causing a problem because RBELogsDir never get cleaned. This CL will fix the shouldCleanupRBELogsDir() method, after this merged in, 1) if a user didn't set the `RBE_proxy_log_dir` flag, the logs in `out/soong/rbe` will be cleaned each time when running `m`, but `lunch` will not touch `out/soong/rbe`, so we still support running `lunch` in different terminals. 2) If user set `RBE_proxy_log_dir` flag to anything rather than `out/song/rbe`, the directory will not be cleaned by running `m`; 3) we have updated our doc everywhere that `out/soong/rbe` is re-client's default log dir, so I think it should be expected if user set `RBE_proxy_log_dir=out/soong/rbe`, then the logs will be deleted when next time `m` invoked, same as if they set `RBE_proxy_log_dir=out/soong/.temp/rbe` before. Test: abtd run is green: http://shortn/_KqTvQelstP, local build with multiply lunch is not affected, and running m after a build can clean the out/soong/rbe directory. Bug: 362798954 Change-Id: I38a7ad650fc59ad06716c5be7de6ecc61ead8eef --- ui/build/config.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/build/config.go b/ui/build/config.go index f02222e1a..d72c8fa01 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -1375,8 +1375,10 @@ func (c *configImpl) shouldCleanupRBELogsDir() bool { // Perform a log directory cleanup only when the log directory // is auto created by the build rather than user-specified. for _, f := range []string{"RBE_proxy_log_dir", "FLAG_output_dir"} { - if _, ok := c.environ.Get(f); ok { - return false + if v, ok := c.environ.Get(f); ok { + if v != c.rbeTmpDir() { + return false + } } } return true