pre-upload: Check for invalid USE-setting in make.*.
Check that $USE isn't overwritten or referenced before it's
been defined in Portage make.conf and make.defaults files.
Also update pre-upload*.py to pass existing checks.
BUG=chromium:453966
TEST=added unit tests; also manually tested with broken and
non-broken make.conf and make.defaults files
Change-Id: I77a6b52ba966c72ae6933f30616c0396dd856912
Reviewed-on: https://chromium-review.googlesource.com/251131
Commit-Queue: Dan Erat <derat@chromium.org>
Trybot-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/errors.py b/errors.py
index 1a0f7dc..95f5e72 100644
--- a/errors.py
+++ b/errors.py
@@ -20,6 +20,9 @@
self.msg = msg
self.items = items
+ def __str__(self):
+ return _FormatHookFailure(self)
+
_INDENT = ' ' * 4
_PROJECT_INFO = 'Errors in PROJECT *%s*!'
diff --git a/pre-upload.py b/pre-upload.py
index 5279da5..d858eaa 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/python2
# Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
@@ -721,6 +721,46 @@
for x in bad_ebuilds]), url))
+def _check_portage_make_use_var(_project, commit):
+ """Verify that $USE is set correctly in make.conf and make.defaults."""
+ files = _filter_files(_get_affected_files(commit, relative=True),
+ [r'(^|/)make.(conf|defaults)$'])
+
+ errors = []
+ for path in files:
+ basename = os.path.basename(path)
+
+ # Has a USE= line already been encountered in this file?
+ saw_use = False
+
+ for i, line in enumerate(_get_file_content(path, commit).splitlines(), 1):
+ if not line.startswith('USE='):
+ continue
+
+ preserves_use = '${USE}' in line or '$USE' in line
+
+ if (basename == 'make.conf' or
+ (basename == 'make.defaults' and saw_use)) and not preserves_use:
+ errors.append('%s:%d: missing ${USE}' % (path, i))
+ elif basename == 'make.defaults' and not saw_use and preserves_use:
+ errors.append('%s:%d: ${USE} referenced in initial declaration' %
+ (path, i))
+
+ saw_use = True
+
+ if errors:
+ return HookFailure(
+ 'One or more Portage make files appear to set USE incorrectly.\n'
+ '\n'
+ 'All USE assignments in make.conf and all assignments after the\n'
+ 'initial declaration in make.defaults should contain "${USE}" to\n'
+ 'preserve previously-set flags.\n'
+ '\n'
+ 'The initial USE declaration in make.defaults should not contain\n'
+ '"${USE}".\n',
+ errors)
+
+
def _check_change_has_proper_changeid(_project, commit):
"""Verify that Change-ID is present in last paragraph of commit message."""
CHANGE_ID_RE = r'\nChange-Id: I[a-f0-9]+\n'
@@ -1070,12 +1110,13 @@
_check_ebuild_keywords,
_check_ebuild_licenses,
_check_ebuild_virtual_pv,
- _check_no_stray_whitespace,
- _check_no_long_lines,
+ _check_for_uprev,
_check_layout_conf,
_check_license,
+ _check_no_long_lines,
+ _check_no_stray_whitespace,
_check_no_tabs,
- _check_for_uprev,
+ _check_portage_make_use_var,
]
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 9bdcfa9..c0b4ea4 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/python2
# -*- coding: utf-8 -*-
# Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
@@ -21,7 +21,6 @@
'..', '..'))
from chromite.cbuildbot import constants
-from chromite.lib import cros_build_lib
from chromite.lib import cros_test_lib
from chromite.lib import git
from chromite.lib import osutils
@@ -39,7 +38,9 @@
def runTest(self):
self.assertEquals(u'', pre_upload._try_utf8_decode(''))
self.assertEquals(u'abc', pre_upload._try_utf8_decode('abc'))
- self.assertEquals(u'ä½ å¥½å¸èæ©', pre_upload._try_utf8_decode('ä½ å¥½å¸èæ©'))
+ self.assertEquals(
+ u'ä½ å¥½å¸èæ©',
+ pre_upload._try_utf8_decode('ä½ å¥½å¸èæ©'))
# Invalid UTF-8
self.assertEquals('\x80', pre_upload._try_utf8_decode('\x80'))
@@ -156,6 +157,54 @@
self.assertFalse(failure)
+class CheckPortageMakeUseVar(cros_test_lib.MockTestCase):
+ """Tests for _check_portage_make_use_var."""
+
+ def setUp(self):
+ self.file_mock = self.PatchObject(pre_upload, '_get_affected_files')
+ self.content_mock = self.PatchObject(pre_upload, '_get_file_content')
+
+ def testMakeConfOmitsOriginalUseValue(self):
+ """Fail for make.conf that discards the previous value of $USE."""
+ self.file_mock.return_value = ['make.conf']
+ self.content_mock.return_value = 'USE="foo"\nUSE="${USE} bar"'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertTrue(failure, failure)
+
+ def testMakeConfCorrectUsage(self):
+ """Succeed for make.conf that preserves the previous value of $USE."""
+ self.file_mock.return_value = ['make.conf']
+ self.content_mock.return_value = 'USE="${USE} foo"\nUSE="${USE}" bar'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertFalse(failure, failure)
+
+ def testMakeDefaultsReferencesOriginalUseValue(self):
+ """Fail for make.defaults that refers to a not-yet-set $USE value."""
+ self.file_mock.return_value = ['make.defaults']
+ self.content_mock.return_value = 'USE="${USE} foo"'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertTrue(failure, failure)
+
+ # Also check for "$USE" without curly brackets.
+ self.content_mock.return_value = 'USE="$USE foo"'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertTrue(failure, failure)
+
+ def testMakeDefaultsOverwritesUseValue(self):
+ """Fail for make.defaults that discards its own $USE value."""
+ self.file_mock.return_value = ['make.defaults']
+ self.content_mock.return_value = 'USE="foo"\nUSE="bar"'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertTrue(failure, failure)
+
+ def testMakeDefaultsCorrectUsage(self):
+ """Succeed for make.defaults that sets and preserves $USE."""
+ self.file_mock.return_value = ['make.defaults']
+ self.content_mock.return_value = 'USE="foo"\nUSE="${USE}" bar'
+ failure = pre_upload._check_portage_make_use_var('PROJECT', 'COMMIT')
+ self.assertFalse(failure, failure)
+
+
class CheckEbuildEapi(cros_test_lib.MockTestCase):
"""Tests for _check_ebuild_eapi."""
