* [PATCH v4 09/11] add.c: extract new die_if_path_beyond_symlink() for reuse
From: Adam Spiers @ 2013-01-06 16:58 UTC (permalink / raw)
To: git list
In-Reply-To: <1357491493-11619-1-git-send-email-git@adamspiers.org>
This will be reused by a new git check-ignore command.
Also document validate_pathspec().
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
Unlike v3, this series doesn't make validate_pathspec() public.
builtin/add.c | 10 ++++++----
pathspec.c | 12 ++++++++++++
pathspec.h | 1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index f95ded2..3716617 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -153,6 +153,11 @@ static void refresh(int verbose, const char **pathspec)
free(seen);
}
+/*
+ * Normalizes argv relative to prefix, via get_pathspec(), and then
+ * runs die_if_path_beyond_symlink() on each path in the normalized
+ * list.
+ */
static const char **validate_pathspec(const char **argv, const char *prefix)
{
const char **pathspec = get_pathspec(prefix, argv);
@@ -160,10 +165,7 @@ static const char **validate_pathspec(const char **argv, const char *prefix)
if (pathspec) {
const char **p;
for (p = pathspec; *p; p++) {
- if (has_symlink_leading_path(*p, strlen(*p))) {
- int len = prefix ? strlen(prefix) : 0;
- die(_("'%s' is beyond a symbolic link"), *p + len);
- }
+ die_if_path_beyond_symlink(*p, prefix);
}
}
diff --git a/pathspec.c b/pathspec.c
index 02d3344..284f397 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -87,3 +87,15 @@ const char *check_path_for_gitlink(const char *path)
}
return path;
}
+
+/*
+ * Dies if the given path refers to a file inside a symlinked
+ * directory in the index.
+ */
+void die_if_path_beyond_symlink(const char *path, const char *prefix)
+{
+ if (has_symlink_leading_path(path, strlen(path))) {
+ int len = prefix ? strlen(prefix) : 0;
+ die(_("'%s' is beyond a symbolic link"), path + len);
+ }
+}
diff --git a/pathspec.h b/pathspec.h
index bf8eb96..db0184a 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -4,5 +4,6 @@
extern char *find_pathspecs_matching_against_index(const char **pathspec);
extern void add_pathspec_matches_against_index(const char **pathspec, char *seen, int specs);
extern const char *check_path_for_gitlink(const char *path);
+extern void die_if_path_beyond_symlink(const char *path, const char *prefix);
#endif /* PATHSPEC_H */
--
1.7.11.7.33.gb8feba5
^ permalink raw reply related
* [PATCH v4 11/11] add git-check-ignore sub-command
From: Adam Spiers @ 2013-01-06 16:58 UTC (permalink / raw)
To: git list
In-Reply-To: <1357491493-11619-1-git-send-email-git@adamspiers.org>
This works in a similar manner to git-check-attr.
Thanks to Jeff King and Junio C Hamano for the idea:
http://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
Several minor improvements since v3:
- rename char *dir to slash
- fix some declaration-after-statement violations
- correctly handle and test the case where --stdin is used
but STDIN is empty
- test that files in the index are not listed as ignored
- test that the presence of a file in the working directory
doesn't impact the result of running check-ignore on that file
- stylistic tweaks
- drop the -q option to grep in the test suite
- fix potential brittleness with future tests which could call
test_expect_success_multi with parameters containing double-quotes
.gitignore | 1 +
Documentation/git-check-ignore.txt | 89 +++
Documentation/gitignore.txt | 6 +-
Documentation/technical/api-directory-listing.txt | 2 +-
Makefile | 1 +
builtin.h | 1 +
builtin/check-ignore.c | 173 ++++++
command-list.txt | 1 +
contrib/completion/git-completion.bash | 1 +
git.c | 1 +
t/t0008-ignores.sh | 632 ++++++++++++++++++++++
11 files changed, 905 insertions(+), 3 deletions(-)
create mode 100644 Documentation/git-check-ignore.txt
create mode 100644 builtin/check-ignore.c
create mode 100755 t/t0008-ignores.sh
diff --git a/.gitignore b/.gitignore
index f1acd3e..20ef4e8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,6 +19,7 @@
/git-bundle
/git-cat-file
/git-check-attr
+/git-check-ignore
/git-check-ref-format
/git-checkout
/git-checkout-index
diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt
new file mode 100644
index 0000000..854e4d0
--- /dev/null
+++ b/Documentation/git-check-ignore.txt
@@ -0,0 +1,89 @@
+git-check-ignore(1)
+===================
+
+NAME
+----
+git-check-ignore - Debug gitignore / exclude files
+
+
+SYNOPSIS
+--------
+[verse]
+'git check-ignore' [options] pathname...
+'git check-ignore' [options] --stdin < <list-of-paths>
+
+DESCRIPTION
+-----------
+
+For each pathname given via the command-line or from a file via
+`--stdin`, show the pattern from .gitignore (or other input files to
+the exclude mechanism) that decides if the pathname is excluded or
+included. Later patterns within a file take precedence over earlier
+ones.
+
+OPTIONS
+-------
+-q, --quiet::
+ Don't output anything, just set exit status. This is only
+ valid with a single pathname.
+
+-v, --verbose::
+ Also output details about the matching pattern (if any)
+ for each given pathname.
+
+--stdin::
+ Read file names from stdin instead of from the command-line.
+
+-z::
+ The output format is modified to be machine-parseable (see
+ below). If `--stdin` is also given, input paths are separated
+ with a NUL character instead of a linefeed character.
+
+OUTPUT
+------
+
+By default, any of the given pathnames which match an ignore pattern
+will be output, one per line. If no pattern matches a given path,
+nothing will be output for that path; this means that path will not be
+ignored.
+
+If `--verbose` is specified, the output is a series of lines of the form:
+
+<source> <COLON> <linenum> <COLON> <pattern> <HT> <pathname>
+
+<pathname> is the path of a file being queried, <pattern> is the
+matching pattern, <source> is the pattern's source file, and <linenum>
+is the line number of the pattern within that source. If the pattern
+contained a `!` prefix or `/` suffix, it will be preserved in the
+output. <source> will be an absolute path when referring to the file
+configured by `core.excludesfile`, or relative to the repository root
+when referring to `.git/info/exclude` or a per-directory exclude file.
+
+If `-z` is specified, the pathnames in the output are delimited by the
+null character; if `--verbose` is also specified then null characters
+are also used instead of colons and hard tabs:
+
+<source> <NULL> <linenum> <NULL> <pattern> <NULL> <pathname> <NULL>
+
+
+EXIT STATUS
+-----------
+
+0::
+ One or more of the provided paths is ignored.
+
+1::
+ None of the provided paths are ignored.
+
+128::
+ A fatal error was encountered.
+
+SEE ALSO
+--------
+linkgit:gitignore[5]
+linkgit:gitconfig[5]
+linkgit:git-ls-files[5]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 2e7328b..f401b8c 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -153,8 +153,10 @@ The second .gitignore prevents git from ignoring
SEE ALSO
--------
-linkgit:git-rm[1], linkgit:git-update-index[1],
-linkgit:gitrepository-layout[5]
+linkgit:git-rm[1],
+linkgit:git-update-index[1],
+linkgit:gitrepository-layout[5],
+linkgit:git-check-ignore[1]
GIT
---
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index fbceb62..9d3e352 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -81,6 +81,6 @@ marked. If you to exclude files, make sure you have loaded index first.
* Use `dir.entries[]`.
-* Call `free_directory()` when none of the contained elements are no longer in use.
+* Call `clear_directory()` when none of the contained elements are no longer in use.
(JC)
diff --git a/Makefile b/Makefile
index 48facad..8476fc8 100644
--- a/Makefile
+++ b/Makefile
@@ -822,6 +822,7 @@ BUILTIN_OBJS += builtin/branch.o
BUILTIN_OBJS += builtin/bundle.o
BUILTIN_OBJS += builtin/cat-file.o
BUILTIN_OBJS += builtin/check-attr.o
+BUILTIN_OBJS += builtin/check-ignore.o
BUILTIN_OBJS += builtin/check-ref-format.o
BUILTIN_OBJS += builtin/checkout-index.o
BUILTIN_OBJS += builtin/checkout.o
diff --git a/builtin.h b/builtin.h
index dffb34e..d57faf4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -58,6 +58,7 @@ extern int cmd_cat_file(int argc, const char **argv, const char *prefix);
extern int cmd_checkout(int argc, const char **argv, const char *prefix);
extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);
extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
+extern int cmd_check_ignore(int argc, const char **argv, const char *prefix);
extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
extern int cmd_cherry(int argc, const char **argv, const char *prefix);
extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
new file mode 100644
index 0000000..709535c
--- /dev/null
+++ b/builtin/check-ignore.c
@@ -0,0 +1,173 @@
+#include "builtin.h"
+#include "cache.h"
+#include "dir.h"
+#include "quote.h"
+#include "pathspec.h"
+#include "parse-options.h"
+
+static int quiet, verbose, stdin_paths;
+static const char * const check_ignore_usage[] = {
+"git check-ignore [options] pathname...",
+"git check-ignore [options] --stdin < <list-of-paths>",
+NULL
+};
+
+static int null_term_line;
+
+static const struct option check_ignore_options[] = {
+ OPT__QUIET(&quiet, N_("suppress progress reporting")),
+ OPT__VERBOSE(&verbose, N_("be verbose")),
+ OPT_GROUP(""),
+ OPT_BOOLEAN(0, "stdin", &stdin_paths,
+ N_("read file names from stdin")),
+ OPT_BOOLEAN('z', NULL, &null_term_line,
+ N_("input paths are terminated by a null character")),
+ OPT_END()
+};
+
+static void output_exclude(const char *path, struct exclude *exclude)
+{
+ char *bang = exclude->flags & EXC_FLAG_NEGATIVE ? "!" : "";
+ char *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? "/" : "";
+ if (!null_term_line) {
+ if (!verbose) {
+ write_name_quoted(path, stdout, '\n');
+ } else {
+ quote_c_style(exclude->el->src, NULL, stdout, 0);
+ printf(":%d:%s%s%s\t",
+ exclude->srcpos,
+ bang, exclude->pattern, slash);
+ quote_c_style(path, NULL, stdout, 0);
+ fputc('\n', stdout);
+ }
+ } else {
+ if (!verbose) {
+ printf("%s%c", path, '\0');
+ } else {
+ printf("%s%c%d%c%s%s%s%c%s%c",
+ exclude->el->src, '\0',
+ exclude->srcpos, '\0',
+ bang, exclude->pattern, slash, '\0',
+ path, '\0');
+ }
+ }
+}
+
+static int check_ignore(const char *prefix, const char **pathspec)
+{
+ struct dir_struct dir;
+ const char *path, *full_path;
+ char *seen;
+ int num_ignored = 0, dtype = DT_UNKNOWN, i;
+ struct path_exclude_check check;
+ struct exclude *exclude;
+
+ /* read_cache() is only necessary so we can watch out for submodules. */
+ if (read_cache() < 0)
+ die(_("index file corrupt"));
+
+ memset(&dir, 0, sizeof(dir));
+ dir.flags |= DIR_COLLECT_IGNORED;
+ setup_standard_excludes(&dir);
+
+ if (!pathspec || !*pathspec) {
+ if (!quiet)
+ fprintf(stderr, "no pathspec given.\n");
+ return 0;
+ }
+
+ path_exclude_check_init(&check, &dir);
+ /*
+ * look for pathspecs matching entries in the index, since these
+ * should not be ignored, in order to be consistent with
+ * 'git status', 'git add' etc.
+ */
+ seen = find_pathspecs_matching_against_index(pathspec);
+ for (i = 0; pathspec[i]; i++) {
+ path = pathspec[i];
+ full_path = prefix_path(prefix, prefix
+ ? strlen(prefix) : 0, path);
+ full_path = check_path_for_gitlink(full_path);
+ die_if_path_beyond_symlink(full_path, prefix);
+ if (!seen[i] && path[0]) {
+ exclude = last_exclude_matching_path(&check, full_path,
+ -1, &dtype);
+ if (exclude) {
+ if (!quiet)
+ output_exclude(path, exclude);
+ num_ignored++;
+ }
+ }
+ }
+ free(seen);
+ clear_directory(&dir);
+ path_exclude_check_clear(&check);
+
+ return num_ignored;
+}
+
+static int check_ignore_stdin_paths(const char *prefix)
+{
+ struct strbuf buf, nbuf;
+ char **pathspec = NULL;
+ size_t nr = 0, alloc = 0;
+ int line_termination = null_term_line ? 0 : '\n';
+ int num_ignored;
+
+ strbuf_init(&buf, 0);
+ strbuf_init(&nbuf, 0);
+ while (strbuf_getline(&buf, stdin, line_termination) != EOF) {
+ if (line_termination && buf.buf[0] == '"') {
+ strbuf_reset(&nbuf);
+ if (unquote_c_style(&nbuf, buf.buf, NULL))
+ die("line is badly quoted");
+ strbuf_swap(&buf, &nbuf);
+ }
+ ALLOC_GROW(pathspec, nr + 1, alloc);
+ pathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));
+ strcpy(pathspec[nr++], buf.buf);
+ }
+ ALLOC_GROW(pathspec, nr + 1, alloc);
+ pathspec[nr] = NULL;
+ num_ignored = check_ignore(prefix, (const char **)pathspec);
+ maybe_flush_or_die(stdout, "attribute to stdout");
+ strbuf_release(&buf);
+ strbuf_release(&nbuf);
+ free(pathspec);
+ return num_ignored;
+}
+
+int cmd_check_ignore(int argc, const char **argv, const char *prefix)
+{
+ int num_ignored;
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, check_ignore_options,
+ check_ignore_usage, 0);
+
+ if (stdin_paths) {
+ if (argc > 0)
+ die(_("cannot specify pathnames with --stdin"));
+ } else {
+ if (null_term_line)
+ die(_("-z only makes sense with --stdin"));
+ if (argc == 0)
+ die(_("no path specified"));
+ }
+ if (quiet) {
+ if (argc > 1)
+ die(_("--quiet is only valid with a single pathname"));
+ if (verbose)
+ die(_("cannot have both --quiet and --verbose"));
+ }
+
+ if (stdin_paths) {
+ num_ignored = check_ignore_stdin_paths(prefix);
+ } else {
+ num_ignored = check_ignore(prefix, argv);
+ maybe_flush_or_die(stdout, "ignore to stdout");
+ }
+
+ return !num_ignored;
+}
diff --git a/command-list.txt b/command-list.txt
index 14ea67a..ef7f39c 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -12,6 +12,7 @@ git-branch mainporcelain common
git-bundle mainporcelain
git-cat-file plumbinginterrogators
git-check-attr purehelpers
+git-check-ignore purehelpers
git-checkout mainporcelain common
git-checkout-index plumbingmanipulators
git-check-ref-format purehelpers
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2e1b5e1..1fb896b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -842,6 +842,7 @@ __git_list_porcelain_commands ()
archimport) : import;;
cat-file) : plumbing;;
check-attr) : plumbing;;
+ check-ignore) : plumbing;;
check-ref-format) : plumbing;;
checkout-index) : plumbing;;
commit-tree) : plumbing;;
diff --git a/git.c b/git.c
index d232de9..0b31e66 100644
--- a/git.c
+++ b/git.c
@@ -340,6 +340,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "bundle", cmd_bundle, RUN_SETUP_GENTLY },
{ "cat-file", cmd_cat_file, RUN_SETUP },
{ "check-attr", cmd_check_attr, RUN_SETUP },
+ { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
{ "check-ref-format", cmd_check_ref_format },
{ "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
{ "checkout-index", cmd_checkout_index,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
new file mode 100755
index 0000000..9b0fcd6
--- /dev/null
+++ b/t/t0008-ignores.sh
@@ -0,0 +1,632 @@
+#!/bin/sh
+
+test_description=check-ignore
+
+. ./test-lib.sh
+
+init_vars () {
+ global_excludes="$(pwd)/global-excludes"
+}
+
+enable_global_excludes () {
+ init_vars &&
+ git config core.excludesfile "$global_excludes"
+}
+
+expect_in () {
+ dest="$HOME/expected-$1" text="$2"
+ if test -z "$text"
+ then
+ >"$dest" # avoid newline
+ else
+ echo "$text" >"$dest"
+ fi
+}
+
+expect () {
+ expect_in stdout "$1"
+}
+
+expect_from_stdin () {
+ cat >"$HOME/expected-stdout"
+}
+
+test_stderr () {
+ expected="$1"
+ expect_in stderr "$1" &&
+ test_cmp "$HOME/expected-stderr" "$HOME/stderr"
+}
+
+stderr_contains () {
+ regexp="$1"
+ if grep "$regexp" "$HOME/stderr"
+ then
+ return 0
+ else
+ echo "didn't find /$regexp/ in $HOME/stderr"
+ cat "$HOME/stderr"
+ return 1
+ fi
+}
+
+stderr_empty_on_success () {
+ expect_code="$1"
+ if test $expect_code = 0
+ then
+ test_stderr ""
+ else
+ # If we expect failure then stderr might or might not be empty
+ # due to --quiet - the caller can check its contents
+ return 0
+ fi
+}
+
+test_check_ignore () {
+ args="$1" expect_code="${2:-0}" global_args="$3"
+
+ init_vars &&
+ rm -f "$HOME/stdout" "$HOME/stderr" "$HOME/cmd" &&
+ echo git $global_args check-ignore $quiet_opt $verbose_opt $args \
+ >"$HOME/cmd" &&
+ test_expect_code "$expect_code" \
+ git $global_args check-ignore $quiet_opt $verbose_opt $args \
+ >"$HOME/stdout" 2>"$HOME/stderr" &&
+ test_cmp "$HOME/expected-stdout" "$HOME/stdout" &&
+ stderr_empty_on_success "$expect_code"
+}
+
+test_expect_success_multi () {
+ prereq=
+ if test $# -eq 4
+ then
+ prereq=$1
+ shift
+ fi
+ testname="$1" expect_verbose="$2" code="$3"
+
+ expect=$( echo "$expect_verbose" | sed -e 's/.* //' )
+
+ test_expect_success $prereq "$testname" '
+ expect "$expect" &&
+ eval "$code"
+ '
+
+ for quiet_opt in '-q' '--quiet'
+ do
+ test_expect_success $prereq "$testname${quiet_opt:+ with $quiet_opt}" "
+ expect '' &&
+ $code
+ "
+ done
+ quiet_opt=
+
+ for verbose_opt in '-v' '--verbose'
+ do
+ test_expect_success $prereq "$testname${verbose_opt:+ with $verbose_opt}" "
+ expect '$expect_verbose' &&
+ $code
+ "
+ done
+ verbose_opt=
+}
+
+test_expect_success 'setup' '
+ init_vars &&
+ mkdir -p a/b/ignored-dir a/submodule b &&
+ if test_have_prereq SYMLINKS
+ then
+ ln -s b a/symlink
+ fi &&
+ (
+ cd a/submodule &&
+ git init &&
+ echo a >a &&
+ git add a &&
+ git commit -m"commit in submodule"
+ ) &&
+ git add a/submodule &&
+ cat <<-\EOF >.gitignore &&
+ one
+ ignored-*
+ EOF
+ touch {,a/}{not-ignored,ignored-{and-untracked,but-in-index}} &&
+ git add -f {,a/}ignored-but-in-index
+ cat <<-\EOF >a/.gitignore &&
+ two*
+ *three
+ EOF
+ cat <<-\EOF >a/b/.gitignore &&
+ four
+ five
+ # this comment should affect the line numbers
+ six
+ ignored-dir/
+ # and so should this blank line:
+
+ !on*
+ !two
+ EOF
+ echo "seven" >a/b/ignored-dir/.gitignore &&
+ test -n "$HOME" &&
+ cat <<-\EOF >"$global_excludes" &&
+ globalone
+ !globaltwo
+ globalthree
+ EOF
+ cat <<-\EOF >>.git/info/exclude
+ per-repo
+ EOF
+'
+
+############################################################################
+#
+# test invalid inputs
+
+test_expect_success_multi 'empty command line' '' '
+ test_check_ignore "" 128 &&
+ stderr_contains "fatal: no path specified"
+'
+
+test_expect_success_multi '--stdin with empty STDIN' '' '
+ test_check_ignore "--stdin" 1 </dev/null &&
+ if test -n "$quiet_opt"; then
+ test_stderr ""
+ else
+ test_stderr "no pathspec given."
+ fi
+'
+
+test_expect_success '-q with multiple args' '
+ expect "" &&
+ test_check_ignore "-q one two" 128 &&
+ stderr_contains "fatal: --quiet is only valid with a single pathname"
+'
+
+test_expect_success '--quiet with multiple args' '
+ expect "" &&
+ test_check_ignore "--quiet one two" 128 &&
+ stderr_contains "fatal: --quiet is only valid with a single pathname"
+'
+
+for verbose_opt in '-v' '--verbose'
+do
+ for quiet_opt in '-q' '--quiet'
+ do
+ test_expect_success "$quiet_opt $verbose_opt" "
+ expect '' &&
+ test_check_ignore '$quiet_opt $verbose_opt foo' 128 &&
+ stderr_contains 'fatal: cannot have both --quiet and --verbose'
+ "
+ done
+done
+
+test_expect_success '--quiet with multiple args' '
+ expect "" &&
+ test_check_ignore "--quiet one two" 128 &&
+ stderr_contains "fatal: --quiet is only valid with a single pathname"
+'
+
+test_expect_success_multi 'erroneous use of --' '' '
+ test_check_ignore "--" 128 &&
+ stderr_contains "fatal: no path specified"
+'
+
+test_expect_success_multi '--stdin with superfluous arg' '' '
+ test_check_ignore "--stdin foo" 128 &&
+ stderr_contains "fatal: cannot specify pathnames with --stdin"
+'
+
+test_expect_success_multi '--stdin -z with superfluous arg' '' '
+ test_check_ignore "--stdin -z foo" 128 &&
+ stderr_contains "fatal: cannot specify pathnames with --stdin"
+'
+
+test_expect_success_multi '-z without --stdin' '' '
+ test_check_ignore "-z" 128 &&
+ stderr_contains "fatal: -z only makes sense with --stdin"
+'
+
+test_expect_success_multi '-z without --stdin and superfluous arg' '' '
+ test_check_ignore "-z foo" 128 &&
+ stderr_contains "fatal: -z only makes sense with --stdin"
+'
+
+test_expect_success_multi 'needs work tree' '' '
+ (
+ cd .git &&
+ test_check_ignore "foo" 128
+ ) &&
+ stderr_contains "fatal: This operation must be run in a work tree"
+'
+
+############################################################################
+#
+# test standard ignores
+
+# First make sure that the presence of a file in the working tree
+# does not impact results, but that the presence of a file in the
+# index does.
+
+for subdir in '' 'a/'
+do
+ if test -z "$subdir"
+ then
+ where="at top-level"
+ else
+ where="in subdir $subdir"
+ fi
+
+ test_expect_success_multi "non-existent file $where not ignored" '' "
+ test_check_ignore '${subdir}non-existent' 1
+ "
+
+ test_expect_success_multi "non-existent file $where ignored" \
+ ".gitignore:1:one ${subdir}one" "
+ test_check_ignore '${subdir}one'
+ "
+
+ test_expect_success_multi "existing untracked file $where not ignored" '' "
+ test_check_ignore '${subdir}not-ignored' 1
+ "
+
+ test_expect_success_multi "existing tracked file $where not ignored" '' "
+ test_check_ignore '${subdir}ignored-but-in-index' 1
+ "
+
+ test_expect_success_multi "existing untracked file $where ignored" \
+ ".gitignore:2:ignored-* ${subdir}ignored-and-untracked" "
+ test_check_ignore '${subdir}ignored-and-untracked'
+ "
+done
+
+# Having established the above, from now on we mostly test against
+# files which do not exist in the working tree or index.
+
+test_expect_success 'sub-directory local ignore' '
+ expect "a/3-three" &&
+ test_check_ignore "a/3-three a/three-not-this-one"
+'
+
+test_expect_success 'sub-directory local ignore with --verbose' '
+ expect "a/.gitignore:2:*three a/3-three" &&
+ test_check_ignore "--verbose a/3-three a/three-not-this-one"
+'
+
+test_expect_success 'local ignore inside a sub-directory' '
+ expect "3-three" &&
+ (
+ cd a &&
+ test_check_ignore "3-three three-not-this-one"
+ )
+'
+test_expect_success 'local ignore inside a sub-directory with --verbose' '
+ expect "a/.gitignore:2:*three 3-three" &&
+ (
+ cd a &&
+ test_check_ignore "--verbose 3-three three-not-this-one"
+ )
+'
+
+test_expect_success_multi 'nested include' \
+ 'a/b/.gitignore:8:!on* a/b/one' '
+ test_check_ignore "a/b/one"
+'
+
+############################################################################
+#
+# test ignored sub-directories
+
+test_expect_success_multi 'ignored sub-directory' \
+ 'a/b/.gitignore:5:ignored-dir/ a/b/ignored-dir' '
+ test_check_ignore "a/b/ignored-dir"
+'
+
+test_expect_success 'multiple files inside ignored sub-directory' '
+ expect_from_stdin <<-\EOF &&
+ a/b/ignored-dir/foo
+ a/b/ignored-dir/twoooo
+ a/b/ignored-dir/seven
+ EOF
+ test_check_ignore "a/b/ignored-dir/foo a/b/ignored-dir/twoooo a/b/ignored-dir/seven"
+'
+
+test_expect_success 'multiple files inside ignored sub-directory with -v' '
+ expect_from_stdin <<-\EOF &&
+ a/b/.gitignore:5:ignored-dir/ a/b/ignored-dir/foo
+ a/b/.gitignore:5:ignored-dir/ a/b/ignored-dir/twoooo
+ a/b/.gitignore:5:ignored-dir/ a/b/ignored-dir/seven
+ EOF
+ test_check_ignore "-v a/b/ignored-dir/foo a/b/ignored-dir/twoooo a/b/ignored-dir/seven"
+'
+
+test_expect_success 'cd to ignored sub-directory' '
+ expect_from_stdin <<-\EOF &&
+ foo
+ twoooo
+ ../one
+ seven
+ ../../one
+ EOF
+ (
+ cd a/b/ignored-dir &&
+ test_check_ignore "foo twoooo ../one seven ../../one"
+ )
+'
+
+test_expect_success 'cd to ignored sub-directory with -v' '
+ expect_from_stdin <<-\EOF &&
+ a/b/.gitignore:5:ignored-dir/ foo
+ a/b/.gitignore:5:ignored-dir/ twoooo
+ a/b/.gitignore:8:!on* ../one
+ a/b/.gitignore:5:ignored-dir/ seven
+ .gitignore:1:one ../../one
+ EOF
+ (
+ cd a/b/ignored-dir &&
+ test_check_ignore "-v foo twoooo ../one seven ../../one"
+ )
+'
+
+############################################################################
+#
+# test handling of symlinks
+
+test_expect_success_multi SYMLINKS 'symlink' '' '
+ test_check_ignore "a/symlink" 1
+'
+
+test_expect_success_multi SYMLINKS 'beyond a symlink' '' '
+ test_check_ignore "a/symlink/foo" 128 &&
+ test_stderr "fatal: '\''a/symlink/foo'\'' is beyond a symbolic link"
+'
+
+test_expect_success_multi SYMLINKS 'beyond a symlink from subdirectory' '' '
+ (
+ cd a &&
+ test_check_ignore "symlink/foo" 128
+ ) &&
+ test_stderr "fatal: '\''symlink/foo'\'' is beyond a symbolic link"
+'
+
+############################################################################
+#
+# test handling of submodules
+
+test_expect_success_multi 'submodule' '' '
+ test_check_ignore "a/submodule/one" 128 &&
+ test_stderr "fatal: Path '\''a/submodule/one'\'' is in submodule '\''a/submodule'\''"
+'
+
+test_expect_success_multi 'submodule from subdirectory' '' '
+ (
+ cd a &&
+ test_check_ignore "submodule/one" 128
+ ) &&
+ test_stderr "fatal: Path '\''a/submodule/one'\'' is in submodule '\''a/submodule'\''"
+'
+
+############################################################################
+#
+# test handling of global ignore files
+
+test_expect_success 'global ignore not yet enabled' '
+ expect_from_stdin <<-\EOF &&
+ .git/info/exclude:7:per-repo per-repo
+ a/.gitignore:2:*three a/globalthree
+ .git/info/exclude:7:per-repo a/per-repo
+ EOF
+ test_check_ignore "-v globalone per-repo a/globalthree a/per-repo not-ignored a/globaltwo"
+'
+
+test_expect_success 'global ignore' '
+ enable_global_excludes &&
+ expect_from_stdin <<-\EOF &&
+ globalone
+ per-repo
+ globalthree
+ a/globalthree
+ a/per-repo
+ globaltwo
+ EOF
+ test_check_ignore "globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
+'
+
+test_expect_success 'global ignore with -v' '
+ enable_global_excludes &&
+ expect_from_stdin <<-EOF &&
+ $global_excludes:1:globalone globalone
+ .git/info/exclude:7:per-repo per-repo
+ $global_excludes:3:globalthree globalthree
+ a/.gitignore:2:*three a/globalthree
+ .git/info/exclude:7:per-repo a/per-repo
+ $global_excludes:2:!globaltwo globaltwo
+ EOF
+ test_check_ignore "-v globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
+'
+
+############################################################################
+#
+# test --stdin
+
+cat <<-\EOF >stdin
+ one
+ not-ignored
+ a/one
+ a/not-ignored
+ a/b/on
+ a/b/one
+ a/b/one one
+ "a/b/one two"
+ "a/b/one\"three"
+ a/b/not-ignored
+ a/b/two
+ a/b/twooo
+ globaltwo
+ a/globaltwo
+ a/b/globaltwo
+ b/globaltwo
+EOF
+cat <<-\EOF >expected-default
+ one
+ a/one
+ a/b/on
+ a/b/one
+ a/b/one one
+ a/b/one two
+ "a/b/one\"three"
+ a/b/two
+ a/b/twooo
+ globaltwo
+ a/globaltwo
+ a/b/globaltwo
+ b/globaltwo
+EOF
+cat <<-EOF >expected-verbose
+ .gitignore:1:one one
+ .gitignore:1:one a/one
+ a/b/.gitignore:8:!on* a/b/on
+ a/b/.gitignore:8:!on* a/b/one
+ a/b/.gitignore:8:!on* a/b/one one
+ a/b/.gitignore:8:!on* a/b/one two
+ a/b/.gitignore:8:!on* "a/b/one\"three"
+ a/b/.gitignore:9:!two a/b/two
+ a/.gitignore:1:two* a/b/twooo
+ $global_excludes:2:!globaltwo globaltwo
+ $global_excludes:2:!globaltwo a/globaltwo
+ $global_excludes:2:!globaltwo a/b/globaltwo
+ $global_excludes:2:!globaltwo b/globaltwo
+EOF
+
+sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
+ tr "\n" "\0" >stdin0
+sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
+ tr "\n" "\0" >expected-default0
+sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
+ tr ":\t\n" "\0" >expected-verbose0
+
+test_expect_success '--stdin' '
+ expect_from_stdin <expected-default &&
+ test_check_ignore "--stdin" <stdin
+'
+
+test_expect_success '--stdin -q' '
+ expect "" &&
+ test_check_ignore "-q --stdin" <stdin
+'
+
+test_expect_success '--stdin -v' '
+ expect_from_stdin <expected-verbose &&
+ test_check_ignore "-v --stdin" <stdin
+'
+
+for opts in '--stdin -z' '-z --stdin'
+do
+ test_expect_success "$opts" "
+ expect_from_stdin <expected-default0 &&
+ test_check_ignore '$opts' <stdin0
+ "
+
+ test_expect_success "$opts -q" "
+ expect "" &&
+ test_check_ignore '-q $opts' <stdin0
+ "
+
+ test_expect_success "$opts -v" "
+ expect_from_stdin <expected-verbose0 &&
+ test_check_ignore '-v $opts' <stdin0
+ "
+done
+
+cat <<-\EOF >stdin
+ ../one
+ ../not-ignored
+ one
+ not-ignored
+ b/on
+ b/one
+ b/one one
+ "b/one two"
+ "b/one\"three"
+ b/two
+ b/not-ignored
+ b/twooo
+ ../globaltwo
+ globaltwo
+ b/globaltwo
+ ../b/globaltwo
+EOF
+cat <<-\EOF >expected-default
+ ../one
+ one
+ b/on
+ b/one
+ b/one one
+ b/one two
+ "b/one\"three"
+ b/two
+ b/twooo
+ ../globaltwo
+ globaltwo
+ b/globaltwo
+ ../b/globaltwo
+EOF
+cat <<-EOF >expected-verbose
+ .gitignore:1:one ../one
+ .gitignore:1:one one
+ a/b/.gitignore:8:!on* b/on
+ a/b/.gitignore:8:!on* b/one
+ a/b/.gitignore:8:!on* b/one one
+ a/b/.gitignore:8:!on* b/one two
+ a/b/.gitignore:8:!on* "b/one\"three"
+ a/b/.gitignore:9:!two b/two
+ a/.gitignore:1:two* b/twooo
+ $global_excludes:2:!globaltwo ../globaltwo
+ $global_excludes:2:!globaltwo globaltwo
+ $global_excludes:2:!globaltwo b/globaltwo
+ $global_excludes:2:!globaltwo ../b/globaltwo
+EOF
+
+sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
+ tr "\n" "\0" >stdin0
+sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
+ tr "\n" "\0" >expected-default0
+sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
+ tr ":\t\n" "\0" >expected-verbose0
+
+test_expect_success '--stdin from subdirectory' '
+ expect_from_stdin <expected-default &&
+ (
+ cd a &&
+ test_check_ignore "--stdin" <../stdin
+ )
+'
+
+test_expect_success '--stdin from subdirectory with -v' '
+ expect_from_stdin <expected-verbose &&
+ (
+ cd a &&
+ test_check_ignore "--stdin -v" <../stdin
+ )
+'
+
+for opts in '--stdin -z' '-z --stdin'
+do
+ test_expect_success "$opts from subdirectory" '
+ expect_from_stdin <expected-default0 &&
+ (
+ cd a &&
+ test_check_ignore "'"$opts"'" <../stdin0
+ )
+ '
+
+ test_expect_success "$opts from subdirectory with -v" '
+ expect_from_stdin <expected-verbose0 &&
+ (
+ cd a &&
+ test_check_ignore "'"$opts"' -v" <../stdin0
+ )
+ '
+done
+
+
+test_done
--
1.7.11.7.33.gb8feba5
^ permalink raw reply related
* [PATCH v4 07/11] pathspec.c: rename newly public functions for clarity
From: Adam Spiers @ 2013-01-06 16:58 UTC (permalink / raw)
To: git list
In-Reply-To: <1357491493-11619-1-git-send-email-git@adamspiers.org>
Perform the following function renames to make it explicit that these
pathspec handling functions are for matching against the index, rather
than against a tree or the working directory.
- fill_pathspec_matches() -> add_pathspec_matches_against_index()
- find_used_pathspec() -> find_pathspecs_matching_against_index()
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
builtin/add.c | 4 ++--
pathspec.c | 17 +++++++++--------
pathspec.h | 4 ++--
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index e51ba44..8c3fdf9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -117,7 +117,7 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
*dst++ = entry;
}
dir->nr = dst - dir->entries;
- fill_pathspec_matches(pathspec, seen, specs);
+ add_pathspec_matches_against_index(pathspec, seen, specs);
return seen;
}
@@ -415,7 +415,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
path_exclude_check_init(&check, &dir);
if (!seen)
- seen = find_used_pathspec(pathspec);
+ seen = find_pathspecs_matching_against_index(pathspec);
for (i = 0; pathspec[i]; i++) {
if (!seen[i] && pathspec[i][0]
&& !file_exists(pathspec[i])) {
diff --git a/pathspec.c b/pathspec.c
index 1472af8..b73b15c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -13,9 +13,10 @@
* altogether if seen[] already only contains non-zero entries.
*
* If seen[] has not already been written to, it may make sense
- * to use find_used_pathspec() instead.
+ * to use find_pathspecs_matching_against_index() instead.
*/
-void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
+void add_pathspec_matches_against_index(const char **pathspec,
+ char *seen, int specs)
{
int num_unmatched = 0, i;
@@ -39,12 +40,12 @@ void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
/*
* Finds which of the given pathspecs match items in the index.
*
- * This is a one-shot wrapper around fill_pathspec_matches() which
- * allocates, populates, and returns a seen[] array indicating the
- * nature of the "closest" (i.e. most specific) matches which each of
- * the given pathspecs achieves against all items in the index.
+ * This is a one-shot wrapper around add_pathspec_matches_against_index()
+ * which allocates, populates, and returns a seen[] array indicating the
+ * nature of the "closest" (i.e. most specific) matches which each of the
+ * given pathspecs achieves against all items in the index.
*/
-char *find_used_pathspec(const char **pathspec)
+char *find_pathspecs_matching_against_index(const char **pathspec)
{
char *seen;
int i;
@@ -52,6 +53,6 @@ char *find_used_pathspec(const char **pathspec)
for (i = 0; pathspec[i]; i++)
; /* just counting */
seen = xcalloc(i, 1);
- fill_pathspec_matches(pathspec, seen, i);
+ add_pathspec_matches_against_index(pathspec, seen, i);
return seen;
}
diff --git a/pathspec.h b/pathspec.h
index 1cb1909..3852bc0 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -1,7 +1,7 @@
#ifndef PATHSPEC_H
#define PATHSPEC_H
-extern char *find_used_pathspec(const char **pathspec);
-extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);
+extern char *find_pathspecs_matching_against_index(const char **pathspec);
+extern void add_pathspec_matches_against_index(const char **pathspec, char *seen, int specs);
#endif /* PATHSPEC_H */
--
1.7.11.7.33.gb8feba5
^ permalink raw reply related
* [PATCH v4 08/11] add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse
From: Adam Spiers @ 2013-01-06 16:58 UTC (permalink / raw)
To: git list
In-Reply-To: <1357491493-11619-1-git-send-email-git@adamspiers.org>
Extract the body of the for loop in treat_gitlinks() into a separate
check_path_for_gitlink() function so that it can be reused elsewhere.
This paves the way for a new check-ignore sub-command.
Also document treat_gitlinks().
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
Unlike v3, this series doesn't make treat_gitlinks() public.
builtin/add.c | 24 ++++++------------------
pathspec.c | 31 +++++++++++++++++++++++++++++++
pathspec.h | 1 +
3 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 8c3fdf9..f95ded2 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -121,6 +121,10 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
return seen;
}
+/*
+ * Checks the index to see whether any path in pathspec refers to
+ * something inside a submodule. If so, dies with an error message.
+ */
static void treat_gitlinks(const char **pathspec)
{
int i;
@@ -128,24 +132,8 @@ static void treat_gitlinks(const char **pathspec)
if (!pathspec || !*pathspec)
return;
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- if (S_ISGITLINK(ce->ce_mode)) {
- int len = ce_namelen(ce), j;
- for (j = 0; pathspec[j]; j++) {
- int len2 = strlen(pathspec[j]);
- if (len2 <= len || pathspec[j][len] != '/' ||
- memcmp(ce->name, pathspec[j], len))
- continue;
- if (len2 == len + 1)
- /* strip trailing slash */
- pathspec[j] = xstrndup(ce->name, len);
- else
- die (_("Path '%s' is in submodule '%.*s'"),
- pathspec[j], len, ce->name);
- }
- }
- }
+ for (i = 0; pathspec[i]; i++)
+ pathspec[i] = check_path_for_gitlink(pathspec[i]);
}
static void refresh(int verbose, const char **pathspec)
diff --git a/pathspec.c b/pathspec.c
index b73b15c..02d3344 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -56,3 +56,34 @@ char *find_pathspecs_matching_against_index(const char **pathspec)
add_pathspec_matches_against_index(pathspec, seen, i);
return seen;
}
+
+/*
+ * Check the index to see whether path refers to a submodule, or
+ * something inside a submodule. If the former, returns the path with
+ * any trailing slash stripped. If the latter, dies with an error
+ * message.
+ */
+const char *check_path_for_gitlink(const char *path)
+{
+ int i, path_len = strlen(path);
+ for (i = 0; i < active_nr; i++) {
+ struct cache_entry *ce = active_cache[i];
+ if (S_ISGITLINK(ce->ce_mode)) {
+ int ce_len = ce_namelen(ce);
+ if (path_len <= ce_len || path[ce_len] != '/' ||
+ memcmp(ce->name, path, ce_len))
+ /* path does not refer to this
+ * submodule or anything inside it */
+ continue;
+ if (path_len == ce_len + 1) {
+ /* path refers to submodule;
+ * strip trailing slash */
+ return xstrndup(ce->name, ce_len);
+ } else {
+ die (_("Path '%s' is in submodule '%.*s'"),
+ path, ce_len, ce->name);
+ }
+ }
+ }
+ return path;
+}
diff --git a/pathspec.h b/pathspec.h
index 3852bc0..bf8eb96 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -3,5 +3,6 @@
extern char *find_pathspecs_matching_against_index(const char **pathspec);
extern void add_pathspec_matches_against_index(const char **pathspec, char *seen, int specs);
+extern const char *check_path_for_gitlink(const char *path);
#endif /* PATHSPEC_H */
--
1.7.11.7.33.gb8feba5
^ permalink raw reply related
* [PATCH v4 06/11] add.c: move pathspec matchers into new pathspec.c for reuse
From: Adam Spiers @ 2013-01-06 16:58 UTC (permalink / raw)
To: git list
In-Reply-To: <1357491493-11619-1-git-send-email-git@adamspiers.org>
Extract the following functions from builtin/add.c to pathspec.c, in
preparation for reuse by a new git check-ignore command:
- fill_pathspec_matches()
- find_used_pathspec()
The functions being extracted are not changed in any way, except
removal of the 'static' qualifier.
Also add comments documenting these newly public functions,
including clarifications that they operate on the index.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
The v3 version of this patch extracted 5 functions from add.c to
pathspec.c, two of which did not need to be extracted. Here we
use more fine-grained commits for extraction, and also wrap pathspec.h
in a PATHSPEC_H gate to avoid duplication.
Makefile | 2 ++
builtin/add.c | 34 +---------------------------------
pathspec.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
pathspec.h | 7 +++++++
4 files changed, 67 insertions(+), 33 deletions(-)
create mode 100644 pathspec.c
create mode 100644 pathspec.h
diff --git a/Makefile b/Makefile
index 13293d3..48facad 100644
--- a/Makefile
+++ b/Makefile
@@ -645,6 +645,7 @@ LIB_H += pack-refs.h
LIB_H += pack-revindex.h
LIB_H += parse-options.h
LIB_H += patch-ids.h
+LIB_H += pathspec.h
LIB_H += pkt-line.h
LIB_H += progress.h
LIB_H += prompt.h
@@ -758,6 +759,7 @@ LIB_OBJS += parse-options-cb.o
LIB_OBJS += patch-delta.o
LIB_OBJS += patch-ids.o
LIB_OBJS += path.o
+LIB_OBJS += pathspec.o
LIB_OBJS += pkt-line.o
LIB_OBJS += preload-index.o
LIB_OBJS += pretty.o
diff --git a/builtin/add.c b/builtin/add.c
index 1f62ba3..e51ba44 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -6,6 +6,7 @@
#include "cache.h"
#include "builtin.h"
#include "dir.h"
+#include "pathspec.h"
#include "exec_cmd.h"
#include "cache-tree.h"
#include "run-command.h"
@@ -97,39 +98,6 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
return !!data.add_errors;
}
-static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
-{
- int num_unmatched = 0, i;
-
- /*
- * Since we are walking the index as if we were walking the directory,
- * we have to mark the matched pathspec as seen; otherwise we will
- * mistakenly think that the user gave a pathspec that did not match
- * anything.
- */
- for (i = 0; i < specs; i++)
- if (!seen[i])
- num_unmatched++;
- if (!num_unmatched)
- return;
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);
- }
-}
-
-static char *find_used_pathspec(const char **pathspec)
-{
- char *seen;
- int i;
-
- for (i = 0; pathspec[i]; i++)
- ; /* just counting */
- seen = xcalloc(i, 1);
- fill_pathspec_matches(pathspec, seen, i);
- return seen;
-}
-
static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
{
char *seen;
diff --git a/pathspec.c b/pathspec.c
new file mode 100644
index 0000000..1472af8
--- /dev/null
+++ b/pathspec.c
@@ -0,0 +1,57 @@
+#include "cache.h"
+#include "dir.h"
+#include "pathspec.h"
+
+/*
+ * Finds which of the given pathspecs match items in the index.
+ *
+ * For each pathspec, sets the corresponding entry in the seen[] array
+ * (which should be specs items long, i.e. the same size as pathspec)
+ * to the nature of the "closest" (i.e. most specific) match found for
+ * that pathspec in the index, if it was a closer type of match than
+ * the existing entry. As an optimization, matching is skipped
+ * altogether if seen[] already only contains non-zero entries.
+ *
+ * If seen[] has not already been written to, it may make sense
+ * to use find_used_pathspec() instead.
+ */
+void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
+{
+ int num_unmatched = 0, i;
+
+ /*
+ * Since we are walking the index as if we were walking the directory,
+ * we have to mark the matched pathspec as seen; otherwise we will
+ * mistakenly think that the user gave a pathspec that did not match
+ * anything.
+ */
+ for (i = 0; i < specs; i++)
+ if (!seen[i])
+ num_unmatched++;
+ if (!num_unmatched)
+ return;
+ for (i = 0; i < active_nr; i++) {
+ struct cache_entry *ce = active_cache[i];
+ match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);
+ }
+}
+
+/*
+ * Finds which of the given pathspecs match items in the index.
+ *
+ * This is a one-shot wrapper around fill_pathspec_matches() which
+ * allocates, populates, and returns a seen[] array indicating the
+ * nature of the "closest" (i.e. most specific) matches which each of
+ * the given pathspecs achieves against all items in the index.
+ */
+char *find_used_pathspec(const char **pathspec)
+{
+ char *seen;
+ int i;
+
+ for (i = 0; pathspec[i]; i++)
+ ; /* just counting */
+ seen = xcalloc(i, 1);
+ fill_pathspec_matches(pathspec, seen, i);
+ return seen;
+}
diff --git a/pathspec.h b/pathspec.h
new file mode 100644
index 0000000..1cb1909
--- /dev/null
+++ b/pathspec.h
@@ -0,0 +1,7 @@
+#ifndef PATHSPEC_H
+#define PATHSPEC_H
+
+extern char *find_used_pathspec(const char **pathspec);
+extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);
+
+#endif /* PATHSPEC_H */
--
1.7.11.7.33.gb8feba5
^ permalink raw reply related
* [PATCH 0/4] ZIP test fixes
From: René Scharfe @ 2013-01-06 17:45 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7vwqw7mb09.fsf@alter.siamese.dyndns.org>
Fix a bug in two scripts that call unzip, use the opportunity for a small
cleanup, move all ZIP related tests out of t5000 and finally skip testing
symlinks if unzip doesn't support them.
The first one allows running t5000 with the unzip from pkgsrc manually on
NetBSD, which succeeds. The last one -- together with the archive-zip
streaming patch I sent earlier today -- makes the ZIP tests succeed on
that platform out of the box.
René
t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead
t0024, t5000: use test_lazy_prereq for UNZIP
t5000, t5002: move ZIP tests into their own script
t5002: check if unzip supports symlinks
t/t0024-crlf-archive.sh | 16 +++---
t/t5000-tar-tree.sh | 71 -----------------------
t/t5002-archive-zip.sh | 131 +++++++++++++++++++++++++++++++++++++++++++
t/t5002/infozip-symlinks.zip | Bin 0 -> 328 bytes
t/test-lib.sh | 2 +
5 files changed, 140 insertions(+), 80 deletions(-)
create mode 100755 t/t5002-archive-zip.sh
create mode 100644 t/t5002/infozip-symlinks.zip
--
1.7.12
^ permalink raw reply
* [PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead
From: René Scharfe @ 2013-01-06 17:47 UTC (permalink / raw)
To: git discussion list; +Cc: Junio C Hamano
In-Reply-To: <50E9B82D.50005@lsrfire.ath.cx>
InfoZIP's unzip takes default parameters from the environment variable
UNZIP. Unset it in the test library and use GIT_UNZIP for specifying
alternate versions of the unzip command instead.
t0024 wasn't even using variable for the actual extraction. t5000
was, but when setting it to InfoZIP's unzip it would try to extract
from itself (because it treats the contents of $UNZIP as parameters),
which failed of course.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t0024-crlf-archive.sh | 6 +++---
t/t5000-tar-tree.sh | 10 +++++-----
t/test-lib.sh | 2 ++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index ec6c1b3..080fe5c 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -3,7 +3,7 @@
test_description='respect crlf in git archive'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
+GIT_UNZIP=${GIT_UNZIP:-unzip}
test_expect_success setup '
@@ -26,7 +26,7 @@ test_expect_success 'tar archive' '
'
-"$UNZIP" -v >/dev/null 2>&1
+"$GIT_UNZIP" -v >/dev/null 2>&1
if [ $? -eq 127 ]; then
say "Skipping ZIP test, because unzip was not found"
else
@@ -37,7 +37,7 @@ test_expect_success UNZIP 'zip archive' '
git archive --format=zip HEAD >test.zip &&
- ( mkdir unzipped && cd unzipped && unzip ../test.zip ) &&
+ ( mkdir unzipped && cd unzipped && "$GIT_UNZIP" ../test.zip ) &&
test_cmp sample unzipped/sample
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..1f7593d 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,7 @@ commit id embedding:
'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
+GIT_UNZIP=${GIT_UNZIP:-unzip}
GZIP=${GZIP:-gzip}
GUNZIP=${GUNZIP:-gzip -d}
@@ -37,9 +37,9 @@ check_zip() {
dir=$1
dir_with_prefix=$dir/$2
- test_expect_success UNZIP " extract ZIP archive" "
- (mkdir $dir && cd $dir && $UNZIP ../$zipfile)
- "
+ test_expect_success UNZIP " extract ZIP archive" '
+ (mkdir $dir && cd $dir && "$GIT_UNZIP" ../$zipfile)
+ '
test_expect_success UNZIP " validate filenames" "
(cd ${dir_with_prefix}a && find .) | sort >$listfile &&
@@ -201,7 +201,7 @@ test_expect_success \
test_cmp a/substfile2 g/prefix/a/substfile2
'
-$UNZIP -v >/dev/null 2>&1
+"$GIT_UNZIP" -v >/dev/null 2>&1
if [ $? -eq 127 ]; then
say "Skipping ZIP tests, because unzip was not found"
else
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8a12cbb..d8ec408 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -85,6 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
.*_TEST
PROVE
VALGRIND
+ UNZIP
PERF_AGGREGATING_LATER
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
@@ -128,6 +129,7 @@ fi
unset CDPATH
unset GREP_OPTIONS
+unset UNZIP
case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
1|2|true)
--
1.7.12
^ permalink raw reply related
* [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP
From: René Scharfe @ 2013-01-06 17:49 UTC (permalink / raw)
To: git discussion list; +Cc: Junio C Hamano
In-Reply-To: <50E9B82D.50005@lsrfire.ath.cx>
This change makes the code smaller and we can put it at the top of
the script, its rightful place as setup code.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t0024-crlf-archive.sh | 12 +++++-------
t/t5000-tar-tree.sh | 12 +++++-------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index 080fe5c..ba397bb 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -5,6 +5,11 @@ test_description='respect crlf in git archive'
. ./test-lib.sh
GIT_UNZIP=${GIT_UNZIP:-unzip}
+test_lazy_prereq UNZIP '
+ "$GIT_UNZIP" -v >/dev/null 2>&1
+ test $? -ne 127
+'
+
test_expect_success setup '
git config core.autocrlf true &&
@@ -26,13 +31,6 @@ test_expect_success 'tar archive' '
'
-"$GIT_UNZIP" -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP test, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success UNZIP 'zip archive' '
git archive --format=zip HEAD >test.zip &&
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 1f7593d..6702157 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -31,6 +31,11 @@ GUNZIP=${GUNZIP:-gzip -d}
SUBSTFORMAT=%H%n
+test_lazy_prereq UNZIP '
+ "$GIT_UNZIP" -v >/dev/null 2>&1
+ test $? -ne 127
+'
+
check_zip() {
zipfile=$1.zip
listfile=$1.lst
@@ -201,13 +206,6 @@ test_expect_success \
test_cmp a/substfile2 g/prefix/a/substfile2
'
-"$GIT_UNZIP" -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP tests, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success \
'git archive --format=zip' \
'git archive --format=zip HEAD >d.zip'
--
1.7.12
^ permalink raw reply related
* [PATCH 3/4] t5000, t5002: move ZIP tests into their own script
From: René Scharfe @ 2013-01-06 17:51 UTC (permalink / raw)
To: git discussion list; +Cc: Junio C Hamano
In-Reply-To: <50E9B82D.50005@lsrfire.ath.cx>
This makes ZIP specific tweaks easier.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t5000-tar-tree.sh | 69 ----------------------------
t/t5002-archive-zip.sh | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 119 insertions(+), 69 deletions(-)
create mode 100755 t/t5002-archive-zip.sh
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 6702157..e7c240f 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,37 +25,11 @@ commit id embedding:
'
. ./test-lib.sh
-GIT_UNZIP=${GIT_UNZIP:-unzip}
GZIP=${GZIP:-gzip}
GUNZIP=${GUNZIP:-gzip -d}
SUBSTFORMAT=%H%n
-test_lazy_prereq UNZIP '
- "$GIT_UNZIP" -v >/dev/null 2>&1
- test $? -ne 127
-'
-
-check_zip() {
- zipfile=$1.zip
- listfile=$1.lst
- dir=$1
- dir_with_prefix=$dir/$2
-
- test_expect_success UNZIP " extract ZIP archive" '
- (mkdir $dir && cd $dir && "$GIT_UNZIP" ../$zipfile)
- '
-
- test_expect_success UNZIP " validate filenames" "
- (cd ${dir_with_prefix}a && find .) | sort >$listfile &&
- test_cmp a.lst $listfile
- "
-
- test_expect_success UNZIP " validate file contents" "
- diff -r a ${dir_with_prefix}a
- "
-}
-
test_expect_success \
'populate workdir' \
'mkdir a b c &&
@@ -206,55 +180,12 @@ test_expect_success \
test_cmp a/substfile2 g/prefix/a/substfile2
'
-test_expect_success \
- 'git archive --format=zip' \
- 'git archive --format=zip HEAD >d.zip'
-
-check_zip d
-
-test_expect_success \
- 'git archive --format=zip in a bare repo' \
- '(cd bare.git && git archive --format=zip HEAD) >d1.zip'
-
-test_expect_success \
- 'git archive --format=zip vs. the same in a bare repo' \
- 'test_cmp d.zip d1.zip'
-
-test_expect_success 'git archive --format=zip with --output' \
- 'git archive --format=zip --output=d2.zip HEAD &&
- test_cmp d.zip d2.zip'
-
-test_expect_success 'git archive with --output, inferring format' '
- git archive --output=d3.zip HEAD &&
- test_cmp d.zip d3.zip
-'
-
test_expect_success 'git archive with --output, override inferred format' '
git archive --format=tar --output=d4.zip HEAD &&
test_cmp b.tar d4.zip
'
test_expect_success \
- 'git archive --format=zip with prefix' \
- 'git archive --format=zip --prefix=prefix/ HEAD >e.zip'
-
-check_zip e prefix/
-
-test_expect_success 'git archive -0 --format=zip on large files' '
- test_config core.bigfilethreshold 1 &&
- git archive -0 --format=zip HEAD >large.zip
-'
-
-check_zip large
-
-test_expect_success 'git archive --format=zip on large files' '
- test_config core.bigfilethreshold 1 &&
- git archive --format=zip HEAD >large-compressed.zip
-'
-
-check_zip large-compressed
-
-test_expect_success \
'git archive --list outside of a git repo' \
'GIT_DIR=some/non-existing/directory git archive --list'
diff --git a/t/t5002-archive-zip.sh b/t/t5002-archive-zip.sh
new file mode 100755
index 0000000..ac9c6d4
--- /dev/null
+++ b/t/t5002-archive-zip.sh
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+test_description='git archive --format=zip test'
+
+. ./test-lib.sh
+GIT_UNZIP=${GIT_UNZIP:-unzip}
+
+SUBSTFORMAT=%H%n
+
+test_lazy_prereq UNZIP '
+ "$GIT_UNZIP" -v >/dev/null 2>&1
+ test $? -ne 127
+'
+
+check_zip() {
+ zipfile=$1.zip
+ listfile=$1.lst
+ dir=$1
+ dir_with_prefix=$dir/$2
+
+ test_expect_success UNZIP " extract ZIP archive" '
+ (mkdir $dir && cd $dir && "$GIT_UNZIP" ../$zipfile)
+ '
+
+ test_expect_success UNZIP " validate filenames" "
+ (cd ${dir_with_prefix}a && find .) | sort >$listfile &&
+ test_cmp a.lst $listfile
+ "
+
+ test_expect_success UNZIP " validate file contents" "
+ diff -r a ${dir_with_prefix}a
+ "
+}
+
+test_expect_success \
+ 'populate workdir' \
+ 'mkdir a b c &&
+ echo simple textfile >a/a &&
+ mkdir a/bin &&
+ cp /bin/sh a/bin &&
+ printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
+ printf "A not substituted O" >a/substfile2 &&
+ if test_have_prereq SYMLINKS; then
+ ln -s a a/l1
+ else
+ printf %s a > a/l1
+ fi &&
+ (p=long_path_to_a_file && cd a &&
+ for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
+ echo text >file_with_long_path) &&
+ (cd a && find .) | sort >a.lst'
+
+test_expect_success \
+ 'add ignored file' \
+ 'echo ignore me >a/ignored &&
+ echo ignored export-ignore >.git/info/attributes'
+
+test_expect_success \
+ 'add files to repository' \
+ 'find a -type f | xargs git update-index --add &&
+ find a -type l | xargs git update-index --add &&
+ treeid=`git write-tree` &&
+ echo $treeid >treeid &&
+ git update-ref HEAD $(TZ=GMT GIT_COMMITTER_DATE="2005-05-27 22:00:00" \
+ git commit-tree $treeid </dev/null)'
+
+test_expect_success \
+ 'create bare clone' \
+ 'git clone --bare . bare.git &&
+ cp .git/info/attributes bare.git/info/attributes'
+
+test_expect_success \
+ 'remove ignored file' \
+ 'rm a/ignored'
+
+test_expect_success \
+ 'git archive --format=zip' \
+ 'git archive --format=zip HEAD >d.zip'
+
+check_zip d
+
+test_expect_success \
+ 'git archive --format=zip in a bare repo' \
+ '(cd bare.git && git archive --format=zip HEAD) >d1.zip'
+
+test_expect_success \
+ 'git archive --format=zip vs. the same in a bare repo' \
+ 'test_cmp d.zip d1.zip'
+
+test_expect_success 'git archive --format=zip with --output' \
+ 'git archive --format=zip --output=d2.zip HEAD &&
+ test_cmp d.zip d2.zip'
+
+test_expect_success 'git archive with --output, inferring format' '
+ git archive --output=d3.zip HEAD &&
+ test_cmp d.zip d3.zip
+'
+
+test_expect_success \
+ 'git archive --format=zip with prefix' \
+ 'git archive --format=zip --prefix=prefix/ HEAD >e.zip'
+
+check_zip e prefix/
+
+test_expect_success 'git archive -0 --format=zip on large files' '
+ test_config core.bigfilethreshold 1 &&
+ git archive -0 --format=zip HEAD >large.zip
+'
+
+check_zip large
+
+test_expect_success 'git archive --format=zip on large files' '
+ test_config core.bigfilethreshold 1 &&
+ git archive --format=zip HEAD >large-compressed.zip
+'
+
+check_zip large-compressed
+
+test_done
--
1.7.12
^ permalink raw reply related
* [PATCH 4/4] t5002: check if unzip supports symlinks
From: René Scharfe @ 2013-01-06 17:59 UTC (permalink / raw)
To: git discussion list; +Cc: Junio C Hamano
In-Reply-To: <50E9B82D.50005@lsrfire.ath.cx>
Only add a symlink to the repository if both the filesystem and
unzip support symlinks. To check the latter, add a ZIP file
containing a symlink, created like this with InfoZIP zip 3.0:
$ echo sample text >textfile
$ ln -s textfile symlink
$ zip -y infozip-symlinks.zip textfile symlink
If we can extract it successfully, we add a symlink to the test
repository for git archive --format=zip, or otherwise skip that
step. Users can see the skipped test and perhaps run it again
with a different unzip version.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t5002-archive-zip.sh | 26 +++++++++++++++++++-------
t/t5002/infozip-symlinks.zip | Bin 0 -> 328 bytes
2 files changed, 19 insertions(+), 7 deletions(-)
create mode 100644 t/t5002/infozip-symlinks.zip
diff --git a/t/t5002-archive-zip.sh b/t/t5002-archive-zip.sh
index ac9c6d4..d35aa24 100755
--- a/t/t5002-archive-zip.sh
+++ b/t/t5002-archive-zip.sh
@@ -12,6 +12,15 @@ test_lazy_prereq UNZIP '
test $? -ne 127
'
+test_lazy_prereq UNZIP_SYMLINKS '
+ (
+ mkdir unzip-symlinks &&
+ cd unzip-symlinks &&
+ "$GIT_UNZIP" "$TEST_DIRECTORY"/t5002/infozip-symlinks.zip &&
+ test -h symlink
+ )
+'
+
check_zip() {
zipfile=$1.zip
listfile=$1.lst
@@ -40,15 +49,18 @@ test_expect_success \
cp /bin/sh a/bin &&
printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
printf "A not substituted O" >a/substfile2 &&
- if test_have_prereq SYMLINKS; then
- ln -s a a/l1
- else
- printf %s a > a/l1
- fi &&
(p=long_path_to_a_file && cd a &&
for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
- echo text >file_with_long_path) &&
- (cd a && find .) | sort >a.lst'
+ echo text >file_with_long_path)
+'
+
+test_expect_success SYMLINKS,UNZIP_SYMLINKS 'add symlink' '
+ ln -s a a/symlink_to_a
+'
+
+test_expect_success 'prepare file list' '
+ (cd a && find .) | sort >a.lst
+'
test_expect_success \
'add ignored file' \
diff --git a/t/t5002/infozip-symlinks.zip b/t/t5002/infozip-symlinks.zip
new file mode 100644
index 0000000000000000000000000000000000000000..065728c631cf1f7ab20a045a83abc3e08455eeba
GIT binary patch
literal 328
zcmWIWW@h1H0D(ty)tzkeJdg4K*&xipAj43ST2YdgnUfkC!pXp_F7Y}5gi9;985mh!
zFf%Z)qyW_wC*~I9q$+@vas|Lmdj&M@9kbsh4zNiK4D3MDiYs$-GV`**hM5Bm0%0`6
zU={{=Gcw6B<8qh;&`<^jMj&3&2x7r>g@&*~oQY;CvT2wOgO~;~=j}p2APILS&@e1c
U4De=U11V+#!r4H2I*7vn0CeC%rvLx|
literal 0
HcmV?d00001
--
1.7.12
^ permalink raw reply related
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-06 18:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marc Khouzam, git, szeder, felipe.contreras
In-Reply-To: <7vehi0qh4x.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 05/01/2013 07:27, Junio C Hamano ha scritto:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>>> I've been playing with it but I'm not getting the expected
>>> behavior when I cd to a sub-directory.
>>
>> Thanks for testing. Manlio?
>
> Can you try the attached patch?
>
Thanks, it seems to fix the problem.
> As I am not familiar with the completion machinery, take this with a
> large grain of salt. Here is my explanation of what is going on in
> this "how about this" fixup:
>
> * Giving --git-dir from the command line (or GIT_DIR environment)
> without specifying GIT_WORK_TREE is to signal Git that you are at
> the top of the working tree. "git ls-files" will then show the
> full tree even outside the real $cwd because you are lying to
> Git.
>
I was not aware of this, and blindly copied the code from the other
existing functions.
However the other completion functions never have to deal with paths in
the working directory.
I have applied the patch to my local branch.
> [...]
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDpu7UACgkQscQJ24LbaUSmUACgl+OKUyvpp183kFZGmBpOfqm1
yqEAnjxcqmZYvWSeIpOo6cNFl/dnMH76
=oE/+
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP
From: Matt Kraai @ 2013-01-06 18:06 UTC (permalink / raw)
To: René Scharfe; +Cc: git discussion list, Junio C Hamano
In-Reply-To: <50E9B90C.2060200@lsrfire.ath.cx>
On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:
> This change makes the code smaller and we can put it at the top of
> the script, its rightful place as setup code.
Would it be better to add the setting of GIT_UNZIP and
test_lazy_prereq to test-lib.sh so they aren't duplicated in both
t0024-crlf-archive.sh and t5000-tar-tree.sh, something like the
following (modulo UNZIP/GIT_UNZIP)?
--
Matt Kraai
https://ftbfs.org/kraai
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index ec6c1b3..084f33c 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -3,7 +3,6 @@
test_description='respect crlf in git archive'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
test_expect_success setup '
@@ -26,13 +25,6 @@ test_expect_success 'tar archive' '
'
-"$UNZIP" -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP test, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success UNZIP 'zip archive' '
git archive --format=zip HEAD >test.zip &&
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..85b64ae 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,6 @@ commit id embedding:
'
. ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
GZIP=${GZIP:-gzip}
GUNZIP=${GUNZIP:-gzip -d}
@@ -201,13 +200,6 @@ test_expect_success \
test_cmp a/substfile2 g/prefix/a/substfile2
'
-$UNZIP -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
- say "Skipping ZIP tests, because unzip was not found"
-else
- test_set_prereq UNZIP
-fi
-
test_expect_success \
'git archive --format=zip' \
'git archive --format=zip HEAD >d.zip'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8a12cbb..4ceabad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -752,6 +752,13 @@ test_lazy_prereq AUTOIDENT '
git var GIT_AUTHOR_IDENT
'
+UNZIP=${UNZIP:-unzip}
+
+test_lazy_prereq UNZIP '
+ "$UNZIP" -v >/dev/null 2>&1
+ test $? -ne 127
+'
+
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test -w / || test_set_prereq SANITY
^ permalink raw reply related
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-06 18:39 UTC (permalink / raw)
To: Marc Khouzam
Cc: Junio C Hamano, git@vger.kernel.org, szeder@ira.uka.de,
felipe.contreras@gmail.com
In-Reply-To: <E59706EF8DB1D147B15BECA3322E4BDC0681FA@eusaamb103.ericsson.se>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> Thanks for this, it improves the situation dramatically.
> I did further testing with your patch and found some less obvious
> issues. I didn't debug the script myself as I'm not that familiar with
> it either, but I think the testcases below should help Manlio or
> someone else look into some regressions.
>
> 1- Using .. or . breaks completion when after the '/':
> [...]
> 2- Maybe related to problem 1. Using .. breaks completion in other ways:
> [...]
> 3- Also probably related to problems 1 and 2. Using absolute paths behaves wierdly and
> worse than before:
> In my opinion, the above three cases are regressions.
>
Yes.
I did not considered this use case, thanks!
I have never done something like this when working with Mercurial.
The problem is caused by the __git_index_file_list_filter function.
The job of this function is to stop path completion at directory
boundary (in order to avoid to suggest files in child
directories [1]), and to make paths relative to current directory.
Unfortunately, what it does is to simply remove the prefix string from
the path name; of course this will not work when the prefix is a non
canonical path name.
The solution is quite simple: canonicalize both the prefix path and each
of the path name returned by git.
This can be done using `readlink -f "$path"` or `realpath $path`, but
the problem is that it is inefficient to execute an external command for
each of the path returned by git; moreover readlink and realpath are not
POSIX and may not be supported on all platforms where git works (but I
found a portable implementation using pushd, popd, `pwd`, `dirname`,
`basename` -- not very efficient).
IMHO, the best solution is to recode __git_index_file_list_filter in Perl.
Another possible solution (as suggested by Junio) is to use the
- --relative option; unfortunately this is only supported by
`git diff-index` and not by `git ls-files`.
And it will not solve the problem when using absolute path names (but
this case can be handled by leaving path completion to bash).
> 4- Completion choices include their entire path, which is not what bash does by default. For example:
>> cd git/contrib
>> ls completion/git-<tab>
> git-completion.bash git-completion.tcsh git-completion.zsh git-prompt.sh
> but
>> git rm completion/git-<tab>
> completion/git-completion.bash completion/git-completion.tcsh completion/git-completion.zsh completion/git-prompt.sh
> notice the extra 'completion/' before each completion.
This is another thing I missed.
The problem is that only the current directory is removed from the path
names returned by git.
> This can get pretty large when completing with
> many directory prefixes. The current tcsh completion has the same problem which I couldn't fix. However, I am
> not sure if it can be fixed for bash.
>
The fix was very easy, and it seems to work.
The problem is in the __git_complete_index_files and
__git_complete_diff_index_files function.
When calling the __git_index_files and git_index_files, the "$cur"
variable should be used, instead of the computed "$pfx".
Not sure if this is correct.
I will post the patch, so you can test it.
> I personally don't think this is regression, just an slight annoyance.
>
> 5- With this feature git-completion.bash will return directories as completions. This is something
> git-completion.tcsh is not handling very well. I will post a patch to fix that.
>
I'll pass on this, thanks.
> Below are two suggestions that are in line with this effort but that are not regressions.
>
> A) It would be nice if
> git commit -a <TAB>
> also completed with untracked files
>
I agree.
And there are other places when it may be useful to check the passed
options (see the comments).
But I think it is better to leave these issues for the future.
I will just add a comment to take note of this use case.
> B) Now that, for example, 'git rm' completion is smart about path completion
> it would be nice to somehow not trigger bash default file completion
> when 'git rm' does not report any completions.
>
Not sure how this can be done, but it is possible and should be easy.
> For example, if I have a file called zz.tar.gz (which is an ignored file)
> and I do 'git rm <tab>', I will get the proper list of files that can be
> removed by git, excluding zz.tar.gz. But if I complete
> 'git rm zz.tar.<tab>' then the completion script will return nothing,
> since git cannot remove that ignored file, but we will then fall-back
> to the bash default completion, which will complete the file to zz.tar.gz.
>
> Although there are some issues, I think this feature will greatly benefit the user
> and is worth the time needed to fix.
>
> Thanks!
>
> Marc
Thanks to you for the review!
Regards Manlio
[1] this is what the Mercurial bash completion script does
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDpxNEACgkQscQJ24LbaURZEgCcD2Uc+7/W+RCrMk3j+vrd5w36
6ogAn1ou4pOarBSMywaQ3zQKdZmofyKA
=iU13
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Junio C Hamano @ 2013-01-06 19:54 UTC (permalink / raw)
To: Mark Levedahl
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <20130106120917.GC22081@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Mark Levedahl wrote:
>
>> However, the newer
>> win32api is provided only for the current cygwin release series, which can
>> be reliably identified by having dll version 1.7.x, while the older frozen
>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>> older api as no updates are being made for the legacy version(s).
>
> Ah. That makes sense, thanks.
>
> (For the future, if we wanted to diagnose an out-of-date win32api and
> print a helpful message, I guess cygcheck would be the command to use.)
Hmph, so we might see somebody who cares about Cygwin to come up
with a solution based on cygcheck (not on uname) to update this
part, perhaps on top of Peff's "split default settings based on
uname into separate file" patch?
If I understood what Mark and Torsten wrote correctly, you will have
the new win32api if you install 1.7.17 (or newer) from scratch, but
if you are on older 1.7.x then you can update the win32api part as a
package update (as opposed to the whole-system upgrade). A test
based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
installation has a newer win32api because the user updated it from
the package (hence the user should not define CYGWIN_V15_WIN32API).
Am I on the same page as you guys, or am I still behind?
In the meantime, perhaps we would need something like this?
diff --git a/Makefile b/Makefile
index 8e225ca..b45b06d 100644
--- a/Makefile
+++ b/Makefile
@@ -281,6 +281,9 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
+# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
+# 1.7.x (this typically is true on Cygwin older than 1.7.17)
+#
# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
# user.
#
^ permalink raw reply related
* Re: [PATCH jk/pathspec-literal] t6130-pathspec-noglob: Windows does not allow a file named "f*"
From: Junio C Hamano @ 2013-01-06 20:00 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Git Mailing List, msysGit
In-Reply-To: <20130106143319.GA10690@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Jan 06, 2013 at 03:07:43PM +0100, Johannes Sixt wrote:
>
>> Windows disallows file names that contain a star. Arrange the test setup
>> to insert the file name "f*" in the repository without the corresponding
>> file in the worktree.
>> [...]
>> - test_commit star "f*" &&
>> + # insert file "f*" in the commit, but in a way that avoids
>> + # the name "f*" in the worktree, because it is not allowed
>> + # on Windows (the tests below do not depend on the presence
>> + # of the file in the worktree)
>> + git update-index --add --cacheinfo 100644 "$(git rev-parse HEAD:foo)" "f*" &&
>> + test_tick &&
>> + git commit -m star &&
>
> Thanks, looks obviously correct to me.
>
> Sorry to break Windows again. It seems I learn about a new gotcha with
> every patch series. :)
Thanks, both.
It appears that pushing things earlier to 'next' (and unfortunately
even to 'master') rather than later seems to be the only way to help
catching little gotchas like this ;-).
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
From: Junio C Hamano @ 2013-01-06 20:25 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <20130106152039.GA2396@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
> On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:
>> Adam Spiers <git@adamspiers.org> writes:
>>
>> > diff --git a/builtin/clean.c b/builtin/clean.c
>> > index 0c7b3d0..bd18b88 100644
>> > --- a/builtin/clean.c
>> > +++ b/builtin/clean.c
>> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>> > if (!ignored)
>> > setup_standard_excludes(&dir);
>> >
>> > + add_exclude_list(&dir, EXC_CMDL);
>> > for (i = 0; i < exclude_list.nr; i++)
>> > add_exclude(exclude_list.items[i].string, "", 0,
>> > - &dir.exclude_list[EXC_CMDL]);
>> > + &dir.exclude_list_groups[EXC_CMDL].ary[0]);
>>
>> This looks somewhat ugly for two reasons.
>>
>> * The abstraction add_exclude() offers to its callers is just to
>> let them add one pattern to the list of patterns for the kind
>> (here, EXC_CMDL); why should they care about .ary[0] part?
>
> Because the caller has to decide which exclude list the new exclude
> should be added to; that is how it has been for a long time, and I am
> not proposing we change that.
Unless I was mistaken, I never objected to the EXC_CMDL, etc
appearing in the text of the calling site of add_exclude().
The objection was about the .ary[0] bit. From the point of view of
a caller of the API, it:
- calls add_exclude_list() to declare "I now start adding new
patterns that come from a new source of patterns"; then
- calls add_exclude() repeatedly to add the patterns that come
from that source.
no? Why does the latter has to keep repeating "Here is the new
pattern for the EXC_CMDL group; it comes from the latest source I
earlier declared, by the way", instead of just "Here is the new
pattern for the EXC_CMDL group"? The ary[0] part always using "0"
(not "4" or "ix") is what repeats that "by the way".
>> Are
>> there cases any sane caller (not the implementation of the
>> exclude_list_group machinery e.g. add_excludes_from_... function)
>> may want to call it with .ary[1]?
>
> Effectively yes, although it is not written like ".ary[1]". For
> example prep_exclude() calls add_excludes_from_file_to_list() for each
> new .gitignore file
That is part of the "implementation of the machinery". If the API
for the outside callers are to call add_exclude_list() to declare
that patterns added by subsequent calls to add_exclude() are from
one new source of the patterns (e.g. .gitignore file in a new
directory level), and then call add_exclude() to add each pattern,
then the callers to add_exclude() shouldn't have to care about the
implementation detail that individual sources in exclude_list_group
is implemented as an array in that sructure, and the latest ones
should go to its ->array[0].
The implementation of the machinery may find it more convenient if
they can add one or more "sources" to an exclude_list_group before
starting to add patterns to ->array[0] or ->array[1] or ->array[2],
and a finer grained internal API that lets the caller pass an
instance of "struct exclude_list" regardless of where in an
exclude_list_group's ary[] that instance sits may be necessary to do
so.
But that does not mean other existing callers has to be aware of
such inner detail. If the implementation of the machinery needs a
helper function that adds an element to any struct exclude_list, not
necessarily the one at the tip of an exclude_list_group, we can
still do that by having the bulk of the logic in the internal, finer
grained helper, say, add_pattern_to_exclude_list(), and keep the
external API simpler by making it a thin wrapper around it, perhaps
like:
static void add_pattern_to_exclude_list(const char *pattern,
const char *base, int baselen,
struct exclude_list *el);
void add_exclude(const char *pattern,
const char *base, int baselen,
struct exclude_list_group *group) {
add_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);
}
no?
^ permalink raw reply
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Junio C Hamano @ 2013-01-06 20:29 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1357486505-21357-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> The documentation for the ALLOC_GROW API implicitly encouraged
> developers to use "ary" as the variable name for the array which is
> dynamically grown. However "ary" is an unusual abbreviation hardly
> used anywhere else in the source tree, and it is also better to name
> variables based on their contents not on their type.
Sounds good. To follow "not type but contents", a further rewrite
with s/array/item/ is even better, no?
I can obviously squash it in without resending, if you agree, or you
can point out why item[] is not a good idea and array[] is better.
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> Documentation/technical/api-allocation-growing.txt | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt
> index 43dbe09..3894815 100644
> --- a/Documentation/technical/api-allocation-growing.txt
> +++ b/Documentation/technical/api-allocation-growing.txt
> @@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.
>
> Define your array with:
>
> -* a pointer (`ary`) that points at the array, initialized to `NULL`;
> +* a pointer (`array`) that points at the array, initialized to `NULL`
> + (although please name the variable based on its contents, not on its
> + type);
>
> * an integer variable (`alloc`) that keeps track of how big the current
> allocation is, initialized to `0`;
> @@ -13,22 +15,22 @@ Define your array with:
> * another integer variable (`nr`) to keep track of how many elements the
> array currently has, initialized to `0`.
>
> -Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,
> +Then before adding `n`th element to the array, call `ALLOC_GROW(array, n,
> alloc)`. This ensures that the array can hold at least `n` elements by
> calling `realloc(3)` and adjusting `alloc` variable.
>
> ------------
> -sometype *ary;
> +sometype *array;
> size_t nr;
> size_t alloc
>
> for (i = 0; i < nr; i++)
> - if (we like ary[i] already)
> + if (we like array[i] already)
> return;
>
> /* we did not like any existing one, so add one */
> -ALLOC_GROW(ary, nr + 1, alloc);
> -ary[nr++] = value you like;
> +ALLOC_GROW(array, nr + 1, alloc);
> +array[nr++] = value you like;
> ------------
>
> You are responsible for updating the `nr` variable.
^ permalink raw reply
* Re: Re: [PATCH] Remove the suggestion to use parsecvs, which is currently broken.
From: Eric S. Raymond @ 2013-01-06 20:32 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git
In-Reply-To: <20130106163420.GA3378@book-mint>
Heiko Voigt <hvoigt@hvoigt.net>:
> > I'm parsecvs's maintainer now. It's not in good shape; there is at
> > least one other known showstopper besides the build issue. I would
> > strongly prefer to direct peoples' attention away from it until I
> > have time to fix it and cut a release. This is not a distant
> > prospect - two or three weeks out, maybe.
>
> So for this short amount of time you want to change gits documentation?
Yes. We should not direct people to a tool that plain doesn't work.
I'll fix parsecvs as soon as I can. Once I do, I will add support to the
new git-cvsimport to use parsecvs as a conversion engine, alongside
cvsps and cvs2git.
You may not have seen the first version of that patch, so I'll
explain. The new git-cvsimport can use multiple conversion engines;
each one is expressed as a Python class that knows how to convert
git-cvsimport options to engine options, and how to generate a
command that ships an import stream to standard output. There's
an -e option that selects an engine.
Currently there are two such classes, one for cvsps and one for cvs2git.
cvsps is the default. When parsecvs is working, it will be the work of
a few minutes to add a parsecvs class.
The architectural goal here is to make it easy for users of
git-cvsimport to be able to experiment with different engines to
get the best possible conversion, without having to fuss with
details of the engine invocation.
> Is this hint causing you trouble? Are there many people asking for
> support because of that?
No. But as a matter of principle I am against having documentation
tell pretty lies, even temporarily. It's bad craftsmanship and bad
faith to do that.
> There is no README so I am not sure how the tests are supposed to be
> build in general. Due to the lack of documentation its probably easier
> for you Eric to port my tests.
At the present state of things, I agree. I have been so busy fighting other
aspects of this problem that I have not yet had time to separate the
test suite from the cvsps code and document it properly.
> The structure of my tests is quite simple:
>
> t/ - All the tests
> t/cvsroot - A cvs module per test
> t/t[0-9]{4}*/expect - The expected cvsps output
>
> You can copy the cvs repository modules and convert the expected cvsps
> output to whatever output you want to test against. It the found
> changeset ordering that is interesting.
Noted. I have a copy and will port them.
> The fix was never clean and AFAIR the reason behind that was that the
> breakage in commit ordering is not easy to fix in cvsps.
Understood. But it's better than no fix at all.
> That and
> because there are other working tools out there was the reason why I
> stopped working on fixing cvsps.
Once I have all three tools working and can run them against a common
test suite, several interesting possibilities will open up.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Torsten Bögershausen @ 2013-01-06 20:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Mark Levedahl, Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vfw2enl2l.fsf@alter.siamese.dyndns.org>
On 06.01.13 20:54, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Mark Levedahl wrote:
>>
>>> However, the newer
>>> win32api is provided only for the current cygwin release series, which can
>>> be reliably identified by having dll version 1.7.x, while the older frozen
>>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>>> older api as no updates are being made for the legacy version(s).
>>
>> Ah. That makes sense, thanks.
>>
>> (For the future, if we wanted to diagnose an out-of-date win32api and
>> print a helpful message, I guess cygcheck would be the command to use.)
>
> Hmph, so we might see somebody who cares about Cygwin to come up
> with a solution based on cygcheck (not on uname) to update this
> part, perhaps on top of Peff's "split default settings based on
> uname into separate file" patch?
>
> If I understood what Mark and Torsten wrote correctly, you will have
> the new win32api if you install 1.7.17 (or newer) from scratch, but
> if you are on older 1.7.x then you can update the win32api part as a
> package update (as opposed to the whole-system upgrade). A test
> based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> installation has a newer win32api because the user updated it from
> the package (hence the user should not define CYGWIN_V15_WIN32API).
>
> Am I on the same page as you guys, or am I still behind?
>
> In the meantime, perhaps we would need something like this?
>
>
> diff --git a/Makefile b/Makefile
> index 8e225ca..b45b06d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -281,6 +281,9 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
> +# 1.7.x (this typically is true on Cygwin older than 1.7.17)
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
> # user.
> #
Hm, I haven't understood the connection between the dll (cygwin1.dll ?)
which is used in runtime, and the header files which are used when compiling.
Are they updated at the same time when updating from 1.7.16 to 1.7.17 ?
Until I updated my cygwin 1.7 (following Marks recommendation) this did the trick for me:
+ifeq ($(shell grep mingw /usr/include/w32api/winsock2.h />/dev/null 2>/dev/null && echo y),y)
+ CYGWIN_V15_WIN32API=YesPlease
+endif
As an alternative, would this be easier to read?
> +# Define CYGWIN_V15_WIN32API for Cygwin versions up to 1.7.16
^ permalink raw reply
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Adam Spiers @ 2013-01-06 20:52 UTC (permalink / raw)
To: git list
In-Reply-To: <7v38yenjgy.fsf@alter.siamese.dyndns.org>
On Sun, Jan 06, 2013 at 12:29:33PM -0800, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
> > The documentation for the ALLOC_GROW API implicitly encouraged
> > developers to use "ary" as the variable name for the array which is
> > dynamically grown. However "ary" is an unusual abbreviation hardly
> > used anywhere else in the source tree, and it is also better to name
> > variables based on their contents not on their type.
>
> Sounds good. To follow "not type but contents", a further rewrite
> with s/array/item/ is even better, no?
>
> I can obviously squash it in without resending, if you agree, or you
> can point out why item[] is not a good idea and array[] is better.
I agree.
^ permalink raw reply
* Re: [PATCH] api-allocation-growing.txt: encourage better variable naming
From: Junio C Hamano @ 2013-01-06 20:58 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <20130106205207.GA6552@pacific.linksys.moosehall>
Adam Spiers <git@adamspiers.org> writes:
>> Sounds good. To follow "not type but contents", a further rewrite
>> with s/array/item/ is even better, no?
>
> I agree.
Thanks for a quick response; let's do this then.
-- >8 --
From: Adam Spiers <git@adamspiers.org>
The documentation for the ALLOC_GROW API implicitly encouraged
developers to use "ary" as the variable name for the array which is
dynamically grown. However "ary" is an unusual abbreviation hardly
used anywhere else in the source tree, and it is also better to name
variables based on their contents not on their type.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-allocation-growing.txt | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt
index 43dbe09..542946b 100644
--- a/Documentation/technical/api-allocation-growing.txt
+++ b/Documentation/technical/api-allocation-growing.txt
@@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.
Define your array with:
-* a pointer (`ary`) that points at the array, initialized to `NULL`;
+* a pointer (`item`) that points at the array, initialized to `NULL`
+ (although please name the variable based on its contents, not on its
+ type);
* an integer variable (`alloc`) that keeps track of how big the current
allocation is, initialized to `0`;
@@ -13,22 +15,22 @@ Define your array with:
* another integer variable (`nr`) to keep track of how many elements the
array currently has, initialized to `0`.
-Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,
+Then before adding `n`th element to the item, call `ALLOC_GROW(item, n,
alloc)`. This ensures that the array can hold at least `n` elements by
calling `realloc(3)` and adjusting `alloc` variable.
------------
-sometype *ary;
+sometype *item;
size_t nr;
size_t alloc
for (i = 0; i < nr; i++)
- if (we like ary[i] already)
+ if (we like item[i] already)
return;
/* we did not like any existing one, so add one */
-ALLOC_GROW(ary, nr + 1, alloc);
-ary[nr++] = value you like;
+ALLOC_GROW(item, nr + 1, alloc);
+item[nr++] = value you like;
------------
You are responsible for updating the `nr` variable.
--
1.8.1.302.g0f4eaa7
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 21:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <7vfw2enl2l.fsf@alter.siamese.dyndns.org>
On 01/06/2013 02:54 PM, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Mark Levedahl wrote:
>>
>>> However, the newer
>>> win32api is provided only for the current cygwin release series, which can
>>> be reliably identified by having dll version 1.7.x, while the older frozen
>>> releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
>>> older api as no updates are being made for the legacy version(s).
>> Ah. That makes sense, thanks.
>>
>> (For the future, if we wanted to diagnose an out-of-date win32api and
>> print a helpful message, I guess cygcheck would be the command to use.)
> Hmph, so we might see somebody who cares about Cygwin to come up
> with a solution based on cygcheck (not on uname) to update this
> part, perhaps on top of Peff's "split default settings based on
> uname into separate file" patch?
>
> If I understood what Mark and Torsten wrote correctly, you will have
> the new win32api if you install 1.7.17 (or newer) from scratch, but
> if you are on older 1.7.x then you can update the win32api part as a
> package update (as opposed to the whole-system upgrade). A test
> based on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> installation has a newer win32api because the user updated it from
> the package (hence the user should not define CYGWIN_V15_WIN32API).
>
> Am I on the same page as you guys, or am I still behind?
>
> In the meantime, perhaps we would need something like this?
>
>
> diff --git a/Makefile b/Makefile
> index 8e225ca..b45b06d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -281,6 +281,9 @@ all::
> #
> # Define NO_REGEX if you have no or inferior regex support in your C library.
> #
> +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
> +# 1.7.x (this typically is true on Cygwin older than 1.7.17)
> +#
> # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
> # user.
> #
>
Looking at a current setup.ini, the obsolete win32 api is a single
package "w32api" with last version 3.17-2, and is now replaced by the
new win32 api is in two packages, "w32api-headers" + "w32api-runtime",
both at version 3.0b_svn5496-1. If setup.exe updated an older
installation of w32api, the old package is not deleted, but replaced by
a special "empty" package with (as of today) version 9999-1. Note that
all of this could change at any time. Also, note that the new w32api
packages have version numbers that are lower than the older obsoleted
version.
Running "cygcheck -c w32api w32api-headers w32api-runtime" on one
machine gives
Cygwin Package Information
Package Version Status
w32api 9999-1 OK
w32api-headers 3.0b_svn5496-1 OK
w32api-runtime 3.0b_svn5496-1 OK
So now, what do folks propose checking for?
a) w32api is installed? Nope - the package is not "removed", it was
updated to a special empty version to delete its former contents, but a
new fresh installation won't have this.
b) w32api-headers is installed? Nope - what happens on the next repackaging?
c) w32api version is 9999-1? Maybe, but that number could change.
etc.
There is no documented, reliable, future-proof, method of determining
the installed w32api version on Cygwin. There are many things that can
be done that will work frequently, except when they won't. I really
think the only sane thing is to follow the guidance of the Cygwin
developers: the only supported configuration is that which the current
setup.exe produces, and in the case of problems, if the installation is
not up to date then updating is the first required action.
So, in the makefile, you might add:
+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. But, the recommended approach is to
+# update your installation if compilation errors occur.
+#
Mark
^ permalink raw reply
* RE: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jason Pyeron @ 2013-01-06 21:33 UTC (permalink / raw)
To: git
In-Reply-To: <50E9E822.4020709@gmail.com>
> -----Original Message-----
> From: git-owner@vger.kernel.org
> [mailto:git-owner@vger.kernel.org] On Behalf Of Mark Levedahl
> Sent: Sunday, January 06, 2013 16:10
> To: Junio C Hamano
> Cc: Jonathan Nieder; Torsten Bögershausen; Stephen & Linda
> Smith; Jason Pyeron; git@vger.kernel.org; Eric Blake
> Subject: Re: Version 1.8.1 does not compile on Cygwin 1.7.14
>
> On 01/06/2013 02:54 PM, Junio C Hamano wrote:
> > Jonathan Nieder <jrnieder@gmail.com> writes:
> >
> >> Mark Levedahl wrote:
> >>
> >>>
> However,
> >>> the newer win32api is provided only for the current
> cygwin release
> >>> series, which can be reliably identified by having dll version
> >>> 1.7.x, while the older frozen releases (dll versions 1.6.x from
> >>> redhat, 1.5.x open source) still have the older api as no
> updates are being made for the legacy version(s).
> >> Ah. That makes sense, thanks.
> >>
> >> (For the future, if we wanted to diagnose an out-of-date
> win32api and
> >> print a helpful message, I guess cygcheck would be the command to
> >> use.)
> > Hmph, so we might see somebody who cares about Cygwin to
> come up with
> > a solution based on cygcheck (not on uname) to update this part,
> > perhaps on top of Peff's "split default settings based on
> uname into
> > separate file" patch?
> >
> > If I understood what Mark and Torsten wrote correctly, you
> will have
> > the new win32api if you install 1.7.17 (or newer) from
> scratch, but if
> > you are on older 1.7.x then you can update the win32api part as a
> > package update (as opposed to the whole-system upgrade). A
> test based
> > on "uname -r" cannot notice that an older 1.7.x (say 1.7.14)
> > installation has a newer win32api because the user updated
> it from the
> > package (hence the user should not define CYGWIN_V15_WIN32API).
> >
> > Am I on the same page as you guys, or am I still behind?
> >
> > In the meantime, perhaps we would need something like this?
> >
> >
> > diff --git a/Makefile b/Makefile
> > index 8e225ca..b45b06d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -281,6 +281,9 @@ all::
> > #
> > # Define NO_REGEX if you have no or inferior regex
> support in your C library.
> > #
> > +# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api
> dll older
> > +than # 1.7.x (this typically is true on Cygwin older than 1.7.17) #
> > # Define HAVE_DEV_TTY if your system can open /dev/tty to
> interact with the
> > # user.
> > #
> >
> Looking at a current setup.ini, the obsolete win32 api is a
> single package "w32api" with last version 3.17-2, and is now
> replaced by the new win32 api is in two packages,
> "w32api-headers" + "w32api-runtime", both at version
> 3.0b_svn5496-1. If setup.exe updated an older installation of
> w32api, the old package is not deleted, but replaced by a
> special "empty" package with (as of today) version 9999-1.
> Note that all of this could change at any time. Also, note
> that the new w32api packages have version numbers that are
> lower than the older obsoleted version.
I would not rely on that information as it is not designed to convey the
information the git build needs.
>
> Running "cygcheck -c w32api w32api-headers w32api-runtime" on
> one machine gives
>
> Cygwin Package Information
> Package Version Status
> w32api 9999-1 OK
> w32api-headers 3.0b_svn5496-1 OK
> w32api-runtime 3.0b_svn5496-1 OK
>
> So now, what do folks propose checking for?
> a) w32api is installed? Nope - the package is not "removed",
> it was updated to a special empty version to delete its
> former contents, but a new fresh installation won't have this.
> b) w32api-headers is installed? Nope - what happens on the
> next repackaging?
> c) w32api version is 9999-1? Maybe, but that number could change.
> etc.
This is what is typically done in a configure script by test compiling.
>
> There is no documented, reliable, future-proof, method of
> determining the installed w32api version on Cygwin. There are
> many things that can be done that will work frequently,
> except when they won't. I really think the only sane thing is
> to follow the guidance of the Cygwin
> developers: the only supported configuration is that which
> the current setup.exe produces, and in the case of problems,
> if the installation is not up to date then updating is the
> first required action.
>
> So, in the makefile, you might add:
>
> +# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x
> but are not
> +# using the current w32api packages. But, the recommended
> approach is
> +to # update your installation if compilation errors occur.
> +#
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100 -
- +1 (443) 269-1555 x333 Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 21:34 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Junio C Hamano, Jonathan Nieder, Stephen & Linda Smith,
Jason Pyeron, git, Eric Blake
In-Reply-To: <50E9E3C5.4070104@web.de>
On 01/06/2013 03:51 PM, Torsten Bögershausen wrote:
> Hm, I haven't understood the connection between the dll (cygwin1.dll
> ?) which is used in runtime, and the header files which are used when
> compiling. Are they updated at the same time when updating from 1.7.16
> to 1.7.17 ? Until I updated my cygwin 1.7 (following Marks
> recommendation) this did the trick for me: +ifeq ($(shell grep mingw
> /usr/include/w32api/winsock2.h />/dev/null 2>/dev/null && echo y),y) +
> CYGWIN_V15_WIN32API=YesPlease +endif As an alternative, would this be
> easier to read?
>> +# Define CYGWIN_V15_WIN32API for Cygwin versions up to 1.7.16
>
>
The cygwin distribution has a very large number of packages, each with
its own unique version number and update rhythm, just as in any linux
distro. There is no "cygwin version", just a version for each individual
package. So, "Cygwin version 1.7.16" is really nonsensical: there is
only cygwin.dll version 1.7.16. What folks are noticing is a
coincidence in the time when the cygwin dll package updated and when the
old w32api was obsoleted. uname -r reports the cygwin dll version, not
the version of any other package. Note that the cygwin api is "stable",
meaning a package compiled against the 1.7.1 dll will still run against
the current one: updating the cygwin dll does not require other packages
to update.
The only hard linkage here is that the Cygwin developers are maintaining
a legacy cygwin version (v1.5.x) as the newer dll series (v.1.7.x)
dropped support for all Windows versions predating (I think) WinXP. So,
someone on an old Windows version must use the legacy cygwin version
which has not been updated since the first v1.7 dll was released, nor
are there any plans by the developers to ever update the v1.5 packages.
Cygwin 1.5 lives in a separate distribution repository, with packages
frozen in time as of the last updates prior to going to v1.7 (packages
compiled against v1.7 will not run on v.1.5).
So, encountering a v1.5.x dll is a guarantee of using the older w32api
shared with the mingw project, rather than the current one now
maintained by the mingw64 project. However, a cygwin with any v1.7.x dll
could in theory have either w32api installed, or in theory yet another
newer one we don't know about yet. Unless and until the w32api
establishes a version number (independent of the Windows API version),
we have nothing reliable to use.
Therefore, if using the v1.7 series, *update*
Mark
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Junio C Hamano @ 2013-01-06 21:35 UTC (permalink / raw)
To: Mark Levedahl
Cc: Jonathan Nieder, Torsten Bögershausen,
Stephen & Linda Smith, Jason Pyeron, git, Eric Blake
In-Reply-To: <50E9E822.4020709@gmail.com>
Thanks; so perhaps you can give me an OK to forge your S-o-b to the
following?
-- >8 --
From: Mark Levedahl <mlevedahl@gmail.com>
Date: Sun, 6 Jan 2013 11:56:33 -0800
Subject: [PATCH] Makefile: add comment on CYGWIN_V15_WIN32API
There is no documented, reliable, and future-proof method to
determine the installed w32api version on Cygwin. There are many
things that can be done that will work frequently, except when they
won't.
The only sane thing is to follow the guidance of the Cygwin
developers: the only supported configuration is that which the
current setup.exe produces, and in the case of problems, if the
installation is not up to date then updating is the first required
action.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Makefile b/Makefile
index 4d47af5..52e298a 100644
--- a/Makefile
+++ b/Makefile
@@ -273,6 +273,10 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. The recommended approach, however,
+# is to update your installation if compilation errors occur.
+#
# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
# user.
#
--
1.8.1.302.g0f4eaa7
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox