From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 03 Jun 2019 05:56:47 -0700 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 18D2F356F8; Mon, 3 Jun 2019 12:56:47 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-13.rdu2.redhat.com [10.10.121.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3AB2A5D9CC; Mon, 3 Jun 2019 12:56:44 +0000 (UTC) Subject: Re: [RFC PATCH 2/2] BaseTools: add script to configure local git options To: Leif Lindholm , devel@edk2.groups.io Cc: Bob Feng , Liming Gao , Yonghong Zhu , Andrew Fish , Michael D Kinney References: <20190530155933.25588-3-leif.lindholm@linaro.org> From: "Laszlo Ersek" Message-ID: <0e949b1f-da5a-4ff6-95f0-896a2a4f0889@redhat.com> Date: Mon, 3 Jun 2019 14:56:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190530155933.25588-3-leif.lindholm@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 03 Jun 2019 12:56:47 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/30/19 17:59, Leif Lindholm wrote: > Patch contribution and review is greatly simplified by following the > steps described in "Laszlo's unkempt guide": > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers > but there are a lot of tedious manual steps in there, so here is a > python script that configures all options I am aware of > *for the repository the script is executed from*. > > Signed-off-by: Leif Lindholm > --- > BaseTools/Scripts/SetupGit.py | 187 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 187 insertions(+) > create mode 100644 BaseTools/Scripts/SetupGit.py looks good to me, I've got superficial comments only: > diff --git a/BaseTools/Scripts/SetupGit.py b/BaseTools/Scripts/SetupGit.py > new file mode 100644 > index 0000000000..3b199f08b2 > --- /dev/null > +++ b/BaseTools/Scripts/SetupGit.py > @@ -0,0 +1,187 @@ > +## @file > +# Set up the git configuration for contributing to TianoCore projects > +# > +# Copyright (c) 2019, Linaro Ltd. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +from __future__ import print_function > +import argparse > +import os.path > +import sys > + > +try: > + import git > +except ImportError: > + print('Unable to load gitpython module - please install and try again.') > + sys.exit(1) > + > +try: > + # Try Python 2 'ConfigParser' module first since helpful lib2to3 will > + # otherwise automagically load it with the name 'configparser' > + import ConfigParser > +except ImportError: > + # Otherwise, try loading the Python 3 'configparser' uner an alias (1) s/uner/under/ > + try: > + import configparser as ConfigParser > + except ImportError: > + print("Unable to load configparser/ConfigParser module - please install and try again!") > + sys.exit(1) > + > + > +# Assumptions: Script is in edk2/BaseTools/Scripts, > +# templates in edk2/BaseTools/Conf > +CONFDIR = os.path.join(os.path.dirname(os.path.dirname(os.path.realpath(__file__))), > + 'Conf') > + > +UPSTREAMS = [ > + {'name': 'edk2', > + 'repo': 'https://github.com/tianocore/edk2.git', > + 'list': 'devel@edk2.groups.io'}, > + {'name': 'edk2-platforms', > + 'repo': 'https://github.com/tianocore/edk2-platforms.git', > + 'list': 'devel@edk2.groups.io', 'prefix': 'edk2-platforms'}, > + {'name': 'edk2-non-osi', > + 'repo': 'https://github.com/tianocore/edk2-non-osi.git', > + 'list': 'devel@edk2.groups.io', 'prefix': 'edk2-non-osi'} > + ] > + > +# The minimum version required for all of the below options to work > +MIN_GIT_VERSION = (1, 9, 0) > + > +# Set of options to be set identically for all repositories > +OPTIONS = [ > + {'section': 'am', 'option': 'keepcr', 'value': True}, > + {'section': 'am', 'option': 'signoff', 'value': True}, > + {'section': 'cherry-pick', 'option': 'signoff', 'value': True}, > + {'section': 'color', 'option': 'diff', 'value': True}, > + {'section': 'color', 'option': 'grep', 'value': 'auto'}, > + {'section': 'commit', 'option': 'signoff', 'value': True}, > + {'section': 'core', 'option': 'abbrev', 'value': 12}, > + {'section': 'core', 'option': 'attributesFile', > + 'value': os.path.join(CONFDIR, 'gitattributes').replace('\\', '/')}, (2) this could be dropped if we modify the first patch to add .gitattributes > + {'section': 'core', 'option': 'whitespace', 'value': 'cr-at-eol'}, > + {'section': 'diff', 'option': 'algorithm', 'value': 'patience'}, > + {'section': 'diff', 'option': 'orderFile', > + 'value': os.path.join(CONFDIR, 'diff.order').replace('\\', '/')}, > + {'section': 'diff', 'option': 'renames', 'value': 'copies'}, > + {'section': 'diff', 'option': 'statGraphWidth', 'value': '20'}, (3) Can we also add --stat=1000 with a permanent setting like this? > + {'section': 'diff "ini"', 'option': 'xfuncname', > + 'value': '^\\\\[[A-Za-z0-9_., ]+]'}, # or 'diff "ini"'? (4) Is the comment stale? With these investigated (and updated as you see fit): Acked-by: Laszlo Ersek Thanks! Laszlo > + {'section': 'format', 'option': 'coverLetter', 'value': True}, > + {'section': 'format', 'option': 'numbered', 'value': True}, > + {'section': 'format', 'option': 'signoff', 'value': False}, > + {'section': 'notes', 'option': 'rewriteRef', 'value': 'refs/notes/commits'}, > + {'section': 'sendemail', 'option': 'chainreplyto', 'value': False}, > + {'section': 'sendemail', 'option': 'thread', 'value': True}, > + ] > + > + > +def locate_repo(): > + """Opens a Repo object for the current tree, searching upwards in the directory hierarchy.""" > + try: > + repo = git.Repo(path='.', search_parent_directories=True) > + except (git.InvalidGitRepositoryError, git.NoSuchPathError): > + print("It doesn't look like we're inside a git repository - aborting.") > + sys.exit(2) > + return repo > + > + > +def get_upstream(url): > + """Extracts the dict for the current repo origin.""" > + for upstream in UPSTREAMS: > + if upstream['repo'] == url: > + return upstream > + print("Unknown upstream '%s' - aborting!" % url) > + sys.exit(3) > + > + > +def check_versions(): > + """Checks versions of dependencies.""" > + version = git.cmd.Git().version_info > + > + if version < MIN_GIT_VERSION: > + print('Need git version %d.%d or later!' % (version[0], version[1])) > + sys.exit(4) > + > + > +def write_config_value(repo, section, option, data): > + """.""" > + with repo.config_writer(config_level='repository') as configwriter: > + configwriter.set_value(section, option, data) > + > + > +if __name__ == '__main__': > + check_versions() > + > + PARSER = argparse.ArgumentParser( > + description='Sets up a git repository according to TianoCore rules.') > + PARSER.add_argument('-c', '--check', > + help='check current config only, printing what would be changed', > + action='store_true', > + required=False) > + PARSER.add_argument('-f', '--force', > + help='overwrite existing settings conflicting with program defaults', > + action='store_true', > + required=False) > + PARSER.add_argument('-v', '--verbose', > + help='enable more detailed output', > + action='store_true', > + required=False) > + ARGS = PARSER.parse_args() > + > + REPO = locate_repo() > + if REPO.bare: > + print('Bare repo - please check out an upstream one!') > + sys.exit(6) > + > + URL = REPO.remotes.origin.url > + > + UPSTREAM = get_upstream(URL) > + if not UPSTREAM: > + print("Upstream '%s' unknown, aborting!" % URL) > + sys.exit(7) > + > + # Set a list email address if our upstream wants it > + if 'list' in UPSTREAM: > + OPTIONS.append({'section': 'sendemail', 'option': 'to', > + 'value': UPSTREAM['list']}) > + # Append a subject prefix entry to OPTIONS if our upstream wants it > + if 'prefix' in UPSTREAM: > + OPTIONS.append({'section': 'format', 'option': 'subjectPrefix', > + 'value': "PATCH " + UPSTREAM['prefix']}) > + > + CONFIG = REPO.config_reader(config_level='repository') > + > + for entry in OPTIONS: > + exists = False > + try: > + # Make sure to read boolean/int settings as real type rather than strings > + if isinstance(entry['value'], bool): > + value = CONFIG.getboolean(entry['section'], entry['option']) > + elif isinstance(entry['value'], int): > + value = CONFIG.getint(entry['section'], entry['option']) > + else: > + value = CONFIG.get(entry['section'], entry['option']) > + > + exists = True > + # Don't bail out from options not already being set > + except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): > + pass > + > + if exists: > + if value == entry['value']: > + if ARGS.verbose: > + print("%s.%s already set (to '%s')" % (entry['section'], entry['option'], value)) > + else: > + if ARGS.force: > + write_config_value(REPO, entry['section'], entry['option'], entry['value']) > + else: > + print("Not overwriting existing %s.%s value:" % (entry['section'], entry['option'])) > + print(" '%s' != '%s'" % (value, entry['value'])) > + print(" add '-f' to command line to force overwriting existing settings") > + else: > + print("%s.%s => '%s'" % (entry['section'], entry['option'], entry['value'])) > + if not ARGS.check: > + write_config_value(REPO, entry['section'], entry['option'], entry['value']) >