From: Johannes Sixt <j6t@kdbg.org>
To: Johannes Schindelin <johannes.schindelin@gmx.de>,
Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Lars Schneider <larsxschneider@gmail.com>,
Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>,
Eric Wong <normalperson@yhbt.net>
Subject: Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
Date: Mon, 28 Mar 2016 09:58:45 +0200 [thread overview]
Message-ID: <56F8E435.3020304@kdbg.org> (raw)
In-Reply-To: <8beb1c208e33e1de8f272caa22fb7a0b662ca4cc.1458668543.git.johannes.schindelin@gmx.de>
Am 22.03.2016 um 18:42 schrieb Johannes Schindelin:
> On Windows, the backslash is the native directory separator, but all
> supported Windows versions also accept the forward slash in most
> circumstances.
>
> Our tests expect forward slashes.
>
> Relative paths are generated by Git using forward slashes.
>
> So let's try to be consistent and use forward slashes in the $HOME part
> of the paths reported by `git config --show-origin`, too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> compat/mingw.h | 6 ++++++
> path.c | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 8c5bf50..c008694 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -396,6 +396,12 @@ static inline char *mingw_find_last_dir_sep(const char *path)
> ret = (char *)path;
> return ret;
> }
> +static inline void convert_slashes(char *path)
> +{
> + for (; *path; path++)
> + if (*path == '\\')
> + *path = '/';
> +}
> #define find_last_dir_sep mingw_find_last_dir_sep
> int mingw_offset_1st_component(const char *path);
> #define offset_1st_component mingw_offset_1st_component
> diff --git a/path.c b/path.c
> index 8b7e168..969b494 100644
> --- a/path.c
> +++ b/path.c
> @@ -584,6 +584,9 @@ char *expand_user_path(const char *path)
> if (!home)
> goto return_null;
> strbuf_addstr(&user_path, home);
> +#ifdef GIT_WINDOWS_NATIVE
> + convert_slashes(user_path.buf);
> +#endif
> } else {
> struct passwd *pw = getpw_str(username, username_len);
> if (!pw)
>
This change is not warranted for the following reasons:
- It is only there to satisfy the --show-origin tests, but not
for the benefit of the users.
- The use of $HOME in those tests is just incidental, but not necessary.
- There is no reason to change only paths depending on $HOME,
but no other paths imported through the environment.
I see no advantage for the users of --show-origin that they now
see C:/foo/bar instead of C:\foo\bar. (For this reason, I'm not
suggesting to change all paths imported from the environment, just
the contrary, to leave them all alone).
A change like this whould have been preferable:
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6767da8..4c96044 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1209,7 +1209,7 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '
HOME="$(pwd)" # convert to Windows path
test_expect_success 'set up --show-origin tests' '
- INCLUDE_DIR="$HOME/include" &&
+ INCLUDE_DIR="$(pwd)/include" &&
mkdir -p "$INCLUDE_DIR" &&
cat >"$INCLUDE_DIR"/absolute.include <<-\EOF &&
[user]
@@ -1219,7 +1219,7 @@ test_expect_success 'set up --show-origin tests' '
[user]
relative = include
EOF
- cat >"$HOME"/.gitconfig <<-EOF &&
+ cat >"$(pwd)"/.gitconfig <<-EOF &&
[user]
global = true
override = global
@@ -1237,9 +1237,9 @@ test_expect_success 'set up --show-origin tests' '
test_expect_success '--show-origin with --list' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfig user.global=true
- file:$HOME/.gitconfig user.override=global
- file:$HOME/.gitconfig include.path=$INCLUDE_DIR/absolute.include
+ file:$(pwd)/.gitconfig user.global=true
+ file:$(pwd)/.gitconfig user.override=global
+ file:$(pwd)/.gitconfig include.path=$INCLUDE_DIR/absolute.include
file:$INCLUDE_DIR/absolute.include user.absolute=include
file:.git/config user.local=true
file:.git/config user.override=local
@@ -1253,9 +1253,9 @@ test_expect_success '--show-origin with --list' '
test_expect_success '--show-origin with --list --null' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfigQuser.global
- trueQfile:$HOME/.gitconfigQuser.override
- globalQfile:$HOME/.gitconfigQinclude.path
+ file:$(pwd)/.gitconfigQuser.global
+ trueQfile:$(pwd)/.gitconfigQuser.override
+ globalQfile:$(pwd)/.gitconfigQinclude.path
$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute
includeQfile:.git/configQuser.local
trueQfile:.git/configQuser.override
@@ -1284,7 +1284,7 @@ test_expect_success '--show-origin with single file' '
test_expect_success '--show-origin with --get-regexp' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfig user.global true
+ file:$(pwd)/.gitconfig user.global true
file:.git/config user.local true
EOF
git config --show-origin --get-regexp "user\.[g|l].*" >output &&
But it seems I'm late in the game. Can't I take a break for a week
without something odd happening to the Windows port...?
-- Hannes
next prev parent reply other threads:[~2016-03-28 7:58 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-22 17:42 [PATCH 0/4] Git for Windows fixes in preparation for 2.8.0 Johannes Schindelin
2016-03-22 17:42 ` [PATCH 1/4] config --show-origin: report paths with forward slashes Johannes Schindelin
2016-03-22 17:58 ` Junio C Hamano
2016-03-23 8:20 ` Johannes Schindelin
2016-03-23 16:34 ` Junio C Hamano
2016-03-28 7:58 ` Johannes Sixt [this message]
2016-03-28 15:14 ` Johannes Schindelin
2016-03-29 19:18 ` Johannes Sixt
2016-03-29 20:05 ` Junio C Hamano
2016-04-02 19:03 ` Johannes Sixt
2016-04-04 15:51 ` Junio C Hamano
2016-04-04 20:42 ` Johannes Schindelin
2016-03-30 5:52 ` Johannes Sixt
2016-04-02 18:51 ` Johannes Sixt
2016-03-22 17:42 ` [PATCH 2/4] Make t1300-repo-config resilient to being run via 'sh -x' Johannes Schindelin
2016-03-22 17:59 ` Jonathan Nieder
2016-03-22 20:34 ` Junio C Hamano
2016-03-22 23:45 ` Jonathan Nieder
2016-03-23 7:21 ` Johannes Schindelin
2016-03-22 18:01 ` Junio C Hamano
2016-03-23 8:22 ` Johannes Schindelin
2016-03-22 17:42 ` [PATCH 3/4] t1300: fix the new --show-origin tests on Windows Johannes Schindelin
2016-03-22 18:13 ` Junio C Hamano
2016-03-23 10:42 ` Johannes Schindelin
2016-03-22 17:43 ` [PATCH 4/4] mingw: skip some tests in t9115 due to file name issues Johannes Schindelin
2016-03-22 18:03 ` Jonathan Nieder
2016-03-22 18:30 ` Torsten Bögershausen
2016-03-22 20:44 ` Junio C Hamano
2016-03-22 22:44 ` Torsten Bögershausen
2016-03-22 22:57 ` Junio C Hamano
2016-03-23 5:54 ` Torsten Bögershausen
2016-03-23 10:49 ` Johannes Schindelin
2016-03-23 15:56 ` Junio C Hamano
2016-03-23 19:08 ` Torsten Bögershausen
2016-03-23 22:44 ` Torsten Bögershausen
2016-03-22 17:50 ` [PATCH 0/4] Git for Windows fixes in preparation for 2.8.0 Junio C Hamano
2016-03-23 10:54 ` [PATCH v2 " Johannes Schindelin
2016-03-23 10:55 ` [PATCH v2 1/4] config --show-origin: report paths with forward slashes Johannes Schindelin
2016-03-23 10:55 ` [PATCH v2 2/4] Make t1300-repo-config resilient to being run via 'sh -x' Johannes Schindelin
2016-03-23 10:55 ` [PATCH v2 3/4] t1300: fix the new --show-origin tests on Windows Johannes Schindelin
2016-03-23 11:08 ` Lars Schneider
2016-03-23 10:55 ` [PATCH v2 4/4] mingw: skip some tests in t9115 due to file name issues Johannes Schindelin
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=56F8E435.3020304@kdbg.org \
--to=j6t@kdbg.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=k_satoda@f2.dion.ne.jp \
--cc=larsxschneider@gmail.com \
--cc=normalperson@yhbt.net \
/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).