Commit c3cb7eec authored by Greg Guterman's avatar Greg Guterman Committed by LUCI CQ

Analyze DEPS autorolls

This new logic tries to figure out which files really changed in a DEPS roll by checking out the old DEPS file and recursively running git diff on all of the repos listed in DEPS. When successful, affected_files turns from just ['DEPS'] into an actual list of affected files. When not successful, it falls back to the original logic.

When DEPS is autorolled, typically the change is just an updated git revision. We don't apply this logic to manual DEPS rolls, as they can contain changes to other variables, whose effects are not easily discernible.

MUST be applied in tandem with https://crrev.com/c/2250928

Bug: 923016
Change-Id: I9e33461ea4ae81dc056d229f8604943745e30389
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2283855
Commit-Queue: Gregory Guterman <guterman@google.com>
Reviewed-by: 's avatarGarrett Beaty <gbeaty@chromium.org>
Reviewed-by: 's avatarEdward Lesmes <ehmaldonado@chromium.org>
parent fad2090a
This diff is collapsed.
DEPS = [ DEPS = [
'git',
'gitiles', 'gitiles',
'recipe_engine/context', 'recipe_engine/context',
'recipe_engine/json', 'recipe_engine/json',
...@@ -6,6 +7,7 @@ DEPS = [ ...@@ -6,6 +7,7 @@ DEPS = [
'recipe_engine/platform', 'recipe_engine/platform',
'recipe_engine/properties', 'recipe_engine/properties',
'recipe_engine/python', 'recipe_engine/python',
'recipe_engine/raw_io',
'recipe_engine/step', 'recipe_engine/step',
'tryserver', 'tryserver',
] ]
...@@ -2,8 +2,11 @@ ...@@ -2,8 +2,11 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import re
from recipe_engine import recipe_api from recipe_engine import recipe_api
class DepsDiffException(Exception):
pass
class RevisionResolver(object): class RevisionResolver(object):
"""Resolves the revision based on build properties.""" """Resolves the revision based on build properties."""
...@@ -358,3 +361,68 @@ class GclientApi(recipe_api.RecipeApi): ...@@ -358,3 +361,68 @@ class GclientApi(recipe_api.RecipeApi):
path, revision = cfg.repo_path_map.get(repo_url, (None, None)) path, revision = cfg.repo_path_map.get(repo_url, (None, None))
if path and revision and path not in cfg.revisions: if path and revision and path not in cfg.revisions:
cfg.revisions[path] = revision cfg.revisions[path] = revision
def diff_deps(self, cwd):
cwd = cwd.join(self.get_gerrit_patch_root())
with self.m.context(cwd=cwd):
step_result = self.m.git(
'-c',
'core.quotePath=false',
'checkout',
'HEAD~',
'--',
'DEPS',
name='checkout the previous DEPS',
stdout=self.m.raw_io.output()
)
try:
cfg = self.c
step_result = self(
'recursively git diff all DEPS',
[
'recurse',
self.resource('diff_deps.py'),
],
stdout=self.m.raw_io.output_text(add_output_log=True),
)
paths = []
# gclient recurse prepends a number and a > to each line
# Let's take that out
for line in step_result.stdout.strip().splitlines():
if 'fatal: bad object' in line:
msg = "Couldn't checkout previous ref: %s" % line
step_result.presentation.logs['DepsDiffException'] = msg
raise self.DepsDiffException(msg)
elif re.match('\d+>', line):
paths.append(line[line.index('>') + 1:])
# Normalize paths
if self.m.platform.is_win:
# Looks like "analyze" wants POSIX slashes even on Windows (since git
# uses that format even on Windows).
paths = [path.replace('\\', '/') for path in paths]
if len(paths) > 0:
return paths
else:
msg = 'Unexpected result: autoroll diff found 0 files changed'
step_result.presentation.logs['DepsDiffException'] = msg
raise self.DepsDiffException(msg)
finally:
self.m.git(
'-c',
'core.quotePath=false',
'checkout',
'HEAD',
'--',
'DEPS',
name="checkout the original DEPS")
@property
def DepsDiffException(self):
return DepsDiffException
#!/usr/bin/env python
import os
import subprocess
import sys
if '@' in os.environ.get('GCLIENT_URL'):
ref = os.environ.get('GCLIENT_URL').split('@')[-1]
else:
sys.exit(0)
diff = subprocess.check_output("git diff --cached --name-only %s" % ref, shell=True)
dep_path = os.environ.get('GCLIENT_DEP_PATH', '')
for line in diff.splitlines():
if line:
print(os.path.join(dep_path, line))
...@@ -7,6 +7,10 @@ import hashlib ...@@ -7,6 +7,10 @@ import hashlib
from recipe_engine import recipe_test_api from recipe_engine import recipe_test_api
class GclientTestApi(recipe_test_api.RecipeTestApi): class GclientTestApi(recipe_test_api.RecipeTestApi):
def diff_deps_test_data(self, files):
return self.m.raw_io.stream_output(
'\n'.join(['10>%s' % fname for fname in files]))
def output_json(self, projects): def output_json(self, projects):
"""Deterministically synthesize json.output test data for gclient's """Deterministically synthesize json.output test data for gclient's
--output-json option. --output-json option.
......
[
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD~",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the previous DEPS"
},
{
"cmd": [
"python",
"-u",
"RECIPE_REPO[depot_tools]/gclient.py",
"recurse",
"RECIPE_MODULE[depot_tools::gclient]/resources/diff_deps.py"
],
"cwd": "[CACHE]/src",
"env_suffixes": {
"PATH": [
"RECIPE_REPO[depot_tools]"
]
},
"infra_step": true,
"name": "gclient recursively git diff all DEPS",
"~followup_annotations": [
"@@@STEP_LOG_LINE@raw_io.output_text@10>third_party/mockfile1@@@",
"@@@STEP_LOG_LINE@raw_io.output_text@10>third_party/mockfile2@@@",
"@@@STEP_LOG_END@raw_io.output_text@@@"
]
},
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the original DEPS"
},
{
"name": "$result"
}
]
\ No newline at end of file
[
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD~",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the previous DEPS"
},
{
"cmd": [
"python",
"-u",
"RECIPE_REPO[depot_tools]/gclient.py",
"recurse",
"RECIPE_MODULE[depot_tools::gclient]/resources/diff_deps.py"
],
"cwd": "[CACHE]/src",
"env_suffixes": {
"PATH": [
"RECIPE_REPO[depot_tools]"
]
},
"infra_step": true,
"name": "gclient recursively git diff all DEPS",
"~followup_annotations": [
"@@@STEP_LOG_LINE@raw_io.output_text@fatal: bad object abcdef1234567890@@@",
"@@@STEP_LOG_END@raw_io.output_text@@@",
"@@@STEP_LOG_LINE@DepsDiffException@Couldn't checkout previous ref: fatal: bad object abcdef1234567890@@@",
"@@@STEP_LOG_END@DepsDiffException@@@"
]
},
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the original DEPS"
},
{
"cmd": [],
"name": "RECIPE CRASH (Uncaught exception)",
"~followup_annotations": [
"@@@STEP_EXCEPTION@@@",
"The recipe has crashed at point 'Uncaught exception'!",
"",
"Traceback (most recent call last):",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/engine.py\", in run_steps",
" raw_result = recipe_obj.run_steps(api, engine)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/recipe_deps.py\", in run_steps",
" properties_def, api=api)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/property_invoker.py\", in invoke_with_properties",
" arg_names, **additional_args)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/property_invoker.py\", in _invoke_with_properties",
" return callable_obj(*props, **additional_args)",
" File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/gclient/tests/diff_deps.py\", line 33, in RunSteps",
" affected_files = api.gclient.diff_deps(api.path['cache'])",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/recipe_api.py\", in _inner",
" return func(*a, **kw)",
" File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/gclient/api.py\", line 398, in diff_deps",
" raise self.DepsDiffException(msg)",
"DepsDiffException: Couldn't checkout previous ref: fatal: bad object abcdef1234567890"
]
},
{
"failure": {
"humanReason": "Uncaught Exception: DepsDiffException(\"Couldn't checkout previous ref: fatal: bad object abcdef1234567890\",)"
},
"name": "$result"
}
]
\ No newline at end of file
[
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD~",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the previous DEPS"
},
{
"cmd": [
"python",
"-u",
"RECIPE_REPO[depot_tools]/gclient.py",
"recurse",
"RECIPE_MODULE[depot_tools::gclient]/resources/diff_deps.py"
],
"cwd": "[CACHE]/src",
"env_suffixes": {
"PATH": [
"RECIPE_REPO[depot_tools]"
]
},
"infra_step": true,
"name": "gclient recursively git diff all DEPS",
"~followup_annotations": [
"@@@STEP_LOG_END@raw_io.output_text@@@",
"@@@STEP_LOG_LINE@DepsDiffException@Unexpected result: autoroll diff found 0 files changed@@@",
"@@@STEP_LOG_END@DepsDiffException@@@"
]
},
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD",
"--",
"DEPS"
],
"cwd": "[CACHE]/src",
"infra_step": true,
"name": "checkout the original DEPS"
},
{
"cmd": [],
"name": "RECIPE CRASH (Uncaught exception)",
"~followup_annotations": [
"@@@STEP_EXCEPTION@@@",
"The recipe has crashed at point 'Uncaught exception'!",
"",
"Traceback (most recent call last):",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/engine.py\", in run_steps",
" raw_result = recipe_obj.run_steps(api, engine)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/recipe_deps.py\", in run_steps",
" properties_def, api=api)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/property_invoker.py\", in invoke_with_properties",
" arg_names, **additional_args)",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/internal/property_invoker.py\", in _invoke_with_properties",
" return callable_obj(*props, **additional_args)",
" File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/gclient/tests/diff_deps.py\", line 33, in RunSteps",
" affected_files = api.gclient.diff_deps(api.path['cache'])",
" File \"RECIPE_REPO[recipe_engine]/recipe_engine/recipe_api.py\", in _inner",
" return func(*a, **kw)",
" File \"RECIPE_REPO[depot_tools]/recipes/recipe_modules/gclient/api.py\", line 414, in diff_deps",
" raise self.DepsDiffException(msg)",
"DepsDiffException: Unexpected result: autoroll diff found 0 files changed"
]
},
{
"failure": {
"humanReason": "Uncaught Exception: DepsDiffException('Unexpected result: autoroll diff found 0 files changed',)"
},
"name": "$result"
}
]
\ No newline at end of file
[
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD~",
"--",
"DEPS"
],
"cwd": "[CACHE]\\src",
"infra_step": true,
"name": "checkout the previous DEPS"
},
{
"cmd": [
"python",
"-u",
"RECIPE_REPO[depot_tools]\\gclient.py",
"recurse",
"RECIPE_MODULE[depot_tools::gclient]\\resources\\diff_deps.py"
],
"cwd": "[CACHE]\\src",
"env_suffixes": {
"PATH": [
"RECIPE_REPO[depot_tools]"
]
},
"infra_step": true,
"name": "gclient recursively git diff all DEPS",
"~followup_annotations": [
"@@@STEP_LOG_LINE@raw_io.output_text@10>third_party/mockfile1@@@",
"@@@STEP_LOG_LINE@raw_io.output_text@10>third_party/mockfile2@@@",
"@@@STEP_LOG_END@raw_io.output_text@@@"
]
},
{
"cmd": [
"git",
"-c",
"core.quotePath=false",
"checkout",
"HEAD",
"--",
"DEPS"
],
"cwd": "[CACHE]\\src",
"infra_step": true,
"name": "checkout the original DEPS"
},
{
"name": "$result"
}
]
\ No newline at end of file
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
from recipe_engine import post_process
DEPS = [
'recipe_engine/assertions',
'recipe_engine/buildbucket',
'recipe_engine/path',
'recipe_engine/platform',
'recipe_engine/properties',
'recipe_engine/raw_io',
'gclient',
]
def RunSteps(api):
src_cfg = api.gclient.make_config(CACHE_DIR=api.path['cache'].join('git'))
soln = src_cfg.solutions.add()
soln.name = 'src'
soln.url = 'https://chromium.googlesource.com/chromium/src.git'
src_cfg.repo_path_map.update({
'https://chromium.googlesource.com/src': ('src', 'HEAD'),
'https://chromium.googlesource.com/v8/v8': ('src/v8', 'HEAD'),
# non-canonical URL
'https://webrtc.googlesource.com/src.git': (
'src/third_party/webrtc', 'HEAD'),
})
api.gclient.c = src_cfg
affected_files = api.gclient.diff_deps(api.path['cache'])
api.assertions.assertEqual(
affected_files,
list(api.properties.get('diff_deps_files')),
)
def GenTests(api):
test_files = (
'third_party/mockfile1',
'third_party/mockfile2'
)
no_test_files = []
yield api.test(
'basic',
api.buildbucket.try_build(),
api.properties(diff_deps_files=test_files),
api.override_step_data(
'gclient recursively git diff all DEPS',
api.gclient.diff_deps_test_data(test_files),
),
api.post_process(post_process.StatusSuccess),
)
yield api.test(
'no change, exception',
api.buildbucket.try_build(),
api.properties(diff_deps_files=no_test_files),
api.override_step_data(
'gclient recursively git diff all DEPS',
api.gclient.diff_deps_test_data(no_test_files),
),
api.expect_exception('DepsDiffException')
)
yield api.test(
'dont have revision yet',
api.buildbucket.try_build(),
api.properties(diff_deps_files=test_files),
api.override_step_data(
'gclient recursively git diff all DEPS',
api.raw_io.stream_output('fatal: bad object abcdef1234567890'),
),
api.expect_exception('DepsDiffException')
)
yield api.test(
'windows',
api.buildbucket.try_build(),
api.properties(diff_deps_files=test_files),
api.platform.name('win'),
api.override_step_data(
'gclient recursively git diff all DEPS',
api.gclient.diff_deps_test_data(test_files),
),
api.post_process(post_process.StatusSuccess),
)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment