pw_presubmit: Allow separate log directory
Allow for the log directory to be separate from the export directory.
This allows CIPD packages being built by pw_presubmit to exclude various
GN/ninja logs.
Change-Id: I03004ce2b602734ca1f9b0d0bc290e7a6317b825
Reviewed-on: https://pigweed-review.googlesource.com/c/infra/recipes/+/69864
Reviewed-by: Oliver Newman <olivernewman@google.com>
Commit-Queue: Rob Mohr <mohrr@google.com>
diff --git a/recipe_modules/pw_presubmit/api.py b/recipe_modules/pw_presubmit/api.py
index 00648b0..8d686ec 100644
--- a/recipe_modules/pw_presubmit/api.py
+++ b/recipe_modules/pw_presubmit/api.py
@@ -161,7 +161,7 @@
return self.m.step(name, cmd, timeout=self._step_timeout(), **kwargs)
- def run(self, step):
+ def run(self, step, log_dir=None):
with self.m.step.nest(step.name) as pres:
args = []
@@ -172,9 +172,15 @@
self._run(args, name=step.name)
+ if log_dir:
+ step_log_dir = log_dir.join(step.name)
+ else:
+ log_dir = step.export_dir
+
if step.export_dir:
self.m.file.ensure_directory(
- 'mkdir {}'.format(self.export_dir_name), step.export_dir,
+ 'mkdir {}'.format(self.export_dir_name), step.export_dir
)
-
- self.m.build.save_logs(step.dir, step.export_dir)
+ if log_dir and log_dir != step.export_dir:
+ self.m.file.ensure_directory('create log dir', log_dir)
+ self.m.build.save_logs(step.dir, log_dir)
diff --git a/recipe_modules/pw_presubmit/tests/full.expected/pigweed.json b/recipe_modules/pw_presubmit/tests/full.expected/pigweed.json
index 4f49c10..fc0c280 100644
--- a/recipe_modules/pw_presubmit/tests/full.expected/pigweed.json
+++ b/recipe_modules/pw_presubmit/tests/full.expected/pigweed.json
@@ -284,6 +284,36 @@
"RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
"--json-output",
"/path/to/tmp/json",
+ "ensure-directory",
+ "--mode",
+ "0777",
+ "[START_DIR]/logs/full_1"
+ ],
+ "infra_step": true,
+ "luci_context": {
+ "realm": {
+ "name": "project:ci"
+ },
+ "resultdb": {
+ "current_invocation": {
+ "name": "invocations/build:8945511751514863184",
+ "update_token": "token"
+ },
+ "hostname": "rdbhost"
+ }
+ },
+ "name": "full_1.create log dir",
+ "~followup_annotations": [
+ "@@@STEP_NEST_LEVEL@1@@@"
+ ]
+ },
+ {
+ "cmd": [
+ "vpython",
+ "-u",
+ "RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
+ "--json-output",
+ "/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/full_1/ninja.log",
"/path/to/tmp/"
@@ -359,7 +389,7 @@
"/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/full_1/ninja.log",
- "[START_DIR]/presubmit/full_1/export/ninja.log"
+ "[START_DIR]/logs/full_1/ninja.log"
],
"infra_step": true,
"luci_context": {
diff --git a/recipe_modules/pw_presubmit/tests/full.expected/step.json b/recipe_modules/pw_presubmit/tests/full.expected/step.json
index fef6a49..651bd89 100644
--- a/recipe_modules/pw_presubmit/tests/full.expected/step.json
+++ b/recipe_modules/pw_presubmit/tests/full.expected/step.json
@@ -246,6 +246,36 @@
"RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
"--json-output",
"/path/to/tmp/json",
+ "ensure-directory",
+ "--mode",
+ "0777",
+ "[START_DIR]/logs/step2"
+ ],
+ "infra_step": true,
+ "luci_context": {
+ "realm": {
+ "name": "project:try"
+ },
+ "resultdb": {
+ "current_invocation": {
+ "name": "invocations/build:8945511751514863184",
+ "update_token": "token"
+ },
+ "hostname": "rdbhost"
+ }
+ },
+ "name": "step2.create log dir",
+ "~followup_annotations": [
+ "@@@STEP_NEST_LEVEL@1@@@"
+ ]
+ },
+ {
+ "cmd": [
+ "vpython",
+ "-u",
+ "RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
+ "--json-output",
+ "/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/step2/ninja.log",
"/path/to/tmp/"
@@ -321,7 +351,7 @@
"/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/step2/ninja.log",
- "[START_DIR]/presubmit/step2/export/ninja.log"
+ "[START_DIR]/logs/step2/ninja.log"
],
"infra_step": true,
"luci_context": {
diff --git a/recipe_modules/pw_presubmit/tests/full.py b/recipe_modules/pw_presubmit/tests/full.py
index c306fb0..b062758 100644
--- a/recipe_modules/pw_presubmit/tests/full.py
+++ b/recipe_modules/pw_presubmit/tests/full.py
@@ -33,8 +33,14 @@
def RunSteps(api): # pylint: disable=invalid-name
api.pw_presubmit.init(api.path['start_dir'].join('checkout'))
- for step in api.pw_presubmit.steps():
- api.pw_presubmit.run(step)
+ log_dir = api.path['start_dir'].join('logs')
+
+ for i, step in enumerate(api.pw_presubmit.steps()):
+ # Need to take path with and without log_dir argument for coverage.
+ kw = {}
+ if i:
+ kw['log_dir'] = log_dir.join(step.name)
+ api.pw_presubmit.run(step, **kw)
# For coverage.
_ = api.pw_presubmit.has_props()
diff --git a/recipes/target_to_cipd.expected/pw-presubmit.json b/recipes/target_to_cipd.expected/pw-presubmit.json
index 2e28101..33a1aed 100644
--- a/recipes/target_to_cipd.expected/pw-presubmit.json
+++ b/recipes/target_to_cipd.expected/pw-presubmit.json
@@ -773,6 +773,37 @@
"RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
"--json-output",
"/path/to/tmp/json",
+ "ensure-directory",
+ "--mode",
+ "0777",
+ "[START_DIR]/logs"
+ ],
+ "env": {
+ "BUILDBUCKET_ID": "0",
+ "BUILDBUCKET_NAME": "project:bucket:builder",
+ "BUILD_NUMBER": "0",
+ "GOCACHE": "[CACHE]/go",
+ "PIP_CACHE_DIR": "[CACHE]/pip",
+ "PW_ENVIRONMENT_NO_ERROR_ON_UNRECOGNIZED": "1",
+ "PW_ENVSETUP_DISABLE_SPINNER": "1",
+ "PW_PRESUBMIT_DISABLE_SUBPROCESS_CAPTURE": "1",
+ "PW_PROJECT_ROOT": "[START_DIR]/checkout",
+ "PW_ROOT": "[START_DIR]/checkout",
+ "TEST_TMPDIR": "[CACHE]/bazel"
+ },
+ "infra_step": true,
+ "name": "step.create log dir",
+ "~followup_annotations": [
+ "@@@STEP_NEST_LEVEL@1@@@"
+ ]
+ },
+ {
+ "cmd": [
+ "vpython",
+ "-u",
+ "RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
+ "--json-output",
+ "/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/step/ninja.log",
"/path/to/tmp/"
@@ -849,7 +880,7 @@
"/path/to/tmp/json",
"copy",
"[START_DIR]/presubmit/step/ninja.log",
- "[START_DIR]/presubmit/step/export/ninja.log"
+ "[START_DIR]/logs/ninja.log"
],
"env": {
"BUILDBUCKET_ID": "0",
diff --git a/recipes/target_to_cipd.py b/recipes/target_to_cipd.py
index 4299681..0688816 100644
--- a/recipes/target_to_cipd.py
+++ b/recipes/target_to_cipd.py
@@ -56,7 +56,8 @@
# values instead of using steps[0].
for step in steps:
assert step.export_dir
- api.pw_presubmit.run(step)
+ log_dir = api.path['start_dir'].join('logs')
+ api.pw_presubmit.run(step, log_dir=log_dir)
build_dir = step.dir
break # Not required but makes flow clearer at a glance.
