Patrick Steinhardt <ps@xxxxxx> writes:
> On Wed, Aug 05, 2015 at 12:41:27PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>> > For completeness, here is what I think the end result (together with
>> > Peff's series) of the test should look like.
>> > ...
>> > Note that ssh://user:passw@rd@host:1234/ and user:passw@rd@host:/
>> > tests fail for the same reason (finding @ should be greedy, I think).
>>
>> And I think this should make it pass. Just remember the last
>> occurrence of '@' by moving the 'start' every time we see an '@'
>> sign.
>>
>> builtin/clone.c | 11 +++++------
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index cae288f..5d86439 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -160,13 +160,12 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
>> start += 3;
>>
>> /*
>> - * Skip authentication data.
>> + * Skip authentication data, if exists.
>> */
>> - ptr = start;
>> - while (ptr < end && !is_dir_sep(*ptr) && *ptr != '@')
>> - ptr++;
>> - if (*ptr == '@')
>> - start = ptr + 1;
>> + for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) {
>> + if (*ptr == '@')
>> + start = ptr + 1;
>> + }
>>
>> /*
>> * Strip trailing spaces, slashes and /.git
>
> I guess it makes sense to skip over @-signs greedily. Is there
> anything I need to do here or will you squash those changes in?
Sorry but I won't have patience to split the "these need squashing
in" into multiple pieces and add them to appropriate commits in the
5 patch series (counting Peff's two fixes on top of which you built
your 3 patch series) correctly. You as the original author are much
better equipped to do so than I am, so I'd appreciate if you can
reroll them as a 5-patch series.
Among the changes, the fix to builtin/clone.c needs to be squashed
into your "clone: do not include authentication data in guessed
dir", and the remainder of the patch to t5603 should go to Peff's
"clone: add tests for output directory", but with most of them
marked initially as failing. And then as you fix them in your
patches, some of the "fail" mark should be dropped.
Thanks.
builtin/clone.c | 11 +++++------
t/t5603-clone-dirname.sh | 14 +++++++++++++-
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index cae288f..5d86439 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -160,13 +160,12 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
start += 3;
/*
- * Skip authentication data.
+ * Skip authentication data, if exists.
*/
- ptr = start;
- while (ptr < end && !is_dir_sep(*ptr) && *ptr != '@')
- ptr++;
- if (*ptr == '@')
- start = ptr + 1;
+ for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) {
+ if (*ptr == '@')
+ start = ptr + 1;
+ }
/*
* Strip trailing spaces, slashes and /.git
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 27dbd6c..4897ea8 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -22,7 +22,8 @@ test_clone_dir () {
expect=success
bare=non-bare
clone_opts=
- for i in "$@"; do
+ for i in "$@"
+ do
case "$i" in
fail)
expect=failure
@@ -61,12 +62,23 @@ test_clone_dir ssh://host/foo/ foo
test_clone_dir ssh://host/foo.git/ foo
test_clone_dir ssh://host/foo/.git/ foo
+test_clone_dir host:foo/ foo
+test_clone_dir host:foo.git/ foo
+test_clone_dir host:foo/.git/ foo
+
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
test_clone_dir ssh://host:1234/ host
test_clone_dir ssh://user@host/ host
+test_clone_dir host:/ host
+
+# auth materials should be redacted
test_clone_dir ssh://user:password@host/ host
test_clone_dir ssh://user:password@host:1234/ host
+test_clone_dir ssh://user:passw@rd@host:1234/ host
+test_clone_dir user@host:/ host
+test_clone_dir user:password@host:/ host
+test_clone_dir user:passw@rd@host:/ host
# trailing port-like numbers should not be stripped for paths
test_clone_dir ssh://user:password@host/test:1234 1234
--
2.5.0-423-ga363579
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html