Skip to content

Commit

Permalink
Remove gn from buildtools/DEPS
Browse files Browse the repository at this point in the history
Projects will have to add entries for gn to their toplevel DEPS instead.

Add this to your `vars` section:

  'gn_version': 'git_revision:dfcbc6fed0a8352696f92d67ccad54048ad182b3',

And this to your `deps` section:

  'src/buildtools/linux64': {
    'packages': [
      {
        'package': 'gn/gn/linux-amd64',
        'version': Var('gn_version'),
      }
    ],
    'dep_type': 'cipd',
    'condition': 'host_os == "linux"',
  },
  'src/buildtools/mac': {
    'packages': [
      {
        'package': 'gn/gn/mac-${{arch}}',
        'version': Var('gn_version'),
      }
    ],
    'dep_type': 'cipd',
    'condition': 'host_os == "mac"',
  },
  'src/buildtools/win': {
    'packages': [
      {
        'package': 'gn/gn/windows-amd64',
        'version': Var('gn_version'),
      }
    ],
    'dep_type': 'cipd',
    'condition': 'host_os == "win"',
  },

Adopt to your project's style as you like.
(Maybe your toplevel directory is not called "src".)

angle and v8 for example do this already for example.

It looks like there's no way to have autoroller update this via transitive_dep,
and it looks like there's no way to have an autoroller for this at the chromium
level yet, see https://bugs.chromium.org/p/skia/issues/detail?id=11712
But it's the last thing keeping buildtools/DEPS around, and several projects
already don't use this hook, so let's remove it.

Bug: 1177288
Change-Id: I3d89361fcc95eebe91fd38ce854ce629f4117e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2743918
Commit-Queue: Nico Weber <thakis@chromium.org>
Auto-Submit: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#861001}
  • Loading branch information
nico authored and Chromium LUCI CQ committed Mar 9, 2021
1 parent 77b3cf8 commit 1e04918
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 127 deletions.
40 changes: 0 additions & 40 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -4130,46 +4130,6 @@ def CheckCorrectProductNameInMessages(input_api, output_api):
return all_problems


def CheckBuildtoolsRevisionsAreInSync(input_api, output_api):
# TODO(crbug.com/941824): We need to make sure the entries in
# //buildtools/DEPS are kept in sync with the entries in //DEPS
# so that users of //buildtools in other projects get the same tooling
# Chromium gets. If we ever fix the referenced bug and add 'includedeps'
# support to gclient, we can eliminate the duplication and delete
# this presubmit check.

# Update this regexp if new revisions are added to the files.
rev_regexp = input_api.re.compile("'gn_version':")

# If a user is changing one revision, they need to change the same
# line in both files. This means that any given change should contain
# exactly the same list of changed lines that match the regexps. The
# replace(' ', '') call allows us to ignore whitespace changes to the
# lines. The 'long_text' parameter to the error will contain the
# list of changed lines in both files, which should make it easy enough
# to spot the error without going overboard in this implementation.
buildtools_deps = input_api.os_path.join('buildtools', 'DEPS')
revs_changes = {
'DEPS': {},
buildtools_deps: {},
}
long_text = ''

for f in input_api.AffectedFiles(
file_filter=lambda f: f.LocalPath() in ('DEPS', buildtools_deps)):
for line_num, line in f.ChangedContents():
if rev_regexp.search(line):
revs_changes[f.LocalPath()][line.replace(' ', '')] = line
long_text += '%s:%d: %s\n' % (f.LocalPath(), line_num, line)

if set(revs_changes['DEPS']) != set(revs_changes[buildtools_deps]):
return [output_api.PresubmitError(
'Change buildtools revisions in sync in both DEPS and ' +
buildtools_deps + '.', long_text=long_text + '\n')]
else:
return []


def CheckForTooLargeFiles(input_api, output_api):
"""Avoid large files, especially binary files, in the repository since
git doesn't scale well for those. They will be in everyone's repo
Expand Down
43 changes: 0 additions & 43 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3481,49 +3481,6 @@ def testIngoreDeletedFiles(self):
self.assertEqual(0, len(results))


class BuildtoolsRevisionsAreInSyncTest(unittest.TestCase):
# TODO(crbug.com/941824): We need to make sure the entries in
# //buildtools/DEPS are kept in sync with the entries in //DEPS
# so that users of //buildtools in other projects get the same tooling
# Chromium gets. If we ever fix the referenced bug and add 'includedeps'
# support to gclient, we can eliminate the duplication and delete
# these tests for the corresponding presubmit check.

def _check(self, files):
mock_input_api = MockInputApi()
mock_input_api.files = []
for fname, contents in files.items():
mock_input_api.files.append(MockFile(fname, contents.splitlines()))
return PRESUBMIT.CheckBuildtoolsRevisionsAreInSync(mock_input_api,
MockOutputApi())

def testOneFileChangedButNotTheOther(self):
results = self._check({
"DEPS": "'gn_version': 'onerev'",
})
self.assertNotEqual(results, [])

def testNeitherFileChanged(self):
results = self._check({
"OWNERS": "foobar@example.com",
})
self.assertEqual(results, [])

def testBothFilesChangedAndMatch(self):
results = self._check({
"DEPS": "'gn_version': 'onerev'",
os.path.join("buildtools", "DEPS"): "'gn_version': 'onerev'",
})
self.assertEqual(results, [])

def testBothFilesWereChangedAndDontMatch(self):
results = self._check({
"DEPS": "'gn_version': 'rev1'",
os.path.join("buildtools", "DEPS"): "'gn_version': 'rev2'",
})
self.assertNotEqual(results, [])


class CheckFuzzTargetsTest(unittest.TestCase):

def _check(self, files):
Expand Down
46 changes: 2 additions & 44 deletions buildtools/DEPS
Original file line number Diff line number Diff line change
@@ -1,51 +1,9 @@
# TODO(crbug.com/1177288): Remove this file. Please don't add to it.

use_relative_paths = True

vars = {
'chromium_url': 'https://chromium.googlesource.com',

# TODO(crbug.com/1177288): Remove this file. Please don't add to it.

# TODO(crbug.com/941824): The values below need to be kept in sync
# between //DEPS and //buildtools/DEPS, so if you're updating one,
# update the other. There is a presubmit check that checks that
# you've done so; if you are adding new tools to //buildtools and
# hence new revisions to this list, make sure you update the
# _CheckBuildtoolsRevsAreInSync in PRESUBMIT.py to include the additional
# revisions.

# GN CIPD package version.
'gn_version': 'git_revision:dfcbc6fed0a8352696f92d67ccad54048ad182b3',
}

deps = {
'linux64': {
'packages': [
{
'package': 'gn/gn/linux-amd64',
'version': Var('gn_version'),
}
],
'dep_type': 'cipd',
'condition': 'host_os == "linux"',
},
'mac': {
'packages': [
{
'package': 'gn/gn/mac-amd64',
'version': Var('gn_version'),
}
],
'dep_type': 'cipd',
'condition': 'host_os == "mac"',
},
'win': {
'packages': [
{
'package': 'gn/gn/windows-amd64',
'version': Var('gn_version'),
}
],
'dep_type': 'cipd',
'condition': 'host_os == "win"',
},
}

0 comments on commit 1e04918

Please sign in to comment.