From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=e9R3Lj84; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by groups.io with SMTP; Mon, 03 Jun 2019 06:06:36 -0700 Received: by mail-wr1-f65.google.com with SMTP id d18so12001718wrs.5 for ; Mon, 03 Jun 2019 06:06:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=q3rS2rtf/7QqEX8sbobSSc9EeAYyRm1XJbDrmfjmYlc=; b=e9R3Lj84rcGucRNAhhUnS/2ssgehBm3eaaQApocsQs1hWakw01FWkIyS8PsVZs4kpt xrfaHE+bwxrJ1rngyLao8fVdF/i+MrkOezybgSDPkhgBy3t0llrw6N5iehK5v1DWG1n0 NN3QSlK/8o3yJjDuK+lRQs9sdZ2/vlq6hk5ulvJryTon4H5ggeKFuiQtTV4jpGKPiBFT ILCiq6gfkoqvvM/AvqCMdOSTAGcRtnmvIjiizqgMZ8cd0hbjAomUiy1BuMnyn2aEQK/I vzUrWNmoPla1672dBeJgN+nKc5o67jAuO8cO/QDFEdGL8fedCRfNQxruFxqJvamlJfj6 yl6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=q3rS2rtf/7QqEX8sbobSSc9EeAYyRm1XJbDrmfjmYlc=; b=sSEKI++kW7nlLxKD2ghfeWuM9a21EU5eJobPBR6O6dKle0Z6bVdC0JtSyPRD2K6k3L QYVH+ga4r3+uMuxOITyhzJ8cB9+tBcPOK4y2BHPVDgJPPr4QUGvjqaq34kG4uciKvi8S d6g4NA0kYa3vx+wmBvZSK/8+gIdKf6QlJmy7/k5OJ/2e2eKkw4tFDzJ6wwUybyWq+WvW uCEFlNtqB/osUFDxmU3pFTdOvT6EWy12qwKF/dPoQlwcZP1LgoDMS8ZCsyUy1/Ym6+cK OtN7DGKIL0SZi0HB5ar9H+SRllueqqPhBWcNxI5+96OCSMnHPNGlmUPEo/3UJFqkpmRw 4zbA== X-Gm-Message-State: APjAAAWsQAug+0bZsjEqkiyyLudw6eUjlYlsFBThf5isrwyrKAxLnFX7 2Ji3x3TDKOdJqEGnQd5gormAyA== X-Google-Smtp-Source: APXvYqwkOunqJyzznPirrmOdHQKuDXF+Jc5Pq1Z7h7hC+sCW9d1HWzjGVqtowyGz9RBr3+ozjMWP4g== X-Received: by 2002:adf:ea44:: with SMTP id j4mr1365837wrn.123.1559567194975; Mon, 03 Jun 2019 06:06:34 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id f2sm14976630wrq.48.2019.06.03.06.06.34 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 03 Jun 2019 06:06:34 -0700 (PDT) Date: Mon, 3 Jun 2019 14:06:32 +0100 From: "Leif Lindholm" To: Laszlo Ersek Cc: devel@edk2.groups.io, Bob Feng , Liming Gao , Yonghong Zhu , Andrew Fish , Michael D Kinney Subject: Re: [RFC PATCH 2/2] BaseTools: add script to configure local git options Message-ID: <20190603130632.it7r6hu2t7b2z5ek@bivouac.eciton.net> References: <20190530155933.25588-3-leif.lindholm@linaro.org> <0e949b1f-da5a-4ff6-95f0-896a2a4f0889@redhat.com> MIME-Version: 1.0 In-Reply-To: <0e949b1f-da5a-4ff6-95f0-896a2a4f0889@redhat.com> User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jun 03, 2019 at 02:56:42PM +0200, Laszlo Ersek wrote: > 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/ Oops, thanks! > > + 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 Yes, but only for the edk2 repo. > > + {'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? Not as far as I can tell. We would need to add that option to git :/ However, it should be quite doable to add it conditionally for versions that support it, if we do. > > + {'section': 'diff "ini"', 'option': 'xfuncname', > > + 'value': '^\\\\[[A-Za-z0-9_., ]+]'}, # or 'diff "ini"'? > > (4) Is the comment stale? Yeah, I noticed that one after sending. On of my "no idea what the layers will do to the input" comments I forgot to drop when I had verified. > With these investigated (and updated as you see fit): > > Acked-by: Laszlo Ersek Thanks! / Leif > 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']) > > >