From e5670c881225ed025c77e0362a7c7edcc912ef9f Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Thu, 7 Jan 2021 22:14:25 -0500 Subject: [PATCH] launcher: add a requirements framework to declare version dependencies Currently we don't have a way for the checked out repo version to declare the version of tools it needs before we start running it. For somethings, like git, it's not a big deal as it can handle all the asserts itself. But for things like Python, it's impossible to reliably check before executing. We're in this state now: - we've been allowing Python 3.4, so the launcher accepts it - the repo codebase starts using Python 3.6 features - launcher tries to import us but hits syntax errors - user is left confused and assuming new repo is broken because they're seeing syntax errors This scenario is playing out with old launchers that still accept Python 2, and will continue to play out as time goes on and we want to require newer versions of Python 3. Lets create a JSON file to declare all these system requirements. That file format is extremely stable, so loading & parsing from even ancient versions of Python shouldn't be a problem. Then the launcher can read these settings and check the system state before attempting to execute any code. If the tools are too old, it can clearly diagnose & display information to the user as to the real problem (and not emit tracebacks or syntax errors). We have a couple of different tool version checks already (git, python, ssh) and can harmonize them in a single place. This also allows us to assert a reverse dependency if the need ever comes up: force the user to upgrade their `repo` launcher before we'll let them run us. Even though the launcher warns whenever a newer release is available, some users seem to ignore that, or they don't use repo that often (on the scale of years), and their upgrade jump is so dramatic that they fall back into the syntax error pit. Hopefully by the end of the year we can assume enough people have upgraded their launcher such that we can delete all of the duplicate version checks in the codebase. But until then, we'll keep them to maintain coverage. Change-Id: I5c12bbffdfd0a8ce978f39aa7f4674026fe9f4f8 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/293003 Reviewed-by: Michael Mortensen Tested-by: Mike Frysinger --- repo | 89 +++++++++++++++++++++++++++++++++++++++++++ requirements.json | 57 +++++++++++++++++++++++++++ tests/test_wrapper.py | 76 ++++++++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+) create mode 100644 requirements.json diff --git a/repo b/repo index 8f13015f..8936f57b 100755 --- a/repo +++ b/repo @@ -246,6 +246,7 @@ GITC_FS_ROOT_DIR = '/gitc/manifest-rw/' import collections import errno +import json import optparse import re import shutil @@ -1035,6 +1036,90 @@ def _ParseArguments(args): return cmd, opt, arg +class Requirements(object): + """Helper for checking repo's system requirements.""" + + REQUIREMENTS_NAME = 'requirements.json' + + def __init__(self, requirements): + """Initialize. + + Args: + requirements: A dictionary of settings. + """ + self.requirements = requirements + + @classmethod + def from_dir(cls, path): + return cls.from_file(os.path.join(path, cls.REQUIREMENTS_NAME)) + + @classmethod + def from_file(cls, path): + try: + with open(path, 'rb') as f: + data = f.read() + except EnvironmentError: + # NB: EnvironmentError is used for Python 2 & 3 compatibility. + # If we couldn't open the file, assume it's an old source tree. + return None + + return cls.from_data(data) + + @classmethod + def from_data(cls, data): + comment_line = re.compile(br'^ *#') + strip_data = b''.join(x for x in data.splitlines() if not comment_line.match(x)) + try: + json_data = json.loads(strip_data) + except Exception: # pylint: disable=broad-except + # If we couldn't parse it, assume it's incompatible. + return None + + return cls(json_data) + + def _get_soft_ver(self, pkg): + """Return the soft version for |pkg| if it exists.""" + return self.requirements.get(pkg, {}).get('soft', ()) + + def _get_hard_ver(self, pkg): + """Return the hard version for |pkg| if it exists.""" + return self.requirements.get(pkg, {}).get('hard', ()) + + @staticmethod + def _format_ver(ver): + """Return a dotted version from |ver|.""" + return '.'.join(str(x) for x in ver) + + def assert_ver(self, pkg, curr_ver): + """Verify |pkg|'s |curr_ver| is new enough.""" + curr_ver = tuple(curr_ver) + soft_ver = tuple(self._get_soft_ver(pkg)) + hard_ver = tuple(self._get_hard_ver(pkg)) + if curr_ver < hard_ver: + print('repo: error: Your version of "%s" (%s) is unsupported; ' + 'Please upgrade to at least version %s to continue.' % + (pkg, self._format_ver(curr_ver), self._format_ver(soft_ver)), + file=sys.stderr) + sys.exit(1) + + if curr_ver < soft_ver: + print('repo: warning: Your version of "%s" (%s) is no longer supported; ' + 'Please upgrade to at least version %s to avoid breakage.' % + (pkg, self._format_ver(curr_ver), self._format_ver(soft_ver)), + file=sys.stderr) + + def assert_all(self): + """Assert all of the requirements are satisified.""" + # See if we need a repo launcher upgrade first. + self.assert_ver('repo', VERSION) + + # Check python before we try to import the repo code. + self.assert_ver('python', sys.version_info) + + # Check git while we're at it. + self.assert_ver('git', ParseGitVersion()) + + def _Usage(): gitc_usage = "" if get_gitc_manifest_dir(): @@ -1192,6 +1277,10 @@ def main(orig_args): print("fatal: unable to find repo entry point", file=sys.stderr) sys.exit(1) + reqs = Requirements.from_dir(os.path.dirname(repo_main)) + if reqs: + reqs.assert_all() + ver_str = '.'.join(map(str, VERSION)) me = [sys.executable, repo_main, '--repo-dir=%s' % rel_repo_dir, diff --git a/requirements.json b/requirements.json new file mode 100644 index 00000000..86b9a46c --- /dev/null +++ b/requirements.json @@ -0,0 +1,57 @@ +# This file declares various requirements for this version of repo. The +# launcher script will load it and check the constraints before trying to run +# us. This avoids issues of the launcher using an old version of Python (e.g. +# 3.5) while the codebase has moved on to requiring something much newer (e.g. +# 3.8). If the launcher tried to import us, it would fail with syntax errors. + +# This is a JSON file with line-level comments allowed. + +# Always keep backwards compatibility in mine. The launcher script is robust +# against missing values, but when a field is renamed/removed, it means older +# versions of the launcher script won't be able to enforce the constraint. + +# When requiring versions, always use lists as they are easy to parse & compare +# in Python. Strings would require futher processing to turn into a list. + +# Version constraints should be expressed in pairs: soft & hard. Soft versions +# are when we start warning users that their software too old and we're planning +# on dropping support for it, so they need to start planning system upgrades. +# Hard versions are when we refuse to work the tool. Users will be shown an +# error message before we abort entirely. + +# When deciding whether to upgrade a version requirement, check out the distro +# lists to see who will be impacted: +# https://gerrit.googlesource.com/git-repo/+/HEAD/docs/release-process.md#Project-References + +{ + # The repo launcher itself. This allows us to force people to upgrade as some + # ignore the warnings about it being out of date, or install ancient versions + # to start with for whatever reason. + # + # NB: Repo launchers started checking this file with repo-2.12, so listing + # versions older than that won't make a difference. + "repo": { + "hard": [2, 11], + "soft": [2, 11] + }, + + # Supported Python versions. + # + # python-3.6 is in Ubuntu Bionic. + # python-3.5 is in Debian Stretch. + "python": { + "hard": [3, 5], + "soft": [3, 6] + }, + + # Supported git versions. + # + # git-1.7.2 is in Debian Squeeze. + # git-1.7.9 is in Ubuntu Precise. + # git-1.9.1 is in Ubuntu Trusty. + # git-1.7.10 is in Debian Wheezy. + "git": { + "hard": [1, 7, 2], + "soft": [1, 9, 1] + } +} diff --git a/tests/test_wrapper.py b/tests/test_wrapper.py index d8713738..6400faf4 100644 --- a/tests/test_wrapper.py +++ b/tests/test_wrapper.py @@ -19,6 +19,7 @@ from io import StringIO import os import re import shutil +import sys import tempfile import unittest from unittest import mock @@ -255,6 +256,81 @@ class CheckGitVersion(RepoWrapperTestCase): self.wrapper._CheckGitVersion() +class Requirements(RepoWrapperTestCase): + """Check Requirements handling.""" + + def test_missing_file(self): + """Don't crash if the file is missing (old version).""" + testdir = os.path.dirname(os.path.realpath(__file__)) + self.assertIsNone(self.wrapper.Requirements.from_dir(testdir)) + self.assertIsNone(self.wrapper.Requirements.from_file( + os.path.join(testdir, 'xxxxxxxxxxxxxxxxxxxxxxxx'))) + + def test_corrupt_data(self): + """If the file can't be parsed, don't blow up.""" + self.assertIsNone(self.wrapper.Requirements.from_file(__file__)) + self.assertIsNone(self.wrapper.Requirements.from_data(b'x')) + + def test_valid_data(self): + """Make sure we can parse the file we ship.""" + self.assertIsNotNone(self.wrapper.Requirements.from_data(b'{}')) + rootdir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) + self.assertIsNotNone(self.wrapper.Requirements.from_dir(rootdir)) + self.assertIsNotNone(self.wrapper.Requirements.from_file(os.path.join( + rootdir, 'requirements.json'))) + + def test_format_ver(self): + """Check format_ver can format.""" + self.assertEqual('1.2.3', self.wrapper.Requirements._format_ver((1, 2, 3))) + self.assertEqual('1', self.wrapper.Requirements._format_ver([1])) + + def test_assert_all_unknown(self): + """Check assert_all works with incompatible file.""" + reqs = self.wrapper.Requirements({}) + reqs.assert_all() + + def test_assert_all_new_repo(self): + """Check assert_all accepts new enough repo.""" + reqs = self.wrapper.Requirements({'repo': {'hard': [1, 0]}}) + reqs.assert_all() + + def test_assert_all_old_repo(self): + """Check assert_all rejects old repo.""" + reqs = self.wrapper.Requirements({'repo': {'hard': [99999, 0]}}) + with self.assertRaises(SystemExit): + reqs.assert_all() + + def test_assert_all_new_python(self): + """Check assert_all accepts new enough python.""" + reqs = self.wrapper.Requirements({'python': {'hard': sys.version_info}}) + reqs.assert_all() + + def test_assert_all_old_repo(self): + """Check assert_all rejects old repo.""" + reqs = self.wrapper.Requirements({'python': {'hard': [99999, 0]}}) + with self.assertRaises(SystemExit): + reqs.assert_all() + + def test_assert_ver_unknown(self): + """Check assert_ver works with incompatible file.""" + reqs = self.wrapper.Requirements({}) + reqs.assert_ver('xxx', (1, 0)) + + def test_assert_ver_new(self): + """Check assert_ver allows new enough versions.""" + reqs = self.wrapper.Requirements({'git': {'hard': [1, 0], 'soft': [2, 0]}}) + reqs.assert_ver('git', (1, 0)) + reqs.assert_ver('git', (1, 5)) + reqs.assert_ver('git', (2, 0)) + reqs.assert_ver('git', (2, 5)) + + def test_assert_ver_old(self): + """Check assert_ver rejects old versions.""" + reqs = self.wrapper.Requirements({'git': {'hard': [1, 0], 'soft': [2, 0]}}) + with self.assertRaises(SystemExit): + reqs.assert_ver('git', (0, 5)) + + class NeedSetupGnuPG(RepoWrapperTestCase): """Check NeedSetupGnuPG behavior."""