From: Karsten Blees <blees@dcon.de>
To: git@vger.kernel.org
Subject: Re: [PATCH 3/3] Windows: teach getenv to do a case-sensitive search
Date: Wed, 5 Oct 2011 16:21:14 +0000 (UTC) [thread overview]
Message-ID: <loom.20111005T175934-633@post.gmane.org> (raw)
In-Reply-To: 4DEC7CDD.10403@viscovery.net
Hmm...this looks like a pretty fragile solution to me. Wouldn't it be simpler
and safer to just fix the conflicting variables, instead of inventing entirely
new environment semantics for Windows?
I looked at the eval_gettext occurences, and only found '$path' in
git-submodule.sh to obviously conflict with existing environment variables. So
the straightforward solution IMO would be to fix the variable in that script.
Small solution (only affects gettext): in git-submodule.sh, replace all
'eval_gettext...$path' with 'modulepath=$path eval_gettext...$modulepath'
Big solution (enables git-submodule-foreach scripts on Windows, but is a
breaking change for existing foreach scripts on Unix): in git-submodule.sh,
t/t7407-submodule-foreach.sh, Documentation/git-submodule.txt, replace all
'$path' with '$modulepath' (also a few 'path=...' and 'while read...path')
Just a few failure scenarios that come to mind with the current solution:
- Given environment variables "Path" and "path", the 'case-sensitive first'
approach works fine for 'getenv("path")', but 'getenv("PATH")' still has a 50%
chance of failure.
- The other environment functions have not been changed to reflect the
'case-sensitive first' logic: setenv("path"...) and setenv("PATH"...) both have
a chance of overwriting the wrong entry, same for putenv.
- Windows applications generally don't support case-sensitive environment
variables, e.g. all MSYS and Cygwin programs convert environment variable names
to upper case on startup, eliminating duplicates in the process. With git.exe
beeing the only case-sensitive tool, any change to git-sh-i18n.sh (e.g.
replacing git-envsubst with a real gettext-envsubst) is likely to break again.
- As you already mentioned, Windows doesn't support case-sensitive environment
variable names, MSDN is pretty clear on that. Expressly violating this
documentation may cease to work with any new Windows version or patch.
Cheers,
Karsten
prev parent reply other threads:[~2011-10-05 16:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-06 6:57 [PATCH 0/3] sh-i18n--envsubst; case-sensitive getenv on Windows Johannes Sixt
2011-06-06 6:59 ` [PATCH 1/3] sh-i18n--envsubst: do not crash when no arguments are given Johannes Sixt
2011-06-06 7:06 ` [PATCH 2/3] mingw.c: move definition of mingw_getenv down Johannes Sixt
2011-06-06 7:08 ` [PATCH 3/3] Windows: teach getenv to do a case-sensitive search Johannes Sixt
2011-10-05 16:21 ` Karsten Blees [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=loom.20111005T175934-633@post.gmane.org \
--to=blees@dcon.de \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).