Re: [PATCH 1 of 2 stable v3] tests: set Git author and committer name and email settings explicitly
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
# 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 = 'm...@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 = 'm...@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
Re: [PATCH 1 of 2 stable v3] tests: set Git author and committer name and email settings explicitly
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?)
/Mads
On 20/04/2023 00:39, Manuel Jacob wrote:
# HG changeset patch
# User Manuel Jacob
# 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 = 'm...@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 = 'm...@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
[PATCH 1 of 2 stable v3] tests: set Git author and committer name and email settings explicitly
# HG changeset patch
# User Manuel Jacob
# 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 = 'm...@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 = 'm...@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@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 2 of 2 stable v3] tests: prevent Git system and global configuration from loading
# HG changeset patch # User Manuel Jacob # Date 1680139355 -7200 # Thu Mar 30 03:22:35 2023 +0200 # Branch stable # Node ID e9a574cea28a8db42ff6a84a41a91b6cbe336cbc # Parent 846ca7f28bd40e07c76ed259ce96a31a85d0c4ea # EXP-Topic tests-git tests: prevent Git system and global configuration from loading This reduces differences between different testing environments. Something similar is already done for Mercurial (in the lines directly above this change). Git’s documentation explicitly mentions that passing a value of /dev/null skips reading the respective file. 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 @@ -150,6 +150,8 @@ testenv['LANGUAGE'] = 'en_US:en' testenv['HGPLAIN'] = '' testenv['HGRCPATH'] = '' +testenv['GIT_CONFIG_SYSTEM'] = '/dev/null' +testenv['GIT_CONFIG_GLOBAL'] = '/dev/null' testenv.update(environ) p = Popen(command, shell=True, stdout=PIPE, stderr=PIPE, cwd=self.cwd, env=testenv) stdout, stderr = p.communicate() ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 19.12, Mads Kiilerich wrote: On 19/04/2023 17:27, Manuel Jacob wrote: On 19/04/2023 17.21, Manuel Jacob wrote: On Arch Linux: % git --version git version 2.40.0 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown On CentOS 7: % git --version git version 2.36.5 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown But with openSUSE Leap 15.4 with git 2.35.3, it works (uses $USER as the name). Same on openSUSE Tumbleweed with git 2.40.0. That's interesting obvervations - thanks. It seems like we are far from a full understanding the problem we are trying to fix. There seems to be a high risk that we make more assumptions that will be invalid on some systems. I verified that it works as I expect on my Fedora 38 system, both with the system git 2.40.0 and with a git build from https://github.com/git/git/ source . I don't really have access to other systems. It could be somewhat interesting if you could try to build git from source on your failing systems. It could give a hint if it is some kind of distro patching or build configuration that makes a difference. Or if it is controlled by some other unknown factor. Git infers the default user name from struct passwd’s pw_gecos attribute. That one happened to be empty for the users on the systems on which Git couldn’t infer a non-empty default user name. It could also be interesting to ask some git experts if they could explain why for example 2.40.0 on Arch and openSUSE respond differently - that is really a question of reliable scripting of git and has nothing to do with Kallithea. The code in https://github.com/git/git/blob/main/ident.c has many small steps and it is not obvious what could make it behave differently. Yes, the code is complex. I stepped through it with a debugger before to find out what’s happening. Then I decided that the inference is too complex and system-dependent and we should not let it get that far. That’s why I sent the patch setting everything uniformly via environment variables. Although at some point I decided (for me) that digging into the details of the inference algorithm too much is not worth it to solve the problem, the approach chosen in this patch series was a deliberate choice insteads of trying around until it works (I’m just clarifying this in case you got a different perception). I’ll resend the patch series with improved changeset descriptions and comments, to hopefully be clearer and less confusing. /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 17:27, Manuel Jacob wrote: On 19/04/2023 17.21, Manuel Jacob wrote: On Arch Linux: % git --version git version 2.40.0 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown On CentOS 7: % git --version git version 2.36.5 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown But with openSUSE Leap 15.4 with git 2.35.3, it works (uses $USER as the name). Same on openSUSE Tumbleweed with git 2.40.0. That's interesting obvervations - thanks. It seems like we are far from a full understanding the problem we are trying to fix. There seems to be a high risk that we make more assumptions that will be invalid on some systems. I verified that it works as I expect on my Fedora 38 system, both with the system git 2.40.0 and with a git build from https://github.com/git/git/ source . I don't really have access to other systems. It could be somewhat interesting if you could try to build git from source on your failing systems. It could give a hint if it is some kind of distro patching or build configuration that makes a difference. Or if it is controlled by some other unknown factor. It could also be interesting to ask some git experts if they could explain why for example 2.40.0 on Arch and openSUSE respond differently - that is really a question of reliable scripting of git and has nothing to do with Kallithea. The code in https://github.com/git/git/blob/main/ident.c has many small steps and it is not obvious what could make it behave differently. /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 17.21, Manuel Jacob wrote: On 19/04/2023 16.27, Mads Kiilerich wrote: On 19/04/2023 15:14, Manuel Jacob wrote: On 19/04/2023 14.27, Mads Kiilerich wrote: On 18/04/2023 20:35, Manuel Jacob wrote: ... I think this changeset should come first. As you say, it makes the test work the same way everywhere. user.useconfigonly is probably just one of many settings that could break it. If it still doesn't work for you with this change (without the previous change), it can't be because your setup has user.useconfigonly set. There must be some other explanation. If I have only this changeset without the parent, command `git commit -m "committed new 0" --author "User ǝɯɐᴎ " "97u1nbm0setup.py"` fails with: Committer identity unknown *** Please tell me who you are. ... Ok, after experimenting and reading git man pages, the shortest and clearest description seems to be: The git commit --author option can tell git to use another name as author when committing, but git still has to know the user to use as committer (to be shown with --format=full). Without any git user configuration (for example because there is no ~/.gitconfig or because GIT_CONFIG_GLOBAL=/dev/null or because user.useConfigOnly), commit will fail with some kind of error, even if --author is specified. But if EMAIL is set (and without user.useConfigOnly), it will use that as username, apparently combined with the getpwnam gecos information. The following thus seems to works for me - no matter what global/system git configuration I have: GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " On Arch Linux: % git --version git version 2.40.0 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown *** Please tell me who you are. Run git config --global user.email "y...@example.com" git config --global user.name "Your Name" to set your account's default identity. Omit --global to set the identity only in this repository. fatal: empty ident name (for ) not allowed On CentOS 7: % git --version git version 2.36.5 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown *** Please tell me who you are. Run git config --global user.email "y...@example.com" git config --global user.name "Your Name" to set your account's default identity. Omit --global to set the identity only in this repository. fatal: empty ident name (for ) not allowed But with openSUSE Leap 15.4 with git 2.35.3, it works (uses $USER as the name). Same on openSUSE Tumbleweed with git 2.40.0. Silently dropping the setting of EMAIL in the first changeset is thus a bit risky. Could the consequences of that perhaps have interfered with your testing? /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 16.27, Mads Kiilerich wrote: On 19/04/2023 15:14, Manuel Jacob wrote: On 19/04/2023 14.27, Mads Kiilerich wrote: On 18/04/2023 20:35, Manuel Jacob wrote: ... I think this changeset should come first. As you say, it makes the test work the same way everywhere. user.useconfigonly is probably just one of many settings that could break it. If it still doesn't work for you with this change (without the previous change), it can't be because your setup has user.useconfigonly set. There must be some other explanation. If I have only this changeset without the parent, command `git commit -m "committed new 0" --author "User ǝɯɐᴎ " "97u1nbm0setup.py"` fails with: Committer identity unknown *** Please tell me who you are. ... Ok, after experimenting and reading git man pages, the shortest and clearest description seems to be: The git commit --author option can tell git to use another name as author when committing, but git still has to know the user to use as committer (to be shown with --format=full). Without any git user configuration (for example because there is no ~/.gitconfig or because GIT_CONFIG_GLOBAL=/dev/null or because user.useConfigOnly), commit will fail with some kind of error, even if --author is specified. But if EMAIL is set (and without user.useConfigOnly), it will use that as username, apparently combined with the getpwnam gecos information. The following thus seems to works for me - no matter what global/system git configuration I have: GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " On Arch Linux: % git --version git version 2.40.0 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown *** Please tell me who you are. Run git config --global user.email "y...@example.com" git config --global user.name "Your Name" to set your account's default identity. Omit --global to set the identity only in this repository. fatal: empty ident name (for ) not allowed On CentOS 7: % git --version git version 2.36.5 % GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Committer identity unknown *** Please tell me who you are. Run git config --global user.email "y...@example.com" git config --global user.name "Your Name" to set your account's default identity. Omit --global to set the identity only in this repository. fatal: empty ident name (for ) not allowed But with openSUSE Leap 15.4 with git 2.35.3, it works (uses $USER as the name). Silently dropping the setting of EMAIL in the first changeset is thus a bit risky. Could the consequences of that perhaps have interfered with your testing? /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 15:14, Manuel Jacob wrote: On 19/04/2023 14.27, Mads Kiilerich wrote: On 18/04/2023 20:35, Manuel Jacob wrote: ... I think this changeset should come first. As you say, it makes the test work the same way everywhere. user.useconfigonly is probably just one of many settings that could break it. If it still doesn't work for you with this change (without the previous change), it can't be because your setup has user.useconfigonly set. There must be some other explanation. If I have only this changeset without the parent, command `git commit -m "committed new 0" --author "User ǝɯɐᴎ " "97u1nbm0setup.py"` fails with: Committer identity unknown *** Please tell me who you are. ... Ok, after experimenting and reading git man pages, the shortest and clearest description seems to be: The git commit --author option can tell git to use another name as author when committing, but git still has to know the user to use as committer (to be shown with --format=full). Without any git user configuration (for example because there is no ~/.gitconfig or because GIT_CONFIG_GLOBAL=/dev/null or because user.useConfigOnly), commit will fail with some kind of error, even if --author is specified. But if EMAIL is set (and without user.useConfigOnly), it will use that as username, apparently combined with the getpwnam gecos information. The following thus seems to works for me - no matter what global/system git configuration I have: GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null EMAIL='foo@bar' git commit -m "committed new 0" --author "User ǝɯɐᴎ " Silently dropping the setting of EMAIL in the first changeset is thus a bit risky. Could the consequences of that perhaps have interfered with your testing? /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 19/04/2023 14.27, Mads Kiilerich wrote: On 18/04/2023 20:35, Manuel Jacob wrote: # HG changeset patch # User Manuel Jacob # Date 1680139355 -7200 # Thu Mar 30 03:22:35 2023 +0200 # Branch stable # Node ID e5251abd0a3c677d7bb0828f3a744789bd6fe4cb # Parent 30082bb9719eb00f3be0081b7221d7c3061d4345 # EXP-Topic tests-git tests: prevent Git system and global configuration from loading This reduces differences between different testing environments. Something similar is already done for Mercurial (in the lines directly above this change). Yes, I agree that it is a problem/bug that the global configuration is used at all. Global configuration should be disabled for git, as we do for hg. The parent changeset has originally been added to support user.useconfigonly. With this changeset, the original motivation for it becomes obsolete. However, it is still necessary to set the committer name via a environment variable, at least on my machine. I think this changeset should come first. As you say, it makes the test work the same way everywhere. user.useconfigonly is probably just one of many settings that could break it. If it still doesn't work for you with this change (without the previous change), it can't be because your setup has user.useconfigonly set. There must be some other explanation. If I have only this changeset without the parent, command `git commit -m "committed new 0" --author "User ǝɯɐᴎ " "97u1nbm0setup.py"` fails with: Committer identity unknown *** Please tell me who you are. Run git config --global user.email "y...@example.com" git config --global user.name "Your Name" to set your account's default identity. Omit --global to set the identity only in this repository. fatal: empty ident name (for ) not allowed [end of output] Because the error shouldn’t be the result of my configuration after this changeset, I would expect that it fails without the parent changeset on other machines as well (at least those with the same Git version as me (2.40.0)). I never intended to imply that user.useconfigonly is the only reason why the previous changeset is required. Sorry for the confusion! (Note that the GIT_CONFIG_ environment variables were introduced two years ago, and Kallithea still supports the 10 year old Git 1.7.4 . We must thus make sure we don't rely too much on the new feature.) 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 @@ -150,6 +150,8 @@ testenv['LANGUAGE'] = 'en_US:en' testenv['HGPLAIN'] = '' testenv['HGRCPATH'] = '' + testenv['GIT_CONFIG_SYSTEM'] = '' + testenv['GIT_CONFIG_GLOBAL'] = '' As mentioned before: the git man page is very clear that the variables should be set to /dev/null to skip reading configuration files. There is no indication in the git man page (or anywhere else I have found) that setting to /dev/null shouldn't work across all platforms. It seems like git in compat/mingw.c takes care of making it cross platform. There is no mention of the semantics of setting it to an empty string. I do not like relying on undefined behaviour. You’re right that the Git documentation mentions `/dev/null` but the empty string. I’ll change it. If you disagree and have found a good reason to use an empty string instead of /dev/null, then please be explicit in code or commit message. /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 2 stable v2] tests: prevent Git system and global configuration from loading
On 18/04/2023 20:35, Manuel Jacob wrote: # HG changeset patch # User Manuel Jacob # Date 1680139355 -7200 # Thu Mar 30 03:22:35 2023 +0200 # Branch stable # Node ID e5251abd0a3c677d7bb0828f3a744789bd6fe4cb # Parent 30082bb9719eb00f3be0081b7221d7c3061d4345 # EXP-Topic tests-git tests: prevent Git system and global configuration from loading This reduces differences between different testing environments. Something similar is already done for Mercurial (in the lines directly above this change). Yes, I agree that it is a problem/bug that the global configuration is used at all. Global configuration should be disabled for git, as we do for hg. The parent changeset has originally been added to support user.useconfigonly. With this changeset, the original motivation for it becomes obsolete. However, it is still necessary to set the committer name via a environment variable, at least on my machine. I think this changeset should come first. As you say, it makes the test work the same way everywhere. user.useconfigonly is probably just one of many settings that could break it. If it still doesn't work for you with this change (without the previous change), it can't be because your setup has user.useconfigonly set. There must be some other explanation. (Note that the GIT_CONFIG_ environment variables were introduced two years ago, and Kallithea still supports the 10 year old Git 1.7.4 . We must thus make sure we don't rely too much on the new feature.) 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 @@ -150,6 +150,8 @@ testenv['LANGUAGE'] = 'en_US:en' testenv['HGPLAIN'] = '' testenv['HGRCPATH'] = '' +testenv['GIT_CONFIG_SYSTEM'] = '' +testenv['GIT_CONFIG_GLOBAL'] = '' As mentioned before: the git man page is very clear that the variables should be set to /dev/null to skip reading configuration files. There is no indication in the git man page (or anywhere else I have found) that setting to /dev/null shouldn't work across all platforms. It seems like git in compat/mingw.c takes care of making it cross platform. There is no mention of the semantics of setting it to an empty string. I do not like relying on undefined behaviour. If you disagree and have found a good reason to use an empty string instead of /dev/null, then please be explicit in code or commit message. /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
