[PATCH 1 of 2 stable v3] tests: set Git author and committer name and email settings explicitly

[Manuel Jacob]

On 20/04/2023 01.07, Mads Kiilerich wrote:
> Thanks - this looks much more approachable.
> 
> But while processing v2 and our discussion and trying to give good input 
> to what I would like to see in v3, I figured it perhaps would be better 
> with a simpler solution.
> 
> What do you think about doing it as in 
> https://kallithea-scm.org/repos/kallithea-incoming/changelog/61393fa29552/?size=2 ? (Possibly incorporating some of the new summary in v3?)

Implementation-wise, either would be fine for me, as long as the 
solution avoids system-dependent behavior (in that aspect, both 
implementations are equivalently good).

My changeset description is a bit more extensive. I’ll let you decide if 
merging the two patches is worth the effort in case you decide for your 
implementation.

> /Mads
> 
> 
> On 20/04/2023 00:39, Manuel Jacob wrote:
>> # HG changeset patch
>> # User Manuel Jacob <me at manueljacob.de>
>> # Date 1680121377 -7200
>> #      Wed Mar 29 22:22:57 2023 +0200
>> # Branch stable
>> # Node ID 846ca7f28bd40e07c76ed259ce96a31a85d0c4ea
>> # Parent  0a9ddb8cd8c117671ecaf2b4126c3eef09e80ce8
>> # EXP-Topic tests-git
>> tests: set Git author and committer name and email settings explicitly
>>
>> Git tries to find out name and email in this order:
>>
>> 1. The author can be set e.g. via the `--author` option of `git commit`.
>> 2. If set, the environment variables GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
>>     GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL are taken.
>> 3. If set, various config options are considered.
>> 4. Unless disabled by the user.useconfigonly config, the names and 
>> emails are
>>     infered from various system sources such as various fields from 
>> /etc/passwd,
>>     /etc/mailname and the environment variable EMAIL.
>>
>> The author was previously passed via (1), but that doesn’t work for the
>> committer.
>>
>> We don’t modify Git’s configuration files, so the result of (3) 
>> depends on the
>> system the tests run on, which should be avoided. A follow-up patch 
>> will be
>> sent for instructing Git to not read the system Git configuration files.
>>
>> (4) is also system-dependent. On my system, (4) was disabled in the Git
>> configuration. If I enabled it, Git tried to infer the committer name 
>> from a
>> field from /etc/passwd that is empty for my user I ran the tests on, 
>> which Git
>> didn’t like. The previous code passed the environment variable EMAIL, 
>> which,
>> according to a comment, is only required on some systems, but it’s 
>> unclear why.
>>
>> By passing the names and emails via (2), we can set the author and 
>> committer
>> name and email uniformly and prevent Git from using the 
>> system-dependent ways
>> (3) and (4). The environment variables were introduced in 2005, so 
>> there should
>> be no backwards compatibility problems.
>>
>> diff --git a/kallithea/tests/other/test_vcs_operations.py 
>> b/kallithea/tests/other/test_vcs_operations.py
>> --- a/kallithea/tests/other/test_vcs_operations.py
>> +++ b/kallithea/tests/other/test_vcs_operations.py
>> @@ -167,6 +167,27 @@
>>       return tempfile.mkdtemp(dir=base.TESTS_TMP_PATH, prefix=prefix, 
>> suffix=suffix)
>> +def _commit(vcs, dest_dir, message, *extra_args):
>> +    email = 'me at example.com'
>> +    if os.name == 'nt':
>> +        name = 'User'
>> +    else:
>> +        name = 'User ǝɯɐᴎ'
>> +
>> +    return Command(dest_dir).execute(
>> +        vcs,
>> +        'commit',
>> +        '-m',
>> +        '"%s"' % message,
>> +        *extra_args,
>> +        HGUSER='%s <%s>' % (name, email),
>> +        GIT_AUTHOR_NAME=name,
>> +        GIT_AUTHOR_EMAIL=email,
>> +        GIT_COMMITTER_NAME=name,
>> +        GIT_COMMITTER_EMAIL=email,
>> +    )
>> +
>> +
>>   def _add_files(vcs, dest_dir, files_no=3):
>>       """
>>       Generate some files, add it to dest_dir repo and push back
>> @@ -179,24 +200,10 @@
>>       open(os.path.join(dest_dir, added_file), 'a').close()
>>       Command(dest_dir).execute(vcs, 'add', added_file)
>> -    email = 'me at example.com'
>> -    if os.name == 'nt':
>> -        author_str = 'User <%s>' % email
>> -    else:
>> -        author_str = 'User ǝɯɐᴎ <%s>' % email
>>       for i in range(files_no):
>>           cmd = """echo "added_line%s" >> %s""" % (i, added_file)
>>           Command(dest_dir).execute(cmd)
>> -        if vcs == 'hg':
>> -            cmd = """hg commit -m "committed new %s" -u "%s" "%s" """ 
>> % (
>> -                i, author_str, added_file
>> -            )
>> -        elif vcs == 'git':
>> -            cmd = """git commit -m "committed new %s" --author "%s" 
>> "%s" """ % (
>> -                i, author_str, added_file
>> -            )
>> -        # git commit needs EMAIL on some machines
>> -        Command(dest_dir).execute(cmd, EMAIL=email)
>> +        _commit(vcs, dest_dir, "committed new %s" % i, added_file)
>>   def _add_files_and_push(webserver, vt, dest_dir, clone_url, 
>> ignoreReturnCode=False, files_no=3):
>>       _add_files(vt.repo_type, dest_dir, files_no=files_no)
>> @@ -618,7 +625,7 @@
>>           # add submodule
>>           stdout, stderr = Command(base.TESTS_TMP_PATH).execute('git 
>> clone', fork_url, dest_dir)
>>           stdout, stderr = Command(dest_dir).execute('git submodule 
>> add', clone_url, 'testsubmodule')
>> -        stdout, stderr = Command(dest_dir).execute('git commit -am 
>> "added testsubmodule pointing to', clone_url, '"', 
>> EMAIL=base.TEST_USER_ADMIN_EMAIL)
>> +        stdout, stderr = _commit('git', dest_dir, "added 
>> testsubmodule pointing to %s" % clone_url, "-a")
>>           stdout, stderr = Command(dest_dir).execute('git push', 
>> fork_url, 'master')
>>           # check for testsubmodule link in files page
>> _______________________________________________
>> kallithea-general mailing list
>> kallithea-general at sfconservancy.org
>> https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
> 
>