{"thread":{"id":"49717","subject":"[PATCH 0/4] mingw: prevent external PERL5LIB from interfering","startedAt":"2018-10-30T18:40:05Z","lastAt":"2018-10-31T11:04:38Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"362000","messageId":"pull.62.git.gitgitgadget@gmail.com","threadId":"49717","inReplyTo":null,"subject":"[PATCH 0/4] mingw: prevent external PERL5LIB from interfering","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-30T18:40:02Z","receivedAt":"2018-10-30T18:40:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In Git for Windows, we do not support running the Perl scripts with any\nrandom Perl interpreter. Instead, we insist on using the shipped one (except\nfor MinGit, where we do not even ship the Perl scripts, to save on space).\n\nHowever, if Git is called from, say, a Perl script running in a different\nPerl interpreter with appropriately configured PERL5LIB, it messes with\nGit's ability to run its Perl scripts.\n\nFor that reason, we devised the presented method of defining a list of\nenvironment variables (via core.unsetEnvVars) that would then be unset\nbefore spawning any process, defaulting to PERL5LIB.\n\nAn alternative approach which was rejected at the time (because it\ninterfered with the then-ongoing work to compile Git for Windows using MS\nVisual C++) would patch the make_environment_block() function to skip the\nspecified environment variables before handing down the environment block to\nthe spawned process. Currently it would interfere with the mingw-utf-8-env\npatch series I sent earlier today\n[https://public-inbox.org/git/pull.57.git.gitgitgadget@gmail.com].\n\nWhile at it, this patch series also cleans up house and moves the\nWindows-specific core.* variable handling to compat/mingw.c rather than\ncluttering environment.c and config.c with things that e.g. developers on\nLinux do not want to care about.\n\nJohannes Schindelin (4):\n config: rename `dummy` parameter to `cb` in git_default_config()\n Allow for platform-specific core.* config settings\n Move Windows-specific config settings into compat/mingw.c\n mingw: unset PERL5LIB by default\n\n Documentation/config.txt | 6 ++++\n cache.h | 8 -----\n compat/mingw.c | 58 +++++++++++++++++++++++++++++++++++-\n compat/mingw.h | 3 ++\n config.c | 18 ++++-------\n environment.c | 1 -\n git-compat-util.h | 8 +++++\n t/t0029-core-unsetenvvars.sh | 30 +++++++++++++++++++\n 8 files changed, 109 insertions(+), 23 deletions(-)\n create mode 100755 t/t0029-core-unsetenvvars.sh\n\n\nbase-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-62%2Fdscho%2Fperl5lib-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-62/dscho/perl5lib-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/62\n-- \ngitgitgadget\n"},{"id":"362001","messageId":"660c740fbc190274e1e124672d87abdada97ec2b.1540924800.git.gitgitgadget@gmail.com","threadId":"49717","inReplyTo":"pull.62.git.gitgitgadget@gmail.com","subject":"[PATCH 1/4] config: rename `dummy` parameter to `cb` in git_default_config()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-30T18:40:03Z","receivedAt":"2018-10-30T18:40:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is the convention elsewhere (and prepares for the case where we may\nneed to pass callback data).\n\nSigned-off-by: Johannes Schindelin \n---\n config.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 4051e3882..3687c6783 100644\n--- a/config.c\n+++ b/config.c\n@@ -1448,13 +1448,13 @@ static int git_default_mailmap_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-int git_default_config(const char *var, const char *value, void *dummy)\n+int git_default_config(const char *var, const char *value, void *cb)\n {\n \tif (starts_with(var, \"core.\"))\n \t\treturn git_default_core_config(var, value);\n \n \tif (starts_with(var, \"user.\"))\n-\t\treturn git_ident_config(var, value, dummy);\n+\t\treturn git_ident_config(var, value, cb);\n \n \tif (starts_with(var, \"i18n.\"))\n \t\treturn git_default_i18n_config(var, value);\n-- \ngitgitgadget\n\n"},{"id":"362002","messageId":"f4050b432d509e6696e630a174dfdfef4e4a66dd.1540924800.git.gitgitgadget@gmail.com","threadId":"49717","inReplyTo":"pull.62.git.gitgitgadget@gmail.com","subject":"[PATCH 2/4] Allow for platform-specific core.* config settings","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-30T18:40:04Z","receivedAt":"2018-10-30T18:40:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the Git for Windows project, we have ample precendent for config\nsettings that apply to Windows, and to Windows only.\n\nLet's formalize this concept by introducing a platform_core_config()\nfunction that can be #define'd in a platform-specific manner.\n\nThis will allow us to contain platform-specific code better, as the\ncorresponding variables no longer need to be exported so that they can\nbe defined in environment.c and be set in config.c\n\nSigned-off-by: Johannes Schindelin \n---\n compat/mingw.c | 5 +++++\n compat/mingw.h | 3 +++\n config.c | 6 +++---\n git-compat-util.h | 8 ++++++++\n 4 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 81ef24286..293f286af 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -203,6 +203,11 @@ static int ask_yes_no_if_possible(const char *format, ...)\n \t}\n }\n \n+int mingw_core_config(const char *var, const char *value, void *cb)\n+{\n+\treturn 0;\n+}\n+\n /* Normalizes NT paths as returned by some low-level APIs. */\n static wchar_t *normalize_ntpath(wchar_t *wbuf)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex f31dcff2b..e9d2b9cdd 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -11,6 +11,9 @@ typedef _sigset_t sigset_t;\n #undef _POSIX_THREAD_SAFE_FUNCTIONS\n #endif\n \n+extern int mingw_core_config(const char *var, const char *value, void *cb);\n+#define platform_core_config mingw_core_config\n+\n /*\n * things that are not available in header files\n */\ndiff --git a/config.c b/config.c\nindex 3687c6783..646b6cca9 100644\n--- a/config.c\n+++ b/config.c\n@@ -1093,7 +1093,7 @@ int git_config_color(char *dest, const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_core_config(const char *var, const char *value)\n+static int git_default_core_config(const char *var, const char *value, void *cb)\n {\n \t/* This needs a better name */\n \tif (!strcmp(var, \"core.filemode\")) {\n@@ -1363,7 +1363,7 @@ static int git_default_core_config(const char *var, const char *value)\n \t}\n \n \t/* Add other config variables here and to Documentation/config.txt. */\n-\treturn 0;\n+\treturn platform_core_config(var, value, cb);\n }\n \n static int git_default_i18n_config(const char *var, const char *value)\n@@ -1451,7 +1451,7 @@ static int git_default_mailmap_config(const char *var, const char *value)\n int git_default_config(const char *var, const char *value, void *cb)\n {\n \tif (starts_with(var, \"core.\"))\n-\t\treturn git_default_core_config(var, value);\n+\t\treturn git_default_core_config(var, value, cb);\n \n \tif (starts_with(var, \"user.\"))\n \t\treturn git_ident_config(var, value, cb);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 96a3f86d8..3a08d9916 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -342,6 +342,14 @@ typedef uintmax_t timestamp_t;\n #define _PATH_DEFPATH \"/usr/local/bin:/usr/bin:/bin\"\n #endif\n \n+#ifndef platform_core_config\n+static inline int noop_core_config(const char *var, const char *value, void *cb)\n+{\n+\treturn 0;\n+}\n+#define platform_core_config noop_core_config\n+#endif\n+\n #ifndef has_dos_drive_prefix\n static inline int git_has_dos_drive_prefix(const char *path)\n {\n-- \ngitgitgadget\n\n"},{"id":"362003","messageId":"62813ef54647bb224a20208124d7d763bd9789d5.1540924800.git.gitgitgadget@gmail.com","threadId":"49717","inReplyTo":"pull.62.git.gitgitgadget@gmail.com","subject":"[PATCH 3/4] Move Windows-specific config settings into compat/mingw.c","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-30T18:40:06Z","receivedAt":"2018-10-30T18:40:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nSigned-off-by: Johannes Schindelin \n---\n cache.h | 8 --------\n compat/mingw.c | 18 ++++++++++++++++++\n config.c | 8 --------\n environment.c | 1 -\n 4 files changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f7fabdde8..0ce95c5a8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -906,14 +906,6 @@ int use_optional_locks(void);\n extern char comment_line_char;\n extern int auto_comment_line_char;\n \n-/* Windows only */\n-enum hide_dotfiles_type {\n-\tHIDE_DOTFILES_FALSE = 0,\n-\tHIDE_DOTFILES_TRUE,\n-\tHIDE_DOTFILES_DOTGITONLY\n-};\n-extern enum hide_dotfiles_type hide_dotfiles;\n-\n enum log_refs_config {\n \tLOG_REFS_UNSET = -1,\n \tLOG_REFS_NONE = 0,\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 293f286af..272d5e11e 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -6,6 +6,7 @@\n #include \"../run-command.h\"\n #include \"../cache.h\"\n #include \"win32/lazyload.h\"\n+#include \"../config.h\"\n \n #define HCAST(type, handle) ((type)(intptr_t)handle)\n \n@@ -203,8 +204,25 @@ static int ask_yes_no_if_possible(const char *format, ...)\n \t}\n }\n \n+/* Windows only */\n+enum hide_dotfiles_type {\n+\tHIDE_DOTFILES_FALSE = 0,\n+\tHIDE_DOTFILES_TRUE,\n+\tHIDE_DOTFILES_DOTGITONLY\n+};\n+\n+static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n+\n int mingw_core_config(const char *var, const char *value, void *cb)\n {\n+\tif (!strcmp(var, \"core.hidedotfiles\")) {\n+\t\tif (value && !strcasecmp(value, \"dotgitonly\"))\n+\t\t\thide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n+\t\telse\n+\t\t\thide_dotfiles = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \ndiff --git a/config.c b/config.c\nindex 646b6cca9..5bf19a23c 100644\n--- a/config.c\n+++ b/config.c\n@@ -1344,14 +1344,6 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.hidedotfiles\")) {\n-\t\tif (value && !strcasecmp(value, \"dotgitonly\"))\n-\t\t\thide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n-\t\telse\n-\t\t\thide_dotfiles = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \tif (!strcmp(var, \"core.partialclonefilter\")) {\n \t\treturn git_config_string(&core_partial_clone_filter_default,\n \t\t\t\t\t var, value);\ndiff --git a/environment.c b/environment.c\nindex 3f3c8746c..30ecd4e78 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -71,7 +71,6 @@ int core_apply_sparse_checkout;\n int merge_log_config = -1;\n int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n-enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;\n \n #ifndef PROTECT_HFS_DEFAULT\n-- \ngitgitgadget\n\n"},{"id":"362004","messageId":"567701fc798fec58e7401bdf88ee3b8dbbfe390d.1540924800.git.gitgitgadget@gmail.com","threadId":"49717","inReplyTo":"pull.62.git.gitgitgadget@gmail.com","subject":"[PATCH 4/4] mingw: unset PERL5LIB by default","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-30T18:40:07Z","receivedAt":"2018-10-30T18:40:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows ships with its own Perl interpreter, and insists on\nusing it, so it will most likely wreak havoc if PERL5LIB is set before\nlaunching Git.\n\nLet's just unset that environment variables when spawning processes.\n\nTo make this feature extensible (and overrideable), there is a new\nconfig setting `core.unsetenvvars` that allows specifying a\ncomma-separated list of names to unset before spawning processes.\n\nReported by Gabriel Fuhrmann.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 6 ++++++\n compat/mingw.c | 35 ++++++++++++++++++++++++++++++++++-\n t/t0029-core-unsetenvvars.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 70 insertions(+), 1 deletion(-)\n create mode 100755 t/t0029-core-unsetenvvars.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 09e95e9e9..f338f0b2c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -921,6 +921,12 @@ relatively high IO latencies. When enabled, Git will do the\n index comparison to the filesystem data in parallel, allowing\n overlapping IO's. Defaults to true.\n \n+core.unsetenvvars::\n+\tWindows-only: comma-separated list of environment variables'\n+\tnames that need to be unset before spawning any other process.\n+\tDefaults to `PERL5LIB` to account for the fact that Git for\n+\tWindows insists on using its own Perl interpreter.\n+\n core.createObject::\n \tYou can set this to 'link', in which case a hardlink followed by\n \ta delete of the source are used to make sure that object creation\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 272d5e11e..181e74c23 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -212,6 +212,7 @@ enum hide_dotfiles_type {\n };\n \n static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n+static char *unset_environment_variables;\n \n int mingw_core_config(const char *var, const char *value, void *cb)\n {\n@@ -223,6 +224,12 @@ int mingw_core_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.unsetenvvars\")) {\n+\t\tfree(unset_environment_variables);\n+\t\tunset_environment_variables = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1180,6 +1187,27 @@ static wchar_t *make_environment_block(char **deltaenv)\n \treturn wenvblk;\n }\n \n+static void do_unset_environment_variables(void)\n+{\n+\tstatic int done;\n+\tchar *p = unset_environment_variables;\n+\n+\tif (done || !p)\n+\t\treturn;\n+\tdone = 1;\n+\n+\tfor (;;) {\n+\t\tchar *comma = strchr(p, ',');\n+\n+\t\tif (comma)\n+\t\t\t*comma = '\\0';\n+\t\tunsetenv(p);\n+\t\tif (!comma)\n+\t\t\tbreak;\n+\t\tp = comma + 1;\n+\t}\n+}\n+\n struct pinfo_t {\n \tstruct pinfo_t *next;\n \tpid_t pid;\n@@ -1198,9 +1226,12 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen\n \twchar_t wcmd[MAX_PATH], wdir[MAX_PATH], *wargs, *wenvblk = NULL;\n \tunsigned flags = CREATE_UNICODE_ENVIRONMENT;\n \tBOOL ret;\n+\tHANDLE cons;\n+\n+\tdo_unset_environment_variables();\n \n \t/* Determine whether or not we are associated to a console */\n-\tHANDLE cons = CreateFile(\"CONOUT$\", GENERIC_WRITE,\n+\tcons = CreateFile(\"CONOUT$\", GENERIC_WRITE,\n \t\t\tFILE_SHARE_WRITE, NULL, OPEN_EXISTING,\n \t\t\tFILE_ATTRIBUTE_NORMAL, NULL);\n \tif (cons == INVALID_HANDLE_VALUE) {\n@@ -2437,6 +2468,8 @@ void mingw_startup(void)\n \t/* fix Windows specific environment settings */\n \tsetup_windows_environment();\n \n+\tunset_environment_variables = xstrdup(\"PERL5LIB\");\n+\n \t/* initialize critical section for waitpid pinfo_t list */\n \tInitializeCriticalSection(&pinfo_cs);\n \ndiff --git a/t/t0029-core-unsetenvvars.sh b/t/t0029-core-unsetenvvars.sh\nnew file mode 100755\nindex 000000000..24ce46a6e\n--- /dev/null\n+++ b/t/t0029-core-unsetenvvars.sh\n@@ -0,0 +1,30 @@\n+#!/bin/sh\n+\n+test_description='test the Windows-only core.unsetenvvars setting'\n+\n+. ./test-lib.sh\n+\n+if ! test_have_prereq MINGW\n+then\n+\tskip_all='skipping Windows-specific tests'\n+\ttest_done\n+fi\n+\n+test_expect_success 'setup' '\n+\tmkdir -p \"$TRASH_DIRECTORY/.git/hooks\" &&\n+\twrite_script \"$TRASH_DIRECTORY/.git/hooks/pre-commit\" <<-\\EOF\n+\techo $HOBBES >&2\n+\tEOF\n+'\n+\n+test_expect_success 'core.unsetenvvars works' '\n+\tHOBBES=Calvin &&\n+\texport HOBBES &&\n+\tgit commit --allow-empty -m with 2>err &&\n+\tgrep Calvin err &&\n+\tgit -c core.unsetenvvars=FINDUS,HOBBES,CALVIN \\\n+\t\tcommit --allow-empty -m without 2>err &&\n+\t! grep Calvin err\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"362055","messageId":"xmqqtvl2ye3b.fsf@gitster-ct.c.googlers.com","threadId":"49717","inReplyTo":"pull.62.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/4] mingw: prevent external PERL5LIB from interfering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-31T03:44:40Z","receivedAt":"2018-10-31T03:44:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> An alternative approach which was rejected at the time (because it\n> interfered with the then-ongoing work to compile Git for Windows using MS\n> Visual C++) would patch the make_environment_block() function to skip the\n> specified environment variables before handing down the environment block to\n> the spawned process. Currently it would interfere with the mingw-utf-8-env\n> patch series I sent earlier today\n> [https://public-inbox.org/git/pull.57.git.gitgitgadget@gmail.com].\n\nSo the rejected approach that was not used in this series would\ninterfere with the other one, but I do not have to worry about it\nbecause this series does not use that approach? I had to read the\nsix lines above twice to figure out that it essentially is saying \"I\nshouldn't care\", but please let me know if I misread the paragraph\nand I need to care ;-)\n\n> While at it, this patch series also cleans up house and moves the\n> Windows-specific core.* variable handling to compat/mingw.c rather than\n> cluttering environment.c and config.c with things that e.g. developers on\n> Linux do not want to care about.\n\nOr Macs. When I skimmed this series earlier, I found that patches 2\n& 3 sensibly implemented to achieve this goal.\n\n>\n> Johannes Schindelin (4):\n> config: rename `dummy` parameter to `cb` in git_default_config()\n> Allow for platform-specific core.* config settings\n> Move Windows-specific config settings into compat/mingw.c\n> mingw: unset PERL5LIB by default\n>\n> Documentation/config.txt | 6 ++++\n> cache.h | 8 -----\n> compat/mingw.c | 58 +++++++++++++++++++++++++++++++++++-\n> compat/mingw.h | 3 ++\n> config.c | 18 ++++-------\n> environment.c | 1 -\n> git-compat-util.h | 8 +++++\n> t/t0029-core-unsetenvvars.sh | 30 +++++++++++++++++++\n> 8 files changed, 109 insertions(+), 23 deletions(-)\n> create mode 100755 t/t0029-core-unsetenvvars.sh\n>\n>\n> base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2\n> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-62%2Fdscho%2Fperl5lib-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-62/dscho/perl5lib-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/62\n"},{"id":"362090","messageId":"nycvar.QRO.7.76.6.1810311151170.4546@tvgsbejvaqbjf.bet","threadId":"49717","inReplyTo":"xmqqtvl2ye3b.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/4] mingw: prevent external PERL5LIB from interfering","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-31T11:04:44Z","receivedAt":"2018-10-31T11:04:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 31 Oct 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > An alternative approach which was rejected at the time (because it\n> > interfered with the then-ongoing work to compile Git for Windows using\n> > MS Visual C++) would patch the make_environment_block() function to\n> > skip the specified environment variables before handing down the\n> > environment block to the spawned process. Currently it would interfere\n> > with the mingw-utf-8-env patch series I sent earlier today\n> > [https://public-inbox.org/git/pull.57.git.gitgitgadget@gmail.com].\n> \n> So the rejected approach that was not used in this series would\n> interfere with the other one, but I do not have to worry about it\n> because this series does not use that approach? I had to read the six\n> lines above twice to figure out that it essentially is saying \"I\n> shouldn't care\", but please let me know if I misread the paragraph and I\n> need to care ;-)\n\nWell, you might want to worry about it. Or not.\n\nThe approach taken by this patch series is to call `unsetenv()` for the\nvariable names listed in `core.unsetenvvars` (if any), just before\nspawning off the first process (if any).\n\nWhat I meant by this comment in the cover letter is that I thought about\ndoing it differently. We already have a perfectly fine function called\n`make_environment_block()` that takes a \"deltaenv\", and then constructs a\nnew environment block from the current environment plus deltaenv. And this\nwould be an obvious alternative place to \"unset\" the variables, as this\nfunction is only called just before spawning new processes.\n\nI was weighing both options, and both back then as right now, there are\nother patches in flight that conflict with the second approach, so the\nfirst approach is what I went with.\n\nFrom a purely aesthetic point of view, the second approach looks nicer, as\nit really only affects the spawned processes (and not the current one),\nand it allows for changing the list between spawning processes.\n\nBut to do it right, I would also have to use a hash set, and it would\ncomplicate the code of `make_environment_block()` even further. And it\nsounds a bit like overkill to me, too.\n\nSo I sided with the approach presented in the current revision of the\npatch series, but I wanted to describe the other approach in case you (or\nother reviewers) have a different opinion.\n\n> > While at it, this patch series also cleans up house and moves the\n> > Windows-specific core.* variable handling to compat/mingw.c rather\n> > than cluttering environment.c and config.c with things that e.g.\n> > developers on Linux do not want to care about.\n> \n> Or Macs.\n\nOr macOS. Or FreeBSD. Or Irix. Or any other, that's right ;-)\n\nTraditionally, we did not really care all that much about platforms other\nthan Linux, though, which is what made me write what I wrote. Having said\nthat, I get the impression that this is changing slowly. The benefits are\npretty clear, too. After all, it was a *Windows* build failure recently\nthat let Peff identify and fix a *non-Windows* bug...\n\n> When I skimmed this series earlier, I found that patches 2 & 3 sensibly\n> implemented to achieve this goal.\n\nThanks!\nDscho\n\n> \n> >\n> > Johannes Schindelin (4):\n> > config: rename `dummy` parameter to `cb` in git_default_config()\n> > Allow for platform-specific core.* config settings\n> > Move Windows-specific config settings into compat/mingw.c\n> > mingw: unset PERL5LIB by default\n> >\n> > Documentation/config.txt | 6 ++++\n> > cache.h | 8 -----\n> > compat/mingw.c | 58 +++++++++++++++++++++++++++++++++++-\n> > compat/mingw.h | 3 ++\n> > config.c | 18 ++++-------\n> > environment.c | 1 -\n> > git-compat-util.h | 8 +++++\n> > t/t0029-core-unsetenvvars.sh | 30 +++++++++++++++++++\n> > 8 files changed, 109 insertions(+), 23 deletions(-)\n> > create mode 100755 t/t0029-core-unsetenvvars.sh\n> >\n> >\n> > base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2\n> > Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-62%2Fdscho%2Fperl5lib-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-62/dscho/perl5lib-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/62\n> \n"}]}