From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
Jiang Xin <worldhello.net@gmail.com>,
Lea Wiemann <lewiemann@gmail.com>, Johannes Sixt <j6t@kdbg.org>,
git@vger.kernel.org
Subject: Re: [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized
Date: Tue, 30 Oct 2012 18:23:45 +0000 [thread overview]
Message-ID: <50901B31.7000402@ramsay1.demon.co.uk> (raw)
In-Reply-To: <1351440987-26636-7-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty wrote:
> Move the responsibility for normalizing prefixes from
> longest_ancestor_length() to its callers. Use slightly different
> normalizations at the two callers:
>
> In setup_git_directory_gently_1(), use the old normalization, which
> ignores paths that are not usable. In the next commit we will change
> this caller to also resolve symlinks in the paths from
> GIT_CEILING_DIRECTORIES as part of the normalization.
>
> In "test-path-utils longest_ancestor_length", use the old
> normalization, but die() if any paths are unusable. Also change t0060
> to only pass normalized paths to the test program (no empty entries or
> non-absolute paths, strip trailing slashes from the paths, and remove
> tests that thereby become redundant).
>
> The point of this change is to reduce the scope of the ancestor_length
> tests in t0060 from testing normalization+longest_prefix to testing
> only mostly longest_prefix. This is necessary because when
> setup_git_directory_gently_1() starts resolving symlinks as part of
> its normalization, it will not be reasonable to do the same in the
> test suite, because that would make the test results depend on the
> contents of the root directory of the filesystem on which the test is
> run. HOWEVER: under Windows, bash mangles arguments that look like
> absolute POSIX paths into DOS paths.
Just to be clear, this is true for the MinGW port to Windows, but *not*
the cygwin port.
:-P
> So we have to retain the level
> of normalization done by normalize_path_copy() to convert the
> bash-mangled DOS paths (which contain backslashes) into paths that use
> forward slashes.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> path.c | 26 +++++++++++---------------
> setup.c | 23 +++++++++++++++++++++++
> t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
> test-path-utils.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 91 insertions(+), 44 deletions(-)
>
[snip]
> diff --git a/test-path-utils.c b/test-path-utils.c
> index acb0560..0092cbf 100644
> --- a/test-path-utils.c
> +++ b/test-path-utils.c
> @@ -1,6 +1,33 @@
> #include "cache.h"
> #include "string-list.h"
>
> +/*
> + * A "string_list_each_func_t" function that normalizes an entry from
> + * GIT_CEILING_DIRECTORIES. If the path is unusable for some reason,
> + * die with an explanation.
> + */
> +static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
> +{
> + const char *ceil = item->string;
> + int len = strlen(ceil);
> + char buf[PATH_MAX+1];
> +
> + if (len == 0)
> + die("Empty path is not supported");
> + if (len > PATH_MAX)
> + die("Path \"%s\" is too long", ceil);
> + if (!is_absolute_path(ceil))
> + die("Path \"%s\" is not absolute", ceil);
> + if (normalize_path_copy(buf, ceil) < 0)
> + die("Path \"%s\" could not be normalized", ceil);
> + len = strlen(buf);
> + if (len > 1 && buf[len-1] == '/')
> + die("Normalized path \"%s\" ended with slash", buf);
> + free(item->string);
> + item->string = xstrdup(buf);
> + return 1;
> +}
> +
> int main(int argc, char **argv)
> {
> if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
> @@ -33,10 +60,26 @@ int main(int argc, char **argv)
> if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
> int len;
> struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
> + char *path = xstrdup(argv[2]);
>
> + /*
> + * We have to normalize the arguments because under
> + * Windows, bash mangles arguments that look like
ditto
> + * absolute POSIX paths or colon-separate lists of
> + * absolute POSIX paths into DOS paths (e.g.,
> + * "/foo:/foo/bar" might be converted to
> + * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"),
> + * whereas longest_ancestor_length() requires paths
> + * that use forward slashes.
> + */
> + if (normalize_path_copy(path, path))
> + die("Path \"%s\" could not be normalized", argv[2]);
> string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
> - len = longest_ancestor_length(argv[2], &ceiling_dirs);
> + filter_string_list(&ceiling_dirs, 0,
> + normalize_ceiling_entry, NULL);
> + len = longest_ancestor_length(path, &ceiling_dirs);
> string_list_clear(&ceiling_dirs, 0);
> + free(path);
> printf("%d\n", len);
> return 0;
> }
HTH
ATB,
Ramsay Jones
next prev parent reply other threads:[~2012-10-30 18:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-28 16:16 [PATCH v4 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 1/8] Introduce new static function real_path_internal() Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 2/8] real_path_internal(): add comment explaining use of cwd Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 3/8] Introduce new function real_path_if_valid() Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 4/8] longest_ancestor_length(): use string_list_split() Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 5/8] longest_ancestor_length(): take a string_list argument for prefixes Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized Michael Haggerty
2012-10-30 18:23 ` Ramsay Jones [this message]
2012-11-06 7:34 ` Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 7/8] setup_git_directory_gently_1(): resolve symlinks in ceiling paths Michael Haggerty
2012-10-28 16:16 ` [PATCH v4 8/8] string_list_longest_prefix(): remove function Michael Haggerty
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50901B31.7000402@ramsay1.demon.co.uk \
--to=ramsay@ramsay1.demon.co.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=lewiemann@gmail.com \
--cc=mhagger@alum.mit.edu \
--cc=peff@peff.net \
--cc=worldhello.net@gmail.com \
/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.
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 NNTP newsgroup(s).