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 <leif.lindh...@linaro.org>
> > ---
> > 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.<BR>
> > +#
> > +# 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 <ler...@redhat.com>
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'])
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#41820): https://edk2.groups.io/g/devel/message/41820
Mute This Topic: https://groups.io/mt/31870258/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
