* [PATCH v4 4/4] mergetool: honor -O<orderfile>
From: David Aguilar @ 2016-10-08 0:01 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
Teach mergetool to pass "-O<orderfile>" down to `git diff` when
specified on the command-line.
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Since v3:
I missed one last piped invocation of "git mergetool" in the tests,
which has been fixed.
Documentation/git-mergetool.txt | 10 ++++++----
git-mergetool.sh | 9 +++++++--
t/t7610-mergetool.sh | 27 +++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
-DIFF ORDER FILES
-----------------
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`. See linkgit:git-config[1] for more details.
+-O<orderfile>::
+ Process files in the order specified in the
+ <orderfile>, which has one shell glob pattern per line.
+ This overrides the `diff.orderFile` configuration variable
+ (see linkgit:git-config[1]). To cancel `diff.orderFile`,
+ use `-O/dev/null`.
TEMPORARY FILES
---------------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..e52b4e4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
+ orderfile=
while test $# != 0
do
@@ -419,6 +420,9 @@ main () {
--prompt)
prompt=true
;;
+ -O*)
+ orderfile="$1"
+ ;;
--)
shift
break
@@ -460,7 +464,8 @@ main () {
fi
files=$(git -c core.quotePath=false \
- diff --name-only --diff-filter=U -- "$@")
+ diff --name-only --diff-filter=U \
+ ${orderfile:+"$orderfile"} -- "$@")
cd_to_toplevel
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 38c1e4d..6d9f215 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -638,5 +638,32 @@ test_expect_success 'diff.orderFile configuration is honored' '
test_cmp expect actual &&
git reset --hard >/dev/null
'
+test_expect_success 'mergetool -Oorder-file is honored' '
+ test_config diff.orderFile order-file &&
+ test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+ test_config mergetool.myecho.trustExitCode true &&
+ test_must_fail git merge order-file-side1 &&
+ cat >expect <<-\EOF &&
+ Merging:
+ a
+ b
+ EOF
+ git mergetool -O/dev/null --no-prompt --tool myecho >output &&
+ git grep --no-index -h -A2 Merging: output >actual &&
+ test_cmp expect actual &&
+ git reset --hard >/dev/null 2>&1 &&
+
+ git config --unset diff.orderFile &&
+ test_must_fail git merge order-file-side1 &&
+ cat >expect <<-\EOF &&
+ Merging:
+ b
+ a
+ EOF
+ git mergetool -Oorder-file --no-prompt --tool myecho >output &&
+ git grep --no-index -h -A2 Merging: output >actual &&
+ test_cmp expect actual &&
+ git reset --hard >/dev/null 2>&1
+'
test_done
--
2.10.1.386.g8ee99a0
^ permalink raw reply related
* [PATCH v3 4/4] mergetool: honor -O<orderfile>
From: David Aguilar @ 2016-10-07 23:58 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <20161007235806.22247-1-davvid@gmail.com>
Teach mergetool to pass "-O<orderfile>" down to `git diff` when
specified on the command-line.
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Changes since v2:
The tests no longer rely on "grep -A" and instead use "git grep"
for portability. The mergetool output in the test is redirected
to a file so that we can catch its exit code.
The $orderfile variable is now passed using ${xxx:+"$xxx"}
to avoid conditional logic.
Documentation/git-mergetool.txt | 10 ++++++----
git-mergetool.sh | 9 +++++++--
t/t7610-mergetool.sh | 27 +++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
-DIFF ORDER FILES
-----------------
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`. See linkgit:git-config[1] for more details.
+-O<orderfile>::
+ Process files in the order specified in the
+ <orderfile>, which has one shell glob pattern per line.
+ This overrides the `diff.orderFile` configuration variable
+ (see linkgit:git-config[1]). To cancel `diff.orderFile`,
+ use `-O/dev/null`.
TEMPORARY FILES
---------------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..e52b4e4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
+ orderfile=
while test $# != 0
do
@@ -419,6 +420,9 @@ main () {
--prompt)
prompt=true
;;
+ -O*)
+ orderfile="$1"
+ ;;
--)
shift
break
@@ -460,7 +464,8 @@ main () {
fi
files=$(git -c core.quotePath=false \
- diff --name-only --diff-filter=U -- "$@")
+ diff --name-only --diff-filter=U \
+ ${orderfile:+"$orderfile"} -- "$@")
cd_to_toplevel
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 38c1e4d..5cfad76 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -638,5 +638,32 @@ test_expect_success 'diff.orderFile configuration is honored' '
test_cmp expect actual &&
git reset --hard >/dev/null
'
+test_expect_success 'mergetool -Oorder-file is honored' '
+ test_config diff.orderFile order-file &&
+ test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+ test_config mergetool.myecho.trustExitCode true &&
+ test_must_fail git merge order-file-side1 &&
+ cat >expect <<-\EOF &&
+ Merging:
+ a
+ b
+ EOF
+ git mergetool -O/dev/null --no-prompt --tool myecho |
+ grep -A 2 Merging: >actual &&
+ test_cmp expect actual &&
+ git reset --hard >/dev/null 2>&1 &&
+
+ git config --unset diff.orderFile &&
+ test_must_fail git merge order-file-side1 &&
+ cat >expect <<-\EOF &&
+ Merging:
+ b
+ a
+ EOF
+ git mergetool -Oorder-file --no-prompt --tool myecho >output &&
+ git grep --no-index -h -A2 Merging: output >actual &&
+ test_cmp expect actual &&
+ git reset --hard >/dev/null 2>&1
+'
test_done
--
2.10.1.386.g8ee99a0
^ permalink raw reply related
* [PATCH v3 3/4] mergetool: honor diff.orderFile
From: David Aguilar @ 2016-10-07 23:58 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <20161007235806.22247-1-davvid@gmail.com>
Teach mergetool to get the list of files to edit via `diff` so that we
gain support for diff.orderFile.
Suggested-by: Luis Gutierrez <luisgutz@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Changes since v2:
The tests no longer rely on "grep -A" and instead use "git grep"
for portability. The mergetool output in the test is redirected
to a file so that we can catch its exit code.
Documentation/git-mergetool.txt | 5 +++++
git-mergetool.sh | 30 +++++++++++++++---------------
t/t7610-mergetool.sh | 33 +++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..c4c1a9b 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,11 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+DIFF ORDER FILES
+----------------
+`git mergetool` honors the `diff.orderFile` configuration variable
+used by `git diff`. See linkgit:git-config[1] for more details.
+
TEMPORARY FILES
---------------
`git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b2cd0a4..65696d8 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -382,6 +382,11 @@ prompt_after_failed_merge () {
done
}
+print_noop_and_exit () {
+ echo "No files need merging"
+ exit 0
+}
+
main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
@@ -445,28 +450,23 @@ main () {
merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
- files=
-
- if test $# -eq 0
+ if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR"
then
- cd_to_toplevel
-
- if test -e "$GIT_DIR/MERGE_RR"
+ set -- $(git rerere remaining)
+ if test $# -eq 0
then
- files=$(git rerere remaining)
- else
- files=$(git ls-files -u |
- sed -e 's/^[^ ]* //' | sort -u)
+ print_noop_and_exit
fi
- else
- files=$(git ls-files -u -- "$@" |
- sed -e 's/^[^ ]* //' | sort -u)
fi
+ files=$(git -c core.quotePath=false \
+ diff --name-only --diff-filter=U -- "$@")
+
+ cd_to_toplevel
+
if test -z "$files"
then
- echo "No files need merging"
- exit 0
+ print_noop_and_exit
fi
printf "Merging:\n"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 7217f39..38c1e4d 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -606,4 +606,37 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
git reset --hard master >/dev/null 2>&1
'
+test_expect_success 'diff.orderFile configuration is honored' '
+ test_config diff.orderFile order-file &&
+ test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+ test_config mergetool.myecho.trustExitCode true &&
+ echo b >order-file &&
+ echo a >>order-file &&
+ git checkout -b order-file-start master &&
+ echo start >a &&
+ echo start >b &&
+ git add a b &&
+ git commit -m start &&
+ git checkout -b order-file-side1 order-file-start &&
+ echo side1 >a &&
+ echo side1 >b &&
+ git add a b &&
+ git commit -m side1 &&
+ git checkout -b order-file-side2 order-file-start &&
+ echo side2 >a &&
+ echo side2 >b &&
+ git add a b &&
+ git commit -m side2 &&
+ test_must_fail git merge order-file-side1 &&
+ cat >expect <<-\EOF &&
+ Merging:
+ b
+ a
+ EOF
+ git mergetool --no-prompt --tool myecho >output &&
+ git grep --no-index -h -A2 Merging: output >actual &&
+ test_cmp expect actual &&
+ git reset --hard >/dev/null
+'
+
test_done
--
2.10.1.386.g8ee99a0
^ permalink raw reply related
* [PATCH v3 2/4] mergetool: move main program flow into a main() function
From: David Aguilar @ 2016-10-07 23:58 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
In-Reply-To: <20161007235806.22247-1-davvid@gmail.com>
Make it easier to follow the program's flow by isolating all
logic into functions. Isolate the main execution code path into
a single unit instead of having prompt_after_failed_merge()
interrupt it partyway through.
The use of a main() function is borrowing a convention from C,
Python, Perl, and many other languages. This helps readers more
familiar with other languages understand the purpose of each
function when diving into the codebase with fresh eyes.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Unchanged since v2; included for completeness.
git-mergetool.sh | 180 ++++++++++++++++++++++++++++---------------------------
1 file changed, 93 insertions(+), 87 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 300ce7f..b2cd0a4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -366,51 +366,6 @@ merge_file () {
return 0
}
-prompt=$(git config --bool mergetool.prompt)
-guessed_merge_tool=false
-
-while test $# != 0
-do
- case "$1" in
- --tool-help=*)
- TOOL_MODE=${1#--tool-help=}
- show_tool_help
- ;;
- --tool-help)
- show_tool_help
- ;;
- -t|--tool*)
- case "$#,$1" in
- *,*=*)
- merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
- ;;
- 1,*)
- usage ;;
- *)
- merge_tool="$2"
- shift ;;
- esac
- ;;
- -y|--no-prompt)
- prompt=false
- ;;
- --prompt)
- prompt=true
- ;;
- --)
- shift
- break
- ;;
- -*)
- usage
- ;;
- *)
- break
- ;;
- esac
- shift
-done
-
prompt_after_failed_merge () {
while true
do
@@ -427,57 +382,108 @@ prompt_after_failed_merge () {
done
}
-git_dir_init
-require_work_tree
+main () {
+ prompt=$(git config --bool mergetool.prompt)
+ guessed_merge_tool=false
+
+ while test $# != 0
+ do
+ case "$1" in
+ --tool-help=*)
+ TOOL_MODE=${1#--tool-help=}
+ show_tool_help
+ ;;
+ --tool-help)
+ show_tool_help
+ ;;
+ -t|--tool*)
+ case "$#,$1" in
+ *,*=*)
+ merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
+ ;;
+ 1,*)
+ usage ;;
+ *)
+ merge_tool="$2"
+ shift ;;
+ esac
+ ;;
+ -y|--no-prompt)
+ prompt=false
+ ;;
+ --prompt)
+ prompt=true
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ git_dir_init
+ require_work_tree
-if test -z "$merge_tool"
-then
- # Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
- # Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
- merge_tool=$(guess_merge_tool) || exit
- guessed_merge_tool=true
+ # Check if a merge tool has been configured
+ merge_tool=$(get_configured_merge_tool)
+ # Try to guess an appropriate merge tool if no tool has been set.
+ if test -z "$merge_tool"
+ then
+ merge_tool=$(guess_merge_tool) || exit
+ guessed_merge_tool=true
+ fi
fi
-fi
-merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
-merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-files=
+ merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
+ merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-if test $# -eq 0
-then
- cd_to_toplevel
+ files=
- if test -e "$GIT_DIR/MERGE_RR"
+ if test $# -eq 0
then
- files=$(git rerere remaining)
+ cd_to_toplevel
+
+ if test -e "$GIT_DIR/MERGE_RR"
+ then
+ files=$(git rerere remaining)
+ else
+ files=$(git ls-files -u |
+ sed -e 's/^[^ ]* //' | sort -u)
+ fi
else
- files=$(git ls-files -u | sed -e 's/^[^ ]* //' | sort -u)
+ files=$(git ls-files -u -- "$@" |
+ sed -e 's/^[^ ]* //' | sort -u)
fi
-else
- files=$(git ls-files -u -- "$@" | sed -e 's/^[^ ]* //' | sort -u)
-fi
-
-if test -z "$files"
-then
- echo "No files need merging"
- exit 0
-fi
-
-printf "Merging:\n"
-printf "%s\n" "$files"
-
-rc=0
-for i in $files
-do
- printf "\n"
- if ! merge_file "$i"
+
+ if test -z "$files"
then
- rc=1
- prompt_after_failed_merge || exit 1
+ echo "No files need merging"
+ exit 0
fi
-done
-exit $rc
+ printf "Merging:\n"
+ printf "%s\n" "$files"
+
+ rc=0
+ for i in $files
+ do
+ printf "\n"
+ if ! merge_file "$i"
+ then
+ rc=1
+ prompt_after_failed_merge || exit 1
+ fi
+ done
+
+ exit $rc
+}
+
+main "$@"
--
2.10.1.386.g8ee99a0
^ permalink raw reply related
* [PATCH v3 1/4] mergetool: add copyright
From: David Aguilar @ 2016-10-07 23:58 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Johannes Sixt, Luis Gutierrez
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Unchanged since v1; included for completeness.
git-mergetool.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..300ce7f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -3,6 +3,7 @@
# This program resolves merge conflicts in git
#
# Copyright (c) 2006 Theodore Y. Ts'o
+# Copyright (c) 2009-2016 David Aguilar
#
# This file is licensed under the GPL v2, or a later version
# at the discretion of Junio C Hamano.
--
2.10.1.386.g8ee99a0
^ permalink raw reply related
* [PATCH v4 5/7] builtin/tag: add --format argument for tag -v
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Lukas Puehringer <luk.puehringer@gmail.com>
Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.
The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
Documentation/git-tag.txt | 2 +-
builtin/tag.c | 34 +++++++++++++++++++++++-----------
builtin/verify-tag.c | 2 +-
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..3bb5e3c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
[--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
-'git tag' -v <tagname>...
+'git tag' -v [--format=<format>] <tagname>...
DESCRIPTION
-----------
diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..7730fd0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d <tagname>..."),
N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
"\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
- N_("git tag -v <tagname>..."),
+ N_("git tag -v [--format=<format>] <tagname>..."),
NULL
};
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
}
typedef int (*each_tag_name_fn)(const char *name, const char *ref,
- const unsigned char *sha1);
+ const unsigned char *sha1, void *cb_data);
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+ void *cb_data)
{
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
had_error = 1;
continue;
}
- if (fn(*p, ref, sha1))
+ if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
}
static int delete_tag(const char *name, const char *ref,
- const unsigned char *sha1)
+ const unsigned char *sha1, void *cb_data)
{
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
}
static int verify_tag(const char *name, const char *ref,
- const unsigned char *sha1)
+ const unsigned char *sha1, void *cb_data)
{
- return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+ int flags;
+ char *fmt_pretty = cb_data;
+ flags = GPG_VERIFY_VERBOSE;
+
+ if (fmt_pretty)
+ flags = GPG_VERIFY_QUIET;
+
+ return verify_and_format_tag(sha1, name, fmt_pretty, flags);
}
static int do_sign(struct strbuf *buffer)
@@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
- const char *format = NULL;
+ char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with -l"));
if (cmdmode == 'd')
- return for_each_tag_name(argv, delete_tag);
- if (cmdmode == 'v')
- return for_each_tag_name(argv, verify_tag);
+ return for_each_tag_name(argv, delete_tag, NULL);
+ if (cmdmode == 'v') {
+ if (format)
+ verify_ref_format(format);
+ return for_each_tag_name(argv, verify_tag, format);
+ }
if (msg.given || msgfile) {
if (msg.given && msgfile)
--
2.10.0
^ permalink raw reply related
* [PATCH v4 7/7] t/t7004-tag: Add --format specifier tests
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Santiago Torres <santiago@nyu.edu>
tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
t/t7004-tag.sh | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..633b089 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -847,6 +847,22 @@ test_expect_success GPG 'verifying a forged tag should fail' '
test_must_fail git tag -v forged-tag
'
+test_expect_success 'verifying a proper tag with --format pass and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : signed-tag
+ EOF
+ git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : forged-tag
+ EOF
+ test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual &&
+ test_cmp expect actual
+'
+
# blank and empty messages for signed tags:
get_tag_header empty-signed-tag $commit commit $time >expect
--
2.10.0
^ permalink raw reply related
* [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Santiago Torres <santiago@nyu.edu>
Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
t/t7030-verify-tag.sh | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a4..ce37fd9 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
'
+test_expect_success 'verifying tag with --format' '
+ cat >expect <<-\EOF &&
+ tagname : fourth-signed
+ EOF
+ git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
+ cat >expect <<-\EOF &&
+ tagname : 7th forged-signed
+ EOF
+ test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
+ test_cmp expect actual-forged
+'
+
test_done
--
2.10.0
^ permalink raw reply related
* [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Lukas Puehringer <luk.puehringer@gmail.com>
Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
gpg-interface.h | 1 +
tag.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
#define GPG_VERIFY_VERBOSE 1
#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET 4
struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
#include "commit.h"
#include "tree.h"
#include "blob.h"
+#include "gpg-interface.h"
const char *tag_type = "tag";
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, &sigc);
- print_signature_buffer(&sigc, flags);
+
+ if (!(flags & GPG_VERIFY_QUIET))
+ print_signature_buffer(&sigc, flags);
signature_check_clear(&sigc);
return ret;
--
2.10.0
^ permalink raw reply related
* [PATCH v4 3/7] tag: add format specifier to gpg_verify_tag
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Lukas Puehringer <luk.puehringer@gmail.com>
Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
builtin/tag.c | 2 +-
builtin/verify-tag.c | 2 +-
tag.c | 17 +++++++++++------
tag.h | 4 ++--
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
{
- return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+ return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
}
static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
- else if (gpg_verify_tag(sha1, name, flags))
+ else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f..d3512c0 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
#include "tree.h"
#include "blob.h"
#include "gpg-interface.h"
+#include "ref-filter.h"
const char *tag_type = "tag";
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
return ret;
}
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
- unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+ const char *fmt_pretty, unsigned flags)
{
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
- name_to_report ?
- name_to_report :
+ name ?
+ name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
buf = read_sha1_file(sha1, &type, &size);
if (!buf)
return error("%s: unable to read file.",
- name_to_report ?
- name_to_report :
+ name ?
+ name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
ret = run_gpg_verify(buf, size, flags);
free(buf);
+
+ if (fmt_pretty)
+ pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
}
diff --git a/tag.h b/tag.h
index a5721b6..896b9c2 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long si
extern int parse_tag(struct tag *item);
extern struct object *deref_tag(struct object *, const char *, int);
extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
- const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+ const char *fmt_pretty, unsigned flags);
#endif /* TAG_H */
--
2.10.0
^ permalink raw reply related
* [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Santiago Torres <santiago@nyu.edu>
Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.
Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.
Signed-off-by: Santiago Torres <santiago@nyu.edu>
---
Documentation/git-verify-tag.txt | 2 +-
builtin/verify-tag.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edc..0b8075d 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
SYNOPSIS
--------
[verse]
-'git verify-tag' <tag>...
+'git verify-tag' [--format=<format>] <tag>...
DESCRIPTION
-----------
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..745b6a6 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
#include <signal.h>
#include "parse-options.h"
#include "gpg-interface.h"
+#include "ref-filter.h"
static const char * const verify_tag_usage[] = {
- N_("git verify-tag [-v | --verbose] <tag>..."),
+ N_("git verify-tag [-v | --verbose] [--format=<format>] <tag>..."),
NULL
};
+
static int git_verify_tag_config(const char *var, const char *value, void *cb)
{
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
{
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+ char *fmt_pretty = NULL;
const struct option verify_tag_options[] = {
OPT__VERBOSE(&verbose, N_("print tag contents")),
OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
+ OPT_STRING( 0 , "format", &fmt_pretty, N_("format"), N_("format to use for the output")),
OPT_END()
};
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
+ if (fmt_pretty) {
+ verify_ref_format(fmt_pretty);
+ flags |= GPG_VERIFY_QUIET;
+ }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
- else if (verify_and_format_tag(sha1, name, NULL, flags))
+ else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
--
2.10.0
^ permalink raw reply related
* [PATCH v4 0/7] Add --format to tag verification
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Santiago Torres
From: Santiago Torres <santiago@nyu.edu>
This is the fourth iteration of the series in [1][2][3], which comes as a
result of the discussion in [4]. The main goal of this patch series is to bring
--format to git tag verification so that upper-layer tools can inspect the
content of a tag and make decisions based on those contents.
In this re-woll we:
* Fixed the author fields and signed off by's throughout the patch
series
* Added two more patches with unit tests to ensure the format specifier
behaves as expected
* Fixed a missing initialization for the format specifier in verify-tag which
caused a crash.
* Fixed an outdated git commit message that had the previous name of a
function definition.
Thanks,
-Santiago
[1] http://public-inbox.org/git/20160930221806.3398-1-santiago@nyu.edu/
[2] http://public-inbox.org/git/20160922185317.349-1-santiago@nyu.edu/
[3] http://public-inbox.org/git/20160926224233.32702-1-santiago@nyu.edu/
[4] http://public-inbox.org/git/20160607195608.16643-1-santiago@nyu.edu/
Lukas Puehringer (4):
tag: add format specifier to gpg_verify_tag
gpg-interface, tag: add GPG_VERIFY_QUIET flag
ref-filter: add function to print single ref_array_item
builtin/tag: add --format argument for tag -v
Santiago Torres (3):
builtin/verify-tag: add --format to verify-tag
t/t7030-verify-tag: Add --format specifier tests
t/t7004-tag: Add --format specifier tests
Documentation/git-tag.txt | 2 +-
Documentation/git-verify-tag.txt | 2 +-
builtin/tag.c | 34 +++++++++++++++++++++++-----------
builtin/verify-tag.c | 13 +++++++++++--
gpg-interface.h | 1 +
ref-filter.c | 10 ++++++++++
ref-filter.h | 3 +++
t/t7004-tag.sh | 16 ++++++++++++++++
t/t7030-verify-tag.sh | 16 ++++++++++++++++
tag.c | 22 +++++++++++++++-------
tag.h | 4 ++--
11 files changed, 99 insertions(+), 24 deletions(-)
--
2.10.0
^ permalink raw reply
* [PATCH v4 2/7] ref-filter: add function to print single ref_array_item
From: santiago @ 2016-10-07 21:07 UTC (permalink / raw)
To: git; +Cc: gitster, peff, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161007210721.20437-1-santiago@nyu.edu>
From: Lukas Puehringer <luk.puehringer@gmail.com>
ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.
Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.
Signed-off-by: Lukas Puehringer <luk.puehringer@gmail.com>
---
ref-filter.c | 10 ++++++++++
ref-filter.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..cfbcd73 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1637,6 +1637,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
putchar('\n');
}
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+ const char *format, unsigned kind)
+{
+ struct ref_array_item *ref_item;
+ ref_item = new_ref_array_item(name, sha1, 0);
+ ref_item->kind = kind;
+ show_ref_array_item(ref_item, format, 0);
+ free_array_item(ref_item);
+}
+
/* If no sorting option is given, use refname to sort as default */
struct ref_sorting *ref_default_sorting(void)
{
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..3d23090 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
/* Function to parse --merged and --no-merged options */
int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+ const char *format, unsigned kind);
+
#endif /* REF_FILTER_H */
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 2/2] use strbuf_add_unique_abbrev() for adding short hashes, part 2
From: René Scharfe @ 2016-10-07 20:45 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Junio C Hamano
In-Reply-To: <20161007004635.xbuaxvc6eyfkyyma@sigill.intra.peff.net>
Am 07.10.2016 um 02:46 schrieb Jeff King:
> On Tue, Sep 27, 2016 at 09:11:58PM +0200, René Scharfe wrote:
>
>> Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
>> instead of taking detours through find_unique_abbrev() and its static
>> buffer. This is shorter and a bit more efficient.
>> [...]
>> diff --git a/diff.c b/diff.c
>> index a178ed3..be11e4e 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -3109,7 +3109,7 @@ static void fill_metainfo(struct strbuf *msg,
>> }
>> strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
>> find_unique_abbrev(one->oid.hash, abbrev));
>> - strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
>> + strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
>> if (one->mode == two->mode)
>> strbuf_addf(msg, " %06o", one->mode);
>> strbuf_addf(msg, "%s\n", reset);
>
> This one is an interesting case, and maybe a good example of why blind
> coccinelle usage can have some pitfalls. :)
Thank you for paying attention. :) In general I agree that the
surrounding code of such changes should be checked; the issue at hand
could be part of a bigger problem.
> We get rid of the strbuf_addstr(), but notice that we leave untouched
> the find_unique_abbrev() call immediately above. There was a symmetry to
> the two that has been lost.
>
> Probably either:
>
> strbuf_addf(msg, "%s%sindex %s..%s", line_prefix, set
> find_unique_abbrev(one->oid.hash, abbrev),
> find_unique_abbrev(two->oid.hash, abbrev));
>
> or:
>
> strbuf_addf(msg, "%s%sindex ", line_prefix, set);
> strbuf_add_unique_abbrev(msg, one->oid.hash, abbrev);
> strbuf_addstr(msg, "..");
> strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
>
> would be a more appropriate refactoring. The problem is in the original
> patch (which also lacks symmetry; either this predates the multi-buffer
> find_unique_abbrev, or the original author didn't know about it), but I
> think your refactor makes it slightly worse.
I still think the automatically generated patch is a net win, but we
shouldn't stop there.
> I noticed because I have another series which touches these lines, and
> it wants to symmetrically swap out find_unique_abbrev for something
> else. :) I don't think it's a big enough deal to switch now (and I've
> already rebased my series which will touch these lines), but I wanted to
> mention it as a thing to watch out for as we do more of these kinds of
> automated transformations.
OK, then I'll wait for that series to land.
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -396,7 +396,7 @@ static void show_submodule_header(FILE *f, const char *path,
>> find_unique_abbrev(one->hash, DEFAULT_ABBREV));
>> if (!fast_backward && !fast_forward)
>> strbuf_addch(&sb, '.');
>> - strbuf_addstr(&sb, find_unique_abbrev(two->hash, DEFAULT_ABBREV));
>> + strbuf_add_unique_abbrev(&sb->hash, two, DEFAULT_ABBREV);
>
> This one is a similar situation, I think.
Yes, and there are some more. Will take a look.
I don't know how to crack printf-style formats using semantic patches.
It's easy for fixed formats (silly example):
- strbuf_addf(sb, "%s%s", a, b);
+ strbuf_addf(sb, "%s", a);
+ strbuf_addf(sb, "%s", b);
But how to do that for arbitrary formats? Probably not worth it..
René
^ permalink raw reply
* RE: Uninitialized submodules as symlinks
From: David Turner @ 2016-10-07 19:59 UTC (permalink / raw)
To: 'Stefan Beller'; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79ka1rJCks5np9Q__HAHYFDrdGDFLtquhb3fY_iL8q9DEkw@mail.gmail.com>
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> Sent: Friday, October 07, 2016 2:56 PM
> To: David Turner
> Cc: git@vger.kernel.org
> Subject: Re: Uninitialized submodules as symlinks
>
> On Fri, Oct 7, 2016 at 11:17 AM, David Turner <David.Turner@twosigma.com>
> wrote:
> > Presently, uninitialized submodules are materialized in the working tree
> > as empty directories.
>
> Right, there has to be something, to hint at the user that creating a file
> with that path is probably not what they want.
>
> > We would like to consider having them be symlinks. Specifically, we'd
> > like them to be symlinks into a FUSE filesystem which retrieves files on
> > demand.
> >
> > We've actually already got a FUSE filesystem written, but we use a
> > different (semi-manual) means to connect it to the initialized submodules.
>
> So you currently do a
>
> git submodule init <pathspec>
> custom-submodule make-symlink <pathspec>
>
> ?
We do something like
For each initialized submodule: symlink it into the right place in .../somedir
For each uninitialized submodule: symlink from the FUSE into the right place in .../somedir
So .../somedir has the structure of the git main repo, but is all symlinks -- some into FUSE, some into the git repo.
This means that when we initialize (or deinitialize) a submodule, we need to re-run the linking script.
> > We hope to release this FUSE filesystem as free software at some point
> > soon, but we do not yet have a fixed schedule for doing so. Having to run
> > a command to create the symlink-based "union" filesystem is not optimal
> > (since we have to re-run it every time we initialize or deinitialize a
> > submodule).
> >
> > But if the uninitialized submodules could be symlinks into the FUSE
> > filesystem, we wouldn't have this problem. This solution isn't
> > necessarily FUSE-specific -- perhaps someone would want copies of the same
> > submodule in multiple repos, and would want to save disk space by having
> > all copies point to the same place. So the symlinks would be configured
> > by a per-submodule config variable.
>
> I'd imagine that you want both a per-submodule config variable as well as
> a global variable that is a default for all submodules?
>
> git config submodule.trySymlinkDefault /mounted/fuse/
> # any (new) submodule tries to be linked to /mounted/fuse/<path>
> git config submodule.<name>.symlinked ~/my/private/symlinked
> # The <name> submodule goes into another path.
>
> As you propose the FUSE filesystem fetches files on demand, you probably
> want to disable things that scan the whole submodule, e.g. look at
> submodule.<name>.ignore to suppress status looking at all files.
I would actually expect that git would detect that the symlink is unmodified from the configured symlink and automatically decide not to look there.
> When looking through the options, you could add the value "symlink" to
> submodule.<name>.update, which then respects the
> submodule.trySymlinkDefault if present, such that
>
> git clone --recurse-submodules ...
>
> works and sets up the FUSE thing correctly.
>
> How does the FUSE system handle different versions, i.e.
> `git submodule update` to checkout another version of the submodule?
> (btw, I plan on working on integrating submodules to "git checkout", so
> "submodule update" would not need to be run there, but we'd hook it into
> checkout instead)
The fuse has a (virtual) directory for each SHA of the main repo, with each submodule mapped to the then-current version of the submodule's code. Actually, it's a bit more complicated because the uninitialized modules point to already-built binaries -- that is, the symlink is to something like $fuse/$SHA/built/$submodule.
If you check out a new version of the main module, in our current setup, you need to again update all of the submodule symlinks (as described above).
Under my proposal, I guess this would still need to happen. A post-checkout hook could handle it either way. Despite this flaw, switching a submodule between an initialized and deinitialized state would still be more seamless with the symlinks.
> > Naturally, this would require some changes to code that examines the
> working tree -- git status, git diff, etc. They would have to report
> "unchanged" for submodules which were still symlinks to the configured
> location. I have not yet looked at the implementation details beyond
> this.
> >
> > Does this idea make any sense? If I were to implement it (probably in a
> few months, but no official timeline yet), would patches be considered?
>
> I am happy to review patches.
Thanks.
^ permalink raw reply
* Re: Uninitialized submodules as symlinks
From: Stefan Beller @ 2016-10-07 18:56 UTC (permalink / raw)
To: David Turner; +Cc: git@vger.kernel.org
In-Reply-To: <fd5bcf57f92944c0b7f6f2f8342c342c@exmbdft7.ad.twosigma.com>
On Fri, Oct 7, 2016 at 11:17 AM, David Turner <David.Turner@twosigma.com> wrote:
> Presently, uninitialized submodules are materialized in the working tree as empty directories.
Right, there has to be something, to hint at the user that creating a
file with that
path is probably not what they want.
> We would like to consider having them be symlinks. Specifically, we'd like them to be symlinks into a FUSE filesystem which retrieves files on demand.
>
> We've actually already got a FUSE filesystem written, but we use a different (semi-manual) means to connect it to the initialized submodules.
So you currently do a
git submodule init <pathspec>
custom-submodule make-symlink <pathspec>
?
> We hope to release this FUSE filesystem as free software at some point soon, but we do not yet have a fixed schedule for doing so. Having to run a command to create the symlink-based "union" filesystem is not optimal (since we have to re-run it every time we initialize or deinitialize a submodule).
>
> But if the uninitialized submodules could be symlinks into the FUSE filesystem, we wouldn't have this problem. This solution isn't necessarily FUSE-specific -- perhaps someone would want copies of the same submodule in multiple repos, and would want to save disk space by having all copies point to the same place. So the symlinks would be configured by a per-submodule config variable.
I'd imagine that you want both a per-submodule config variable as
well as a global variable that is a default for all submodules?
git config submodule.trySymlinkDefault /mounted/fuse/
# any (new) submodule tries to be linked to /mounted/fuse/<path>
git config submodule.<name>.symlinked ~/my/private/symlinked
# The <name> submodule goes into another path.
As you propose the FUSE filesystem fetches files on demand, you
probably want to disable things that scan the whole submodule,
e.g. look at submodule.<name>.ignore to suppress status looking
at all files.
When looking through the options, you could add the value "symlink" to
submodule.<name>.update, which then respects the
submodule.trySymlinkDefault if present, such that
git clone --recurse-submodules ...
works and sets up the FUSE thing correctly.
How does the FUSE system handle different versions, i.e.
`git submodule update` to checkout another version of the submodule?
(btw, I plan on working on integrating submodules to "git checkout",
so "submodule update" would not need to be run there, but we'd hook
it into checkout instead)
>
> Naturally, this would require some changes to code that examines the working tree -- git status, git diff, etc. They would have to report "unchanged" for submodules which were still symlinks to the configured location. I have not yet looked at the implementation details beyond this.
>
> Does this idea make any sense? If I were to implement it (probably in a few months, but no official timeline yet), would patches be considered?
I am happy to review patches.
^ permalink raw reply
* Re: "Purposes, Concepts,Misfits, and a Redesign of Git" (a research paper)
From: Jakub Narębski @ 2016-10-07 18:56 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20161007205220.e7cc582a042f12cd13b6aef7@domain007.com>
W dniu 07.10.2016 o 19:52, Konstantin Khomoutov pisze:
> On Fri, 30 Sep 2016 19:14:13 +0300
> Konstantin Khomoutov <kostix+git@007spb.ru> wrote:
>
>> The "It Will Never Work in Theory" blog has just posted a summary of a
>> study which tried to identify shortcomings in the design of Git.
>>
>> In the hope it might be interesting, I post this summary here.
>> URL: http://neverworkintheory.org/2016/09/30/rethinking-git.html
>
> I think it would be cool if someone among subscribers could post a link
> to our discussion thread [2] back onto that page. Unfortunatrly that
> requires a Disqus login which I don't have, so may be someone in
> possession of such login could do that? ;-)
>
> 2. https://public-inbox.org/git/ce42f934-4a94-fa29-cff0-5ebb0f004eb5@gmail.com/T/#e95875b7940512b432ab2e29b3dd50ca448df9720
Posted. Thanks for the idea.
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v7 0/4] recursive support for ls-files
From: Brandon Williams @ 2016-10-07 18:45 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jeff King, Junio C Hamano
In-Reply-To: <CAGZ79kZo-hL5-rCdw2VA5ce+xpizbGUdkS8ddb6eMC8qQoRSVw@mail.gmail.com>
On 10/07, Stefan Beller wrote:
> On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller <sbeller@google.com> wrote:
> > On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote:
> >> Few minor fixes pointed out Stefan
> >
>
> The series itself looks good though. :)
>
> Thanks,
> Stefan
Thanks! And I'll keep that in mind for the future. Still lots to learn
about contributing to the project.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v7 0/4] recursive support for ls-files
From: Stefan Beller @ 2016-10-07 18:35 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Jeff King, Junio C Hamano
In-Reply-To: <CAGZ79kacgsPg-dzx7JRyhA2bq9JMb2Cyg9PjWbnKi9ZMTPw7RQ@mail.gmail.com>
On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote:
>> Few minor fixes pointed out Stefan
>
The series itself looks good though. :)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v7 0/4] recursive support for ls-files
From: Stefan Beller @ 2016-10-07 18:34 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Jeff King, Junio C Hamano
In-Reply-To: <20161007181851.15063-1-bmwill@google.com>
On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams <bmwill@google.com> wrote:
> Few minor fixes pointed out Stefan
Nit: This is not very informative, so you could provide an "interdiff"
to the last patch, (see https://github.com/trast/tbdiff for a
sophisticated tool,
I usually do a git diff origin/sb/<what-junio-queued>)
That helps reviewers as they do not need to review it all again, but
they may remember what they annotated and see how you addressed it.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v2 3/3] batch check whether submodule needs pushing into one call
From: Stefan Beller @ 2016-10-07 18:30 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Jeff King, git@vger.kernel.org, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <67d4c48dc0129f20041c88d27a49c7a21188c882.1475851621.git.hvoigt@hvoigt.net>
On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> We run a command for each sha1 change in a submodule. This is
> unnecessary since we can simply batch all sha1's we want to check into
> one command. Lets do it so we can speedup the check when many submodule
> changes are in need of checking.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
> submodule.c | 63 +++++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5044afc2f8..a05c2a34b1 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -529,27 +529,49 @@ static int append_hash_to_argv(const unsigned char sha1[20], void *data)
> return 0;
> }
>
> -static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
> +static int check_has_hash(const unsigned char sha1[20], void *data)
> {
> - if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> + int *has_hash = (int *) data;
> +
> + if (!lookup_commit_reference(sha1))
> + *has_hash = 0;
> +
> + return 0;
> +}
> +
> +static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
> +{
> + int has_hash = 1;
> +
> + if (add_submodule_odb(path))
> + return 0;
> +
> + sha1_array_for_each_unique(hashes, check_has_hash, &has_hash);
> + return has_hash;
> +}
> +
> +static int submodule_needs_pushing(const char *path, struct sha1_array *hashes)
> +{
> + if (!submodule_has_hashes(path, hashes))
So the above is an implicit lookup already, but we did that before,
too, so it's fine.
> @@ -658,13 +665,11 @@ int find_unpushed_submodules(struct sha1_array *hashes,
> argv_array_clear(&argv);
>
> for (i = 0; i < submodules.nr; i++) {
> - struct string_list_item *item = &submodules.items[i];
> - struct collect_submodule_from_sha1s_data data;
> - data.submodule_path = item->string;
> - data.needs_pushing = needs_pushing;
> - sha1_array_for_each_unique((struct sha1_array *) item->util,
> - collect_submodules_from_sha1s,
> - &data);
> + struct string_list_item *submodule = &submodules.items[i];
> + struct sha1_array *hashes = (struct sha1_array *) submodule->util;
> +
> + if (submodule_needs_pushing(submodule->string, hashes))
> + string_list_insert(needs_pushing, submodule->string);
That makes sense.
Thanks!
Stefan
^ permalink raw reply
* [PATCH v7 4/4] ls-files: add pathspec matching for submodules
From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, peff, gitster
In-Reply-To: <20161007181851.15063-1-bmwill@google.com>
Pathspecs can be a bit tricky when trying to apply them to submodules.
The main challenge is that the pathspecs will be with respect to the
superproject and not with respect to paths in the submodule. The
approach this patch takes is to pass in the identical pathspec from the
superproject to the submodule in addition to the submodule-prefix, which
is the path from the root of the superproject to the submodule, and then
we can compare an entry in the submodule prepended with the
submodule-prefix to the pathspec in order to determine if there is a
match.
This patch also permits the pathspec logic to perform a prefix match against
submodules since a pathspec could refer to a file inside of a submodule.
Due to limitations in the wildmatch logic, a prefix match is only done
literally. If any wildcard character is encountered we'll simply punt
and produce a false positive match. More accurate matching will be done
once inside the submodule. This is due to the superproject not knowing
what files could exist in the submodule.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-ls-files.txt | 3 +-
builtin/ls-files.c | 27 +++++++--
dir.c | 46 +++++++++++++-
dir.h | 4 ++
t/t3007-ls-files-recurse-submodules.sh | 108 ++++++++++++++++++++++++++++++++-
5 files changed, 175 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index ea01d45..446209e 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -140,8 +140,7 @@ a space) at the start of each line:
--recurse-submodules::
Recursively calls ls-files on each submodule in the repository.
- Currently there is only support for the --cached mode without a
- pathspec.
+ Currently there is only support for the --cached mode.
--abbrev[=<n>]::
Instead of showing the full 40-byte hexadecimal object
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6144a5..0f25914 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -195,6 +195,7 @@ static void show_gitlink(const struct cache_entry *ce)
{
struct child_process cp = CHILD_PROCESS_INIT;
int status;
+ int i;
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
super_prefix ? super_prefix : "",
@@ -205,6 +206,15 @@ static void show_gitlink(const struct cache_entry *ce)
/* add supported options */
argv_array_pushv(&cp.args, submodules_options.argv);
+ /*
+ * Pass in the original pathspec args. The submodule will be
+ * responsible for prepending the 'submodule_prefix' prior to comparing
+ * against the pathspec for matches.
+ */
+ argv_array_push(&cp.args, "--");
+ for (i = 0; i < pathspec.nr; i++)
+ argv_array_push(&cp.args, pathspec.items[i].original);
+
cp.git_cmd = 1;
cp.dir = ce->name;
status = run_command(&cp);
@@ -223,7 +233,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of prefix");
- if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+ submodule_path_match(&pathspec, name.buf, ps_matched)) {
show_gitlink(ce);
} else if (match_pathspec(&pathspec, name.buf, name.len,
len, ps_matched,
@@ -602,16 +613,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
die("ls-files --recurse-submodules does not support "
"--error-unmatch");
- if (recurse_submodules && argc)
- die("ls-files --recurse-submodules does not support pathspec");
-
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
- /* Find common prefix for all pathspec's */
- max_prefix = common_prefix(&pathspec);
+ /*
+ * Find common prefix for all pathspec's
+ * This is used as a performance optimization which unfortunately cannot
+ * be done when recursing into submodules
+ */
+ if (recurse_submodules)
+ max_prefix = NULL;
+ else
+ max_prefix = common_prefix(&pathspec);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
/* Treat unmatching pathspec elements as errors */
diff --git a/dir.c b/dir.c
index 0ea235f..28e9736 100644
--- a/dir.c
+++ b/dir.c
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
return 1;
}
-#define DO_MATCH_EXCLUDE 1
-#define DO_MATCH_DIRECTORY 2
+#define DO_MATCH_EXCLUDE (1<<0)
+#define DO_MATCH_DIRECTORY (1<<1)
+#define DO_MATCH_SUBMODULE (1<<2)
/*
* Does 'match' match the given name?
@@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
+ /* Perform checks to see if "name" is a super set of the pathspec */
+ if (flags & DO_MATCH_SUBMODULE) {
+ /* name is a literal prefix of the pathspec */
+ if ((namelen < matchlen) &&
+ (match[namelen] == '/') &&
+ !ps_strncmp(item, match, name, namelen))
+ return MATCHED_RECURSIVELY;
+
+ /* name" doesn't match up to the first wild character */
+ if (item->nowildcard_len < item->len &&
+ ps_strncmp(item, match, name,
+ item->nowildcard_len - prefix))
+ return 0;
+
+ /*
+ * Here is where we would perform a wildmatch to check if
+ * "name" can be matched as a directory (or a prefix) against
+ * the pathspec. Since wildmatch doesn't have this capability
+ * at the present we have to punt and say that it is a match,
+ * potentially returning a false positive
+ * The submodules themselves will be able to perform more
+ * accurate matching to determine if the pathspec matches.
+ */
+ return MATCHED_RECURSIVELY;
+ }
+
return 0;
}
@@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps,
return negative ? 0 : positive;
}
+/**
+ * Check if a submodule is a superset of the pathspec
+ */
+int submodule_path_match(const struct pathspec *ps,
+ const char *submodule_name,
+ char *seen)
+{
+ int matched = do_match_pathspec(ps, submodule_name,
+ strlen(submodule_name),
+ 0, seen,
+ DO_MATCH_DIRECTORY |
+ DO_MATCH_SUBMODULE);
+ return matched;
+}
+
int report_path_error(const char *ps_matched,
const struct pathspec *pathspec,
const char *prefix)
diff --git a/dir.h b/dir.h
index da1a858..97c83bb 100644
--- a/dir.h
+++ b/dir.h
@@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item,
const char *pattern, const char *string,
int prefix);
+extern int submodule_path_match(const struct pathspec *ps,
+ const char *submodule_name,
+ char *seen);
+
static inline int ce_path_match(const struct cache_entry *ce,
const struct pathspec *pathspec,
char *seen)
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index 33a2ea7..a542617 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' '
test_cmp expect actual
'
-test_expect_success '--recurse-submodules does not support using path arguments' '
- test_must_fail git ls-files --recurse-submodules b 2>actual &&
- test_i18ngrep "does not support pathspec" actual
+test_expect_success '--recurse-submodules and pathspecs setup' '
+ echo e >submodule/subsub/e.txt &&
+ git -C submodule/subsub add e.txt &&
+ git -C submodule/subsub commit -m "adding e.txt" &&
+ echo f >submodule/f.TXT &&
+ echo g >submodule/g.txt &&
+ git -C submodule add f.TXT g.txt &&
+ git -C submodule commit -m "add f and g" &&
+ echo h >h.txt &&
+ mkdir sib &&
+ echo sib >sib/file &&
+ git add h.txt sib/file &&
+ git commit -m "add h and sib/file" &&
+ git init sub &&
+ echo sub >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+ git submodule add ./sub &&
+ git commit -m "added sub" &&
+
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ h.txt
+ sib/file
+ sub/file
+ submodule/.gitmodules
+ submodule/c
+ submodule/f.TXT
+ submodule/g.txt
+ submodule/subsub/d
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual &&
+ cat actual &&
+ git ls-files --recurse-submodules "*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/g.txt
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules "*.txt" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/f.TXT
+ submodule/g.txt
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules ":(icase)*.txt" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/f.TXT
+ submodule/g.txt
+ EOF
+
+ git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ sub/file
+ EOF
+
+ git ls-files --recurse-submodules "sub" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "sub/" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "sub/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "su*/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "su?/file" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ sib/file
+ sub/file
+ EOF
+
+ git ls-files --recurse-submodules "s??/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "s???file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "s*file" >actual &&
+ test_cmp expect actual
'
test_expect_success '--recurse-submodules does not support --error-unmatch' '
--
2.10.0
^ permalink raw reply related
* [PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules
From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, peff, gitster
In-Reply-To: <20161007181851.15063-1-bmwill@google.com>
Pass through some known-safe options when recursing into submodules.
(--cached, -v, -t, -z, --debug, --eol)
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/ls-files.c | 30 +++++++++++++++++++++++++++---
t/t3007-ls-files-recurse-submodules.sh | 16 ++++++++++++----
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 63befed..b6144a5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -30,6 +30,7 @@ static int line_terminator = '\n';
static int debug_mode;
static int show_eol;
static int recurse_submodules;
+static struct argv_array submodules_options = ARGV_ARRAY_INIT;
static const char *prefix;
static const char *super_prefix;
@@ -168,6 +169,25 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/*
+ * Compile an argv_array with all of the options supported by --recurse_submodules
+ */
+static void compile_submodule_options(const struct dir_struct *dir, int show_tag)
+{
+ if (line_terminator == '\0')
+ argv_array_push(&submodules_options, "-z");
+ if (show_tag)
+ argv_array_push(&submodules_options, "-t");
+ if (show_valid_bit)
+ argv_array_push(&submodules_options, "-v");
+ if (show_cached)
+ argv_array_push(&submodules_options, "--cached");
+ if (show_eol)
+ argv_array_push(&submodules_options, "--eol");
+ if (debug_mode)
+ argv_array_push(&submodules_options, "--debug");
+}
+
/**
* Recursively call ls-files on a submodule
*/
@@ -182,6 +202,9 @@ static void show_gitlink(const struct cache_entry *ce)
argv_array_push(&cp.args, "ls-files");
argv_array_push(&cp.args, "--recurse-submodules");
+ /* add supported options */
+ argv_array_pushv(&cp.args, submodules_options.argv);
+
cp.git_cmd = 1;
cp.dir = ce->name;
status = run_command(&cp);
@@ -567,11 +590,12 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
+ if (recurse_submodules)
+ compile_submodule_options(&dir, show_tag);
+
if (recurse_submodules &&
(show_stage || show_deleted || show_others || show_unmerged ||
- show_killed || show_modified || show_resolve_undo ||
- show_valid_bit || show_tag || show_eol || with_tree ||
- (line_terminator == '\0')))
+ show_killed || show_modified || show_resolve_undo || with_tree))
die("ls-files --recurse-submodules unsupported mode");
if (recurse_submodules && error_unmatch)
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index b5a53c3..33a2ea7 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in submodule' '
test_cmp expect actual
'
+test_expect_success 'ls-files correctly outputs files in submodule with -z' '
+ lf_to_nul >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ git ls-files --recurse-submodules -z >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'ls-files does not output files not added to a repo' '
cat >expect <<-\EOF &&
.gitmodules
@@ -86,15 +98,11 @@ test_incompatible_with_recurse_submodules () {
"
}
-test_incompatible_with_recurse_submodules -z
-test_incompatible_with_recurse_submodules -v
-test_incompatible_with_recurse_submodules -t
test_incompatible_with_recurse_submodules --deleted
test_incompatible_with_recurse_submodules --modified
test_incompatible_with_recurse_submodules --others
test_incompatible_with_recurse_submodules --stage
test_incompatible_with_recurse_submodules --killed
test_incompatible_with_recurse_submodules --unmerged
-test_incompatible_with_recurse_submodules --eol
test_done
--
2.10.0
^ permalink raw reply related
* [PATCH v7 2/4] ls-files: optionally recurse into submodules
From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, peff, gitster
In-Reply-To: <20161007181851.15063-1-bmwill@google.com>
Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules. This is done by forking off a
process to recursively call ls-files on all submodules. Use top-level
--super-prefix option to pass a path to the submodule which it can
use to prepend to output or pathspec matching logic.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-ls-files.txt | 8 +-
builtin/ls-files.c | 138 ++++++++++++++++++++++++---------
git.c | 2 +-
t/t3007-ls-files-recurse-submodules.sh | 100 ++++++++++++++++++++++++
4 files changed, 208 insertions(+), 40 deletions(-)
create mode 100755 t/t3007-ls-files-recurse-submodules.sh
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0d933ac..ea01d45 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -18,7 +18,8 @@ SYNOPSIS
[--exclude-per-directory=<file>]
[--exclude-standard]
[--error-unmatch] [--with-tree=<tree-ish>]
- [--full-name] [--abbrev] [--] [<file>...]
+ [--full-name] [--recurse-submodules]
+ [--abbrev] [--] [<file>...]
DESCRIPTION
-----------
@@ -137,6 +138,11 @@ a space) at the start of each line:
option forces paths to be output relative to the project
top directory.
+--recurse-submodules::
+ Recursively calls ls-files on each submodule in the repository.
+ Currently there is only support for the --cached mode without a
+ pathspec.
+
--abbrev[=<n>]::
Instead of showing the full 40-byte hexadecimal object
lines, show only a partial prefix.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 00ea91a..63befed 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,7 @@
#include "resolve-undo.h"
#include "string-list.h"
#include "pathspec.h"
+#include "run-command.h"
static int abbrev;
static int show_deleted;
@@ -28,8 +29,10 @@ static int show_valid_bit;
static int line_terminator = '\n';
static int debug_mode;
static int show_eol;
+static int recurse_submodules;
static const char *prefix;
+static const char *super_prefix;
static int max_prefix_len;
static int prefix_len;
static struct pathspec pathspec;
@@ -68,11 +71,24 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
static void write_name(const char *name)
{
/*
+ * Prepend the super_prefix to name to construct the full_name to be
+ * written.
+ */
+ struct strbuf full_name = STRBUF_INIT;
+ if (super_prefix) {
+ strbuf_addstr(&full_name, super_prefix);
+ strbuf_addstr(&full_name, name);
+ name = full_name.buf;
+ }
+
+ /*
* With "--full-name", prefix_len=0; this caller needs to pass
* an empty string in that case (a NULL is good for "").
*/
write_name_quoted_relative(name, prefix_len ? prefix : NULL,
stdout, line_terminator);
+
+ strbuf_release(&full_name);
}
static void show_dir_entry(const char *tag, struct dir_entry *ent)
@@ -152,55 +168,84 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/**
+ * Recursively call ls-files on a submodule
+ */
+static void show_gitlink(const struct cache_entry *ce)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ int status;
+
+ argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
+ super_prefix ? super_prefix : "",
+ ce->name);
+ argv_array_push(&cp.args, "ls-files");
+ argv_array_push(&cp.args, "--recurse-submodules");
+
+ cp.git_cmd = 1;
+ cp.dir = ce->name;
+ status = run_command(&cp);
+ if (status)
+ exit(status);
+}
+
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
{
+ struct strbuf name = STRBUF_INIT;
int len = max_prefix_len;
+ if (super_prefix)
+ strbuf_addstr(&name, super_prefix);
+ strbuf_addstr(&name, ce->name);
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of prefix");
- if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
- len, ps_matched,
- S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
- return;
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ show_gitlink(ce);
+ } else if (match_pathspec(&pathspec, name.buf, name.len,
+ len, ps_matched,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode))) {
+ if (tag && *tag && show_valid_bit &&
+ (ce->ce_flags & CE_VALID)) {
+ static char alttag[4];
+ memcpy(alttag, tag, 3);
+ if (isalpha(tag[0]))
+ alttag[0] = tolower(tag[0]);
+ else if (tag[0] == '?')
+ alttag[0] = '!';
+ else {
+ alttag[0] = 'v';
+ alttag[1] = tag[0];
+ alttag[2] = ' ';
+ alttag[3] = 0;
+ }
+ tag = alttag;
+ }
- if (tag && *tag && show_valid_bit &&
- (ce->ce_flags & CE_VALID)) {
- static char alttag[4];
- memcpy(alttag, tag, 3);
- if (isalpha(tag[0]))
- alttag[0] = tolower(tag[0]);
- else if (tag[0] == '?')
- alttag[0] = '!';
- else {
- alttag[0] = 'v';
- alttag[1] = tag[0];
- alttag[2] = ' ';
- alttag[3] = 0;
+ if (!show_stage) {
+ fputs(tag, stdout);
+ } else {
+ printf("%s%06o %s %d\t",
+ tag,
+ ce->ce_mode,
+ find_unique_abbrev(ce->sha1,abbrev),
+ ce_stage(ce));
+ }
+ write_eolinfo(ce, ce->name);
+ write_name(ce->name);
+ if (debug_mode) {
+ const struct stat_data *sd = &ce->ce_stat_data;
+
+ printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
+ printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
+ printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
+ printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
+ printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
}
- tag = alttag;
}
- if (!show_stage) {
- fputs(tag, stdout);
- } else {
- printf("%s%06o %s %d\t",
- tag,
- ce->ce_mode,
- find_unique_abbrev(ce->sha1,abbrev),
- ce_stage(ce));
- }
- write_eolinfo(ce, ce->name);
- write_name(ce->name);
- if (debug_mode) {
- const struct stat_data *sd = &ce->ce_stat_data;
-
- printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
- printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
- printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
- printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
- printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
- }
+ strbuf_release(&name);
}
static void show_ru_info(void)
@@ -468,6 +513,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
N_("make the output relative to the project top directory"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recurse through submodules")),
OPT_BOOL(0, "error-unmatch", &error_unmatch,
N_("if any <file> is not in the index, treat this as an error")),
OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"),
@@ -484,6 +531,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
prefix = cmd_prefix;
if (prefix)
prefix_len = strlen(prefix);
+ super_prefix = get_super_prefix();
git_config(git_default_config, NULL);
if (read_cache() < 0)
@@ -519,6 +567,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
+ if (recurse_submodules &&
+ (show_stage || show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo ||
+ show_valid_bit || show_tag || show_eol || with_tree ||
+ (line_terminator == '\0')))
+ die("ls-files --recurse-submodules unsupported mode");
+
+ if (recurse_submodules && error_unmatch)
+ die("ls-files --recurse-submodules does not support "
+ "--error-unmatch");
+
+ if (recurse_submodules && argc)
+ die("ls-files --recurse-submodules does not support pathspec");
+
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
diff --git a/git.c b/git.c
index 469a83f..df73768 100644
--- a/git.c
+++ b/git.c
@@ -443,7 +443,7 @@ static struct cmd_struct commands[] = {
{ "init-db", cmd_init_db },
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
- { "ls-files", cmd_ls_files, RUN_SETUP },
+ { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo },
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
new file mode 100755
index 0000000..b5a53c3
--- /dev/null
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -0,0 +1,100 @@
+#!/bin/sh
+
+test_description='Test ls-files recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly lists files from
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodules' '
+ echo a >a &&
+ mkdir b &&
+ echo b >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ git init submodule &&
+ echo c >submodule/c &&
+ git -C submodule add c &&
+ git -C submodule commit -m "add c" &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+test_expect_success 'ls-files correctly outputs files in submodule' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files does not output files not added to a repo' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ echo a >not_added &&
+ echo b >b/not_added &&
+ echo c >submodule/not_added &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files recurses more than 1 level' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/.gitmodules
+ submodule/c
+ submodule/subsub/d
+ EOF
+
+ git init submodule/subsub &&
+ echo d >submodule/subsub/d &&
+ git -C submodule/subsub add d &&
+ git -C submodule/subsub commit -m "add d" &&
+ git -C submodule submodule add ./subsub &&
+ git -C submodule commit -m "added subsub" &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules does not support using path arguments' '
+ test_must_fail git ls-files --recurse-submodules b 2>actual &&
+ test_i18ngrep "does not support pathspec" actual
+'
+
+test_expect_success '--recurse-submodules does not support --error-unmatch' '
+ test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual &&
+ test_i18ngrep "does not support --error-unmatch" actual
+'
+
+test_incompatible_with_recurse_submodules () {
+ test_expect_success "--recurse-submodules and $1 are incompatible" "
+ test_must_fail git ls-files --recurse-submodules $1 2>actual &&
+ test_i18ngrep 'unsupported mode' actual
+ "
+}
+
+test_incompatible_with_recurse_submodules -z
+test_incompatible_with_recurse_submodules -v
+test_incompatible_with_recurse_submodules -t
+test_incompatible_with_recurse_submodules --deleted
+test_incompatible_with_recurse_submodules --modified
+test_incompatible_with_recurse_submodules --others
+test_incompatible_with_recurse_submodules --stage
+test_incompatible_with_recurse_submodules --killed
+test_incompatible_with_recurse_submodules --unmerged
+test_incompatible_with_recurse_submodules --eol
+
+test_done
--
2.10.0
^ permalink raw reply related
* [PATCH v7 1/4] git: make super-prefix option
From: Brandon Williams @ 2016-10-07 18:18 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, peff, gitster
In-Reply-To: <20161007181851.15063-1-bmwill@google.com>
Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX'
which can be used to specify a path from above a repository down to its
root. When such a super-prefix is specified, the paths reported by Git
are prefixed with it to make them relative to that directory "above".
The paths given by the user on the command line
(e.g. "git subcmd --output-file=path/to/a/file" and pathspecs) are taken
relative to the directory "above" to match.
The immediate use of this option is by commands which have a
--recurse-submodule option in order to give context to submodules about
how they were invoked. This option is currently only allowed for
builtins which support a super-prefix.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git.txt | 6 ++++++
cache.h | 2 ++
environment.c | 13 +++++++++++++
git.c | 25 +++++++++++++++++++++++++
4 files changed, 46 insertions(+)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7913fc2..2188ae6 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
[-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
[--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
+ [--super-prefix=<path>]
<command> [<args>]
DESCRIPTION
@@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string.
details. Equivalent to setting the `GIT_NAMESPACE` environment
variable.
+--super-prefix=<path>::
+ Currently for internal use only. Set a prefix which gives a path from
+ above a repository down to its root. One use is to give submodules
+ context about the superproject that invoked it.
+
--bare::
Treat the repository as a bare repository. If GIT_DIR
environment is not set, it is set to the current working
diff --git a/cache.h b/cache.h
index 3556326..8cf495d 100644
--- a/cache.h
+++ b/cache.h
@@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
#define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
#define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX"
+#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX"
#define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
#define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
#define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
@@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
extern int get_common_dir(struct strbuf *sb, const char *gitdir);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
+extern const char *get_super_prefix(void);
extern const char *get_git_work_tree(void);
/*
diff --git a/environment.c b/environment.c
index ca72464..d12d7db 100644
--- a/environment.c
+++ b/environment.c
@@ -100,6 +100,8 @@ static char *work_tree;
static const char *namespace;
static size_t namespace_len;
+static const char *super_prefix;
+
static const char *git_dir, *git_common_dir;
static char *git_object_dir, *git_index_file, *git_graft_file;
int git_db_env, git_index_env, git_graft_env, git_common_dir_env;
@@ -120,6 +122,7 @@ const char * const local_repo_env[] = {
NO_REPLACE_OBJECTS_ENVIRONMENT,
GIT_REPLACE_REF_BASE_ENVIRONMENT,
GIT_PREFIX_ENVIRONMENT,
+ GIT_SUPER_PREFIX_ENVIRONMENT,
GIT_SHALLOW_FILE_ENVIRONMENT,
GIT_COMMON_DIR_ENVIRONMENT,
NULL
@@ -222,6 +225,16 @@ const char *strip_namespace(const char *namespaced_ref)
return namespaced_ref + namespace_len;
}
+const char *get_super_prefix(void)
+{
+ static int initialized;
+ if (!initialized) {
+ super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT);
+ initialized = 1;
+ }
+ return super_prefix;
+}
+
static int git_work_tree_initialized;
/*
diff --git a/git.c b/git.c
index 1c61151..469a83f 100644
--- a/git.c
+++ b/git.c
@@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strcmp(cmd, "--super-prefix")) {
+ if (*argc < 2) {
+ fprintf(stderr, "No prefix given for --super-prefix.\n" );
+ usage(git_usage_string);
+ }
+ setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1);
+ if (envchanged)
+ *envchanged = 1;
+ (*argv)++;
+ (*argc)--;
+ } else if (skip_prefix(cmd, "--super-prefix=", &cmd)) {
+ setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1);
+ if (envchanged)
+ *envchanged = 1;
} else if (!strcmp(cmd, "--bare")) {
char *cwd = xgetcwd();
is_bare_repository_cfg = 1;
@@ -310,6 +324,7 @@ static int handle_alias(int *argcp, const char ***argv)
* RUN_SETUP for reading from the configuration file.
*/
#define NEED_WORK_TREE (1<<3)
+#define SUPPORT_SUPER_PREFIX (1<<4)
struct cmd_struct {
const char *cmd;
@@ -344,6 +359,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
}
commit_pager_choice();
+ if (!help && get_super_prefix()) {
+ if (!(p->option & SUPPORT_SUPER_PREFIX))
+ die("%s doesn't support --super-prefix", p->cmd);
+ if (prefix)
+ die("can't use --super-prefix from a subdirectory");
+ }
+
if (!help && p->option & NEED_WORK_TREE)
setup_work_tree();
@@ -558,6 +580,9 @@ static void execv_dashed_external(const char **argv)
const char *tmp;
int status;
+ if (get_super_prefix())
+ die("%s doesn't support --super-prefix", argv[0]);
+
if (use_pager == -1)
use_pager = check_pager_config(argv[0]);
commit_pager_choice();
--
2.10.0
^ 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