[PATCH v3 7/8] branch.c: use 'ref-filter' APIs
Make 'branch.c' use 'ref-filter' APIs for iterating through refs
sorting. This removes most of the code used in 'branch.c' replacing it
with calls to the 'ref-filter' library.
Make 'branch.c' use the 'filter_refs()' function provided by 'ref-filter'
to filter out tags based on the options set.
We provide a sorting option provided for 'branch.c' by using the sorting
options provided by 'ref-filter'.
Also remove the 'ignore' variable from ref_array_item as it was
previously used for the '--merged' option and now that is handled by
ref-filter.
The test t1430 'git branch shows badly named ref' has been changed to
check the stderr for the warning regarding the broken ref. This is
done as ref-filter throws a warning for broken refs rather than
directly printing them.
Modify documentation and add tests for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-branch.txt | 9 +-
builtin/branch.c | 213 +++
ref-filter.c | 2 +-
ref-filter.h | 1 -
t/t1430-bad-ref-name.sh | 2 +-
t/t3203-branch-output.sh | 11 +++
6 files changed, 56 insertions(+), 182 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index a67138a..897cd81 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git branch' [--color[=] | --no-color] [-r | -a]
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
- [(--merged | --no-merged | --contains) []] [...]
+ [(--merged | --no-merged | --contains) []] [--sort=]
[...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f]
[]
'git branch' (--set-upstream-to= | -u ) []
'git branch' --unset-upstream []
@@ -229,6 +229,13 @@ start-point is either a local or remote-tracking branch.
The new name for an existing branch. The same restrictions as for
apply.
+--sort=::
+ Sort based on the key given. Prefix `-` to sort in descending
+ order of the value. You may use the --sort= option
+ multiple times, in which case the last key becomes the primary
+ key. The keys supported are the same as those in `git
+ for-each-ref`. Sort order defaults to sorting based on branch
+ type.
Examples
diff --git a/builtin/branch.c b/builtin/branch.c
index 5cb7ef0..e0aa44c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -270,125 +270,6 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
return(ret);
}
-static char *resolve_symref(const char *src, const char *prefix)
-{
- unsigned char sha1[20];
- int flag;
- const char *dst;
-
- dst = resolve_ref_unsafe(src, 0, sha1, &flag);
- if (!(dst && (flag & REF_ISSYMREF)))
- return NULL;
- if (prefix)
- skip_prefix(dst, prefix, &dst);
- return xstrdup(dst);
-}
-
-static int match_patterns(const char **pattern, const char *refname)
-{
- if (!*pattern)
- return 1; /* no pattern always matches */
- while (*pattern) {
- if (!wildmatch(*pattern, refname, 0, NULL))
- return 1;
- pattern++;
- }
- return 0;
-}
-
-/*
- * Allocate memory for a new ref_array_item and insert that into the
- * given ref_array. Doesn't take the objectname unlike
- * new_ref_array_item(). This is a temporary function which will be
- * removed when we port branch.c to use ref-filter APIs.
- */
-static struct ref_array_item *ref_array_append(struct ref_array *array, const
char *refname)
-{
- size_t len = strlen(refname);
- struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) +
len + 1);
- memcpy(ref->refname, refname, len);
- ref->refname[len] = '\0';
- REALLOC_ARRAY(array->items, array->nr + 1);
- array->items[array->nr++] = ref;
- return ref;
-}
-
-static int append_ref(const char *refname, const struct object_id *oid, int
flags, void *cb_data)
-{
- struct ref_filter_cbdata *cb = (struct ref_filter_cbdata *)(cb_data);
- struct ref_filter *filter = cb->filter;
- struct ref_array *array = cb->array;
- struct ref_array_item *item;
- struct commit *commit;
- int kind, i;
- const char *prefix, *orig_refname = refname;
-
- static struct {
- int kind;
- const char *prefix;
- } ref_kind[] = {
- { FILTER_REFS_BRANCHES, "refs/heads/" },
- { FILTER_REFS_REMOTES, "refs/remotes/" },
- };
-
- /* Detect kind */
- for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
- prefix = ref_kind[i].prefix;
- if (skip_prefix(refname, prefix, &refname)) {
- kind = ref_kind[i].kind;
- break;
- }
-
[PATCH v3 8/8] branch: add '--points-at' option
Add the '--points-at' option provided by 'ref-filter'. The option lets
the user to list only branches which points at the given object.
Add documentation and tests for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-branch.txt | 6 +-
builtin/branch.c | 7 ++-
t/t3203-branch-output.sh | 9 +
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 897cd81..211cfc3 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -11,7 +11,8 @@ SYNOPSIS
'git branch' [--color[=] | --no-color] [-r | -a]
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
- [(--merged | --no-merged | --contains) []] [--sort=]
[...]
+ [(--merged | --no-merged | --contains) []] [--sort=]
+ [--points-at ] [...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f]
[]
'git branch' (--set-upstream-to= | -u ) []
'git branch' --unset-upstream []
@@ -237,6 +238,9 @@ start-point is either a local or remote-tracking branch.
for-each-ref`. Sort order defaults to sorting based on branch
type.
+--points-at ::
+ Only list branches of the given object.
+
Examples
diff --git a/builtin/branch.c b/builtin/branch.c
index e0aa44c..32a0d11 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -26,6 +26,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-l] [-f] []"),
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
+ N_("git branch [] [-r | -a] [--points-at]"),
NULL
};
@@ -652,6 +653,10 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_COLUMN(0, "column", &colopts, N_("list branches in
columns")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
N_("field name to sort on"),
&parse_opt_ref_sorting),
+ {
+ OPTION_CALLBACK, 0, "points-at", &filter.points_at,
N_("object"),
+ N_("print only branches of the object"), 0,
parse_opt_object_name
+ },
OPT_END(),
};
@@ -680,7 +685,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (!delete && !rename && !edit_description && !new_upstream &&
!unset_upstream && argc == 0)
list = 1;
- if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE)
+ if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE ||
filter.points_at.nr)
list = 1;
if (!!delete + !!rename + !!new_upstream +
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 53d166d..c819f3e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -154,4 +154,13 @@ test_expect_success 'git branch `--sort` option' '
test_i18ncmp expect actual
'
+test_expect_success 'git branch --points-at option' '
+ cat >expect <<-\EOF &&
+ master
+ branch-one
+ EOF
+ git branch --points-at=branch-one >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/8] branch.c: use 'ref-filter' data structures
Make 'branch.c' use 'ref-filter' data structures and make changes to
support the new data structures. This is a part of the process of
porting 'branch.c' to use 'ref-filter' APIs.
This is a temporary step before porting 'branch.c' to use 'ref-filter'
completely. As this is a temporary step, most of the code introduced
here will be removed when 'branch.c' is ported over to use
'ref-filter' APIs.
Make 'free_array_item()' of ref-filter.c a non static function. This
is used to free memory allocated to a detached head ref, before we
print other refs.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 328 ++-
ref-filter.c | 2 +-
ref-filter.h | 9 +-
3 files changed, 141 insertions(+), 198 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 8b9da60..5cb7ef0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -19,6 +19,7 @@
#include "column.h"
#include "utf8.h"
#include "wt-status.h"
+#include "ref-filter.h"
static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r | -a] [--merged | --no-merged]"),
@@ -28,10 +29,6 @@ static const char * const builtin_branch_usage[] = {
NULL
};
-#define REF_LOCAL_BRANCH0x01
-#define REF_REMOTE_BRANCH 0x02
-#define REF_DETACHED_HEAD 0x04
-
static const char *head;
static unsigned char head_sha1[20];
@@ -53,13 +50,6 @@ enum color_branch {
BRANCH_COLOR_UPSTREAM = 5
};
-static enum merge_filter {
- NO_FILTER = 0,
- SHOW_NOT_MERGED,
- SHOW_MERGED
-} merge_filter;
-static unsigned char merge_filter_ref[20];
-
static struct string_list output = STRING_LIST_INIT_DUP;
static unsigned int colopts;
@@ -122,7 +112,7 @@ static int branch_merged(int kind, const char *name,
void *reference_name_to_free = NULL;
int merged;
- if (kind == REF_LOCAL_BRANCH) {
+ if (kind == FILTER_REFS_BRANCHES) {
struct branch *branch = branch_get(name);
const char *upstream = branch_get_upstream(branch, NULL);
unsigned char sha1[20];
@@ -200,14 +190,14 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
struct strbuf bname = STRBUF_INIT;
switch (kinds) {
- case REF_REMOTE_BRANCH:
+ case FILTER_REFS_REMOTES:
fmt = "refs/remotes/%s";
/* For subsequent UI messages */
remote_branch = 1;
force = 1;
break;
- case REF_LOCAL_BRANCH:
+ case FILTER_REFS_BRANCHES:
fmt = "refs/heads/%s";
break;
default:
@@ -224,7 +214,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
int flags = 0;
strbuf_branchname(&bname, argv[i]);
- if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
+ if (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {
error(_("Cannot delete the branch '%s' "
"which you are currently on."), bname.buf);
ret = 1;
@@ -280,22 +270,6 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
return(ret);
}
-struct ref_item {
- char *name;
- char *dest;
- unsigned int kind;
- struct commit *commit;
- int ignore;
-};
-
-struct ref_list {
- struct rev_info revs;
- int index, alloc, verbose, abbrev;
- struct ref_item *list;
- struct commit_list *with_commit;
- int kinds;
-};
-
static char *resolve_symref(const char *src, const char *prefix)
{
unsigned char sha1[20];
@@ -310,11 +284,6 @@ static char *resolve_symref(const char *src, const char
*prefix)
return xstrdup(dst);
}
-struct append_ref_cb {
- struct ref_list *ref_list;
- const char **pattern;
-};
-
static int match_patterns(const char **pattern, const char *refname)
{
if (!*pattern)
@@ -327,11 +296,29 @@ static int match_patterns(const char **pattern, const
char *refname)
return 0;
}
+/*
+ * Allocate memory for a new ref_array_item and insert that into the
+ * given ref_array. Doesn't take the objectname unlike
+ * new_ref_array_item(). This is a temporary function which will be
+ * removed when we port branch.c to use ref-filter APIs.
+ */
+static struct ref_array_item *ref_array_append(struct ref_array *array, const
char *refname)
+{
+ size_t len = strlen(refname);
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) +
len + 1);
+ memcpy(ref->refname, refname, len);
+ ref->refname[len] = '\0';
+ REALLOC_ARRAY(array->items, array->nr + 1);
+ array->items[array->nr++] = ref;
+ return ref;
+}
+
static int append_ref(const char *refname, const struct object_id *oid, int
flags
[PATCH v3 3/8] branch: roll show_detached HEAD into regular ref_list
Remove show_detached() and make detached HEAD to be rolled into
regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and
supporting the same in append_ref(). This eliminates the need for an
extra function and helps in easier porting of branch.c to use
ref-filter APIs.
Before show_detached() used to check if the HEAD branch satisfies the
'--contains' option, now that is taken care by append_ref().
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 68 +---
1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 193296a..6ba7a3f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -30,6 +30,7 @@ static const char * const builtin_branch_usage[] = {
#define REF_LOCAL_BRANCH0x01
#define REF_REMOTE_BRANCH 0x02
+#define REF_DETACHED_HEAD 0x04
static const char *head;
static unsigned char head_sha1[20];
@@ -352,8 +353,12 @@ static int append_ref(const char *refname, const struct
object_id *oid, int flag
break;
}
}
- if (ARRAY_SIZE(ref_kind) <= i)
- return 0;
+ if (ARRAY_SIZE(ref_kind) <= i) {
+ if (!strcmp(refname, "HEAD"))
+ kind = REF_DETACHED_HEAD;
+ else
+ return 0;
+ }
/* Don't add types the caller doesn't want */
if ((kind & ref_list->kinds) == 0)
@@ -535,6 +540,8 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
const char *prefix = "";
+ const char *desc = item->name;
+ char *to_free = NULL;
if (item->ignore)
return;
@@ -547,6 +554,10 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
color = BRANCH_COLOR_REMOTE;
prefix = remote_prefix;
break;
+ case REF_DETACHED_HEAD:
+ color = BRANCH_COLOR_CURRENT;
+ desc = to_free = get_head_description();
+ break;
default:
color = BRANCH_COLOR_PLAIN;
break;
@@ -558,7 +569,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
color = BRANCH_COLOR_CURRENT;
}
- strbuf_addf(&name, "%s%s", prefix, item->name);
+ strbuf_addf(&name, "%s%s", prefix, desc);
if (verbose) {
int utf8_compensation = strlen(name.buf) -
utf8_strwidth(name.buf);
strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color),
@@ -581,6 +592,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
}
strbuf_release(&name);
strbuf_release(&out);
+ free(to_free);
}
static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
@@ -601,25 +613,9 @@ static int calc_maxwidth(struct ref_list *refs, int
remote_bonus)
return max;
}
-static void show_detached(struct ref_list *ref_list, int maxwidth)
-{
- struct commit *head_commit = lookup_commit_reference_gently(head_sha1,
1);
-
- if (head_commit && is_descendant_of(head_commit,
ref_list->with_commit)) {
- struct ref_item item;
- item.name = get_head_description();
- item.kind = REF_LOCAL_BRANCH;
- item.dest = NULL;
- item.commit = head_commit;
- item.ignore = 0;
- print_ref_item(&item, maxwidth, ref_list->verbose,
ref_list->abbrev, 1, "");
- free(item.name);
- }
-}
-
static int print_ref_list(int kinds, int detached, int verbose, int abbrev,
struct commit_list *with_commit, const char **pattern)
{
- int i;
+ int i, index;
struct append_ref_cb cb;
struct ref_list ref_list;
int maxwidth = 0;
@@ -643,7 +639,14 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
cb.ref_list = &ref_list;
cb.pattern = pattern;
cb.ret = 0;
+ /*
+* First we obtain all regular branch refs and if the HEAD is
+* detached then we insert that ref to the end of the ref_fist
+* so that it can be printed and removed first.
+*/
for_each_rawref(append_ref, &cb);
+ if (detached)
+ head_ref(append_ref, &cb);
/*
* The following implementation is currently duplicated in ref-filter.
It
* will eventually be removed when we port branch.c to use ref-filter
APIs.
@@ -679,15 +682,20 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
if (verbose)
maxwidth = calc_maxwidth(&ref_list, strlen(remote_prefix));
- qsort(ref_list.list, ref_list.index
[PATCH v3 4/8] branch: move 'current' check down to the presentation layer
We check if given ref is the current branch in print_ref_list(). Move
this check to print_ref_item() where it is checked right before
printing.
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 6ba7a3f..4d9e4d0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -534,9 +534,10 @@ static char *get_head_description(void)
}
static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
- int abbrev, int current, const char *remote_prefix)
+ int abbrev, int detached, const char *remote_prefix)
{
char c;
+ int current = 0;
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
const char *prefix = "";
@@ -548,15 +549,18 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
switch (item->kind) {
case REF_LOCAL_BRANCH:
- color = BRANCH_COLOR_LOCAL;
+ if (!detached && !strcmp(item->name, head))
+ current = 1;
+ else
+ color = BRANCH_COLOR_LOCAL;
break;
case REF_REMOTE_BRANCH:
color = BRANCH_COLOR_REMOTE;
prefix = remote_prefix;
break;
case REF_DETACHED_HEAD:
- color = BRANCH_COLOR_CURRENT;
desc = to_free = get_head_description();
+ current = 1;
break;
default:
color = BRANCH_COLOR_PLAIN;
@@ -685,21 +689,17 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
index = ref_list.index;
/* Print detached HEAD before sorting and printing the rest */
- if (detached && (ref_list.list[index - 1].kind == REF_DETACHED_HEAD) &&
- !strcmp(ref_list.list[index - 1].name, head)) {
+ if (detached) {
print_ref_item(&ref_list.list[index - 1], maxwidth, verbose,
abbrev,
- 1, remote_prefix);
+ detached, remote_prefix);
index -= 1;
}
qsort(ref_list.list, index, sizeof(struct ref_item), ref_cmp);
- for (i = 0; i < index; i++) {
- int current = !detached && (ref_list.list[i].kind ==
REF_LOCAL_BRANCH) &&
- !strcmp(ref_list.list[i].name, head);
+ for (i = 0; i < index; i++)
print_ref_item(&ref_list.list[i], maxwidth, verbose,
- abbrev, current, remote_prefix);
- }
+ abbrev, detached, remote_prefix);
free_ref_list(&ref_list);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/8] branch: drop non-commit error reporting
Remove the error reporting variable to make the code easier to port
over to using ref-filter APIs. This variable is not required as in
ref-filter we already check for possible errors and report them.
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 4d9e4d0..8b9da60 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -313,7 +313,6 @@ static char *resolve_symref(const char *src, const char
*prefix)
struct append_ref_cb {
struct ref_list *ref_list;
const char **pattern;
- int ret;
};
static int match_patterns(const char **pattern, const char *refname)
@@ -370,10 +369,8 @@ static int append_ref(const char *refname, const struct
object_id *oid, int flag
commit = NULL;
if (ref_list->verbose || ref_list->with_commit || merge_filter !=
NO_FILTER) {
commit = lookup_commit_reference_gently(oid->hash, 1);
- if (!commit) {
- cb->ret = error(_("branch '%s' does not point at a
commit"), refname);
+ if (!commit)
return 0;
- }
/* Filter with with_commit if specified */
if (!is_descendant_of(commit, ref_list->with_commit))
@@ -617,7 +614,7 @@ static int calc_maxwidth(struct ref_list *refs, int
remote_bonus)
return max;
}
-static int print_ref_list(int kinds, int detached, int verbose, int abbrev,
struct commit_list *with_commit, const char **pattern)
+static void print_ref_list(int kinds, int detached, int verbose, int abbrev,
struct commit_list *with_commit, const char **pattern)
{
int i, index;
struct append_ref_cb cb;
@@ -642,7 +639,6 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
init_revisions(&ref_list.revs, NULL);
cb.ref_list = &ref_list;
cb.pattern = pattern;
- cb.ret = 0;
/*
* First we obtain all regular branch refs and if the HEAD is
* detached then we insert that ref to the end of the ref_fist
@@ -702,11 +698,6 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
abbrev, detached, remote_prefix);
free_ref_list(&ref_list);
-
- if (cb.ret)
- error(_("some refs could not be read"));
-
- return cb.ret;
}
static void rename_branch(const char *oldname, const char *newname, int force)
@@ -922,15 +913,14 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
die(_("branch name required"));
return delete_branches(argc, argv, delete > 1, kinds, quiet);
} else if (list) {
- int ret;
/* git branch --local also shows HEAD when it is detached */
if (kinds & REF_LOCAL_BRANCH)
kinds |= REF_DETACHED_HEAD;
- ret = print_ref_list(kinds, detached, verbose, abbrev,
+ print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
print_columns(&output, colopts, NULL);
string_list_clear(&output, 0);
- return ret;
+ return 0;
}
else if (edit_description) {
const char *branch_name;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/8] branch: bump get_head_description() to the top
This is a preperatory patch for 'roll show_detached HEAD into regular
ref_list'. This patch moves get_head_description() to the top so that
it can be used in print_ref_item().
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 62
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 28a10d6..193296a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -497,6 +497,37 @@ static void add_verbose_info(struct strbuf *out, struct
ref_item *item,
strbuf_release(&subject);
}
+static char *get_head_description(void)
+{
+ struct strbuf desc = STRBUF_INIT;
+ struct wt_status_state state;
+ memset(&state, 0, sizeof(state));
+ wt_status_get_state(&state, 1);
+ if (state.rebase_in_progress ||
+ state.rebase_interactive_in_progress)
+ strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+ state.branch);
+ else if (state.bisect_in_progress)
+ strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
+ state.branch);
+ else if (state.detached_from) {
+ /* TRANSLATORS: make sure these match _("HEAD detached at ")
+ and _("HEAD detached from ") in wt-status.c */
+ if (state.detached_at)
+ strbuf_addf(&desc, _("(HEAD detached at %s)"),
+ state.detached_from);
+ else
+ strbuf_addf(&desc, _("(HEAD detached from %s)"),
+ state.detached_from);
+ }
+ else
+ strbuf_addstr(&desc, _("(no branch)"));
+ free(state.branch);
+ free(state.onto);
+ free(state.detached_from);
+ return strbuf_detach(&desc, NULL);
+}
+
static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
int abbrev, int current, const char *remote_prefix)
{
@@ -570,37 +601,6 @@ static int calc_maxwidth(struct ref_list *refs, int
remote_bonus)
return max;
}
-static char *get_head_description(void)
-{
- struct strbuf desc = STRBUF_INIT;
- struct wt_status_state state;
- memset(&state, 0, sizeof(state));
- wt_status_get_state(&state, 1);
- if (state.rebase_in_progress ||
- state.rebase_interactive_in_progress)
- strbuf_addf(&desc, _("(no branch, rebasing %s)"),
- state.branch);
- else if (state.bisect_in_progress)
- strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
- state.branch);
- else if (state.detached_from) {
- /* TRANSLATORS: make sure these match _("HEAD detached at ")
- and _("HEAD detached from ") in wt-status.c */
- if (state.detached_at)
- strbuf_addf(&desc, _("(HEAD detached at %s)"),
- state.detached_from);
- else
- strbuf_addf(&desc, _("(HEAD detached from %s)"),
- state.detached_from);
- }
- else
- strbuf_addstr(&desc, _("(no branch)"));
- free(state.branch);
- free(state.onto);
- free(state.detached_from);
- return strbuf_detach(&desc, NULL);
-}
-
static void show_detached(struct ref_list *ref_list, int maxwidth)
{
struct commit *head_commit = lookup_commit_reference_gently(head_sha1,
1);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/8] branch: refactor width computation
From: Karthik Nayak
Remove unnecessary variables from ref_list and ref_item which were
used for width computation. This is to make ref_item similar to
ref-filter's ref_array_item. This will ensure a smooth port of
branch.c to use ref-filter APIs in further patches.
Previously the maxwidth was computed when inserting the refs into the
ref_list. Now, we obtain the entire ref_list and then compute
maxwidth.
Based-on-patch-by: Jeff King
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 64 ++--
1 file changed, 34 insertions(+), 30 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 4fc8beb..28a10d6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -282,14 +282,14 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
struct ref_item {
char *name;
char *dest;
- unsigned int kind, width;
+ unsigned int kind;
struct commit *commit;
int ignore;
};
struct ref_list {
struct rev_info revs;
- int index, alloc, maxwidth, verbose, abbrev;
+ int index, alloc, verbose, abbrev;
struct ref_item *list;
struct commit_list *with_commit;
int kinds;
@@ -386,15 +386,8 @@ static int append_ref(const char *refname, const struct
object_id *oid, int flag
newitem->name = xstrdup(refname);
newitem->kind = kind;
newitem->commit = commit;
- newitem->width = utf8_strwidth(refname);
newitem->dest = resolve_symref(orig_refname, prefix);
newitem->ignore = 0;
- /* adjust for "remotes/" */
- if (newitem->kind == REF_REMOTE_BRANCH &&
- ref_list->kinds != REF_REMOTE_BRANCH)
- newitem->width += 8;
- if (newitem->width > ref_list->maxwidth)
- ref_list->maxwidth = newitem->width;
return 0;
}
@@ -505,11 +498,12 @@ static void add_verbose_info(struct strbuf *out, struct
ref_item *item,
}
static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
- int abbrev, int current, char *prefix)
+ int abbrev, int current, const char *remote_prefix)
{
char c;
int color;
struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
+ const char *prefix = "";
if (item->ignore)
return;
@@ -520,6 +514,7 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
break;
case REF_REMOTE_BRANCH:
color = BRANCH_COLOR_REMOTE;
+ prefix = remote_prefix;
break;
default:
color = BRANCH_COLOR_PLAIN;
@@ -557,16 +552,22 @@ static void print_ref_item(struct ref_item *item, int
maxwidth, int verbose,
strbuf_release(&out);
}
-static int calc_maxwidth(struct ref_list *refs)
+static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
{
- int i, w = 0;
+ int i, max = 0;
for (i = 0; i < refs->index; i++) {
- if (refs->list[i].ignore)
+ struct ref_item *it = &refs->list[i];
+ int w;
+
+ if (it->ignore)
continue;
- if (refs->list[i].width > w)
- w = refs->list[i].width;
+ w = utf8_strwidth(it->name);
+ if (it->kind == REF_REMOTE_BRANCH)
+ w += remote_bonus;
+ if (w > max)
+ max = w;
}
- return w;
+ return max;
}
static char *get_head_description(void)
@@ -600,21 +601,18 @@ static char *get_head_description(void)
return strbuf_detach(&desc, NULL);
}
-static void show_detached(struct ref_list *ref_list)
+static void show_detached(struct ref_list *ref_list, int maxwidth)
{
struct commit *head_commit = lookup_commit_reference_gently(head_sha1,
1);
if (head_commit && is_descendant_of(head_commit,
ref_list->with_commit)) {
struct ref_item item;
item.name = get_head_description();
- item.width = utf8_strwidth(item.name);
item.kind = REF_LOCAL_BRANCH;
item.dest = NULL;
item.commit = head_commit;
item.ignore = 0;
- if (item.width > ref_list->maxwidth)
- ref_list->maxwidth = item.width;
- print_ref_item(&item, ref_list->maxwidth, ref_list->verbose,
ref_list->abbrev, 1, "");
+ print_ref_item(&item, maxwidth, ref_list->verbose,
ref_list->abbrev, 1, "");
free(item.name);
}
}
@@ -624,6 +622,16 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
int i;
struct append_ref_cb cb;
struct ref_list ref_list;
+ int maxwidth = 0;
+ const char *remote_
[PATCH v3 0/8] port the filtering part of ref-filter to branch.c
This is a follow up to porting tag.c to use ref-fitler APIs.
v2 of this patch series can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/276147
Changes made in this series:
* Improve comment in 3/8 and fix grammar in 5/8.
* Fix the test in t1430 to check stderr for the broken ref warning.
* Instead of showing the detached head, reducing the no of array elements
and displaying all of the other refs and then free'ing all of the refs. We
now free the detached head ref immediately after displaying so we don't have
to bother about decrementing and incrementing the no of array elements.
*
Karthik Nayak (8):
branch: refactor width computation
branch: bump get_head_description() to the top
branch: roll show_detached HEAD into regular ref_list
branch: move 'current' check down to the presentation layer
branch: drop non-commit error reporting
branch.c: use 'ref-filter' data structures
branch.c: use 'ref-filter' APIs
branch: add '--points-at' option
Documentation/git-branch.txt | 13 +-
builtin/branch.c | 506 +--
ref-filter.c | 4 +-
ref-filter.h | 8 +-
t/t1430-bad-ref-name.sh | 2 +-
t/t3203-branch-output.sh | 20 ++
6 files changed, 197 insertions(+), 356 deletions(-)
Interdiff:
diff --git a/builtin/branch.c b/builtin/branch.c
index dd2fdbe..32a0d11 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -475,7 +475,7 @@ static int calc_maxwidth(struct ref_array *refs, int
remote_bonus)
static void print_ref_list(struct ref_filter *filter, struct ref_sorting
*sorting)
{
- int i, index;
+ int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
@@ -493,17 +493,16 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
memset(&array, 0, sizeof(array));
verify_ref_format("%(refname)%(symref)");
- filter_refs(&array, filter, filter->kind);
+ filter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);
if (filter->verbose)
maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
- index = array.nr;
-
/* Print detached HEAD before sorting and printing the rest */
if (filter->kind & FILTER_REFS_DETACHED_HEAD) {
- format_and_print_ref_item(array.items[index - 1], maxwidth,
filter, remote_prefix);
- array.nr -= 1;
+ format_and_print_ref_item(array.items[array.nr - 1], maxwidth,
filter, remote_prefix);
+ free_array_item(array.items[array.nr - 1]);
+ array.nr--;
}
if (!sorting) {
@@ -517,7 +516,6 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
for (i = 0; i < array.nr; i++)
format_and_print_ref_item(array.items[i], maxwidth, filter,
remote_prefix);
- array.nr = index;
ref_array_clear(&array);
}
diff --git a/ref-filter.c b/ref-filter.c
index 112feaa..3cd0c00 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1251,7 +1251,7 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
}
/* Free memory allocated for a ref_array_item */
-static void free_array_item(struct ref_array_item *item)
+void free_array_item(struct ref_array_item *item)
{
free((char *)item->symref);
free(item);
diff --git a/ref-filter.h b/ref-filter.h
index 3e29e5d..3e25d84 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -89,6 +89,8 @@ struct ref_filter_cbdata {
* filtered refs in the ref_array structure.
*/
int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned
int type);
+/* Clear memory allocated to a ref_array_item */
+void free_array_item(struct ref_array_item *item);
/* Clear all memory allocated to ref_array */
void ref_array_clear(struct ref_array *array);
/* Parse format string and sort specifiers */
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index dcf2931..db3627e 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -38,11 +38,11 @@ test_expect_success 'fast-import: fail on invalid branch
name "bad[branch]name"'
test_must_fail git fast-import output &&
- grep -e "broken\.\.\.ref" output
+test_expect_success 'git branch shows badly named ref as warning' '
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+ git branch 2>output &&
+ grep -e "broken\.\.\.ref" output
'
test_expect_success 'branch -d can delete badly named ref' '
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 1deb7cb..c819f3e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -144,21 +144,21 @@ EOF
'
test_expect_success 'git branch `--sort` option' '
- cat >expect
[PATCH v13 11/12] tag.c: implement '--format' option
Implement the '--format' option provided by 'ref-filter'.
This lets the user list tags as per desired format similar
to the implementation in 'git for-each-ref'.
Add tests and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-tag.txt | 8 +++-
builtin/tag.c | 19 +++
t/t7004-tag.sh| 12
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 3ac4a96..0c7f4e6 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,8 @@ SYNOPSIS
[ | ]
'git tag' -d ...
'git tag' [-n[]] -l [--contains ] [--points-at ]
- [--column[=] | --no-column] [--create-reflog] [--sort=]
[...]
+ [--column[=] | --no-column] [--create-reflog] [--sort=]
+ [--format=] [...]
'git tag' -v ...
DESCRIPTION
@@ -158,6 +159,11 @@ This option is only applicable when listing tags without
annotation lines.
The object that the new tag will refer to, usually a commit.
Defaults to HEAD.
+::
+ A string that interpolates `%(fieldname)` from the object
+ pointed at by a ref being shown. The format is the same as
+ that of linkgit:git-for-each-ref[1]. When unspecified,
+ defaults to `%(refname:short)`.
CONFIGURATION
-
diff --git a/builtin/tag.c b/builtin/tag.c
index 501fc52..4b8d6df 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,17 +23,16 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m | -F ]
[]"),
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
- "\n\t\t[...]"),
+ "\n\t\t[--format=] [...]"),
N_("git tag -v ..."),
NULL
};
static unsigned int colopts;
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
const char *format)
{
struct ref_array array;
- char *format;
int i;
memset(&array, 0, sizeof(array));
@@ -41,10 +40,12 @@ static int list_tags(struct ref_filter *filter, struct
ref_sorting *sorting)
if (filter->lines == -1)
filter->lines = 0;
- if (filter->lines)
- format = "%(align:16,left)%(refname:short)%(end)";
- else
- format = "%(refname:short)";
+ if (!format) {
+ if (filter->lines)
+ format = "%(align:16,left)%(refname:short)%(end)";
+ else
+ format = "%(refname:short)";
+ }
verify_ref_format(format);
filter_refs(&array, filter, FILTER_REFS_TAGS);
@@ -327,6 +328,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;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -359,6 +361,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at,
N_("object"),
N_("print only tags of the object"), 0,
parse_opt_object_name
},
+ OPT_STRING( 0 , "format", &format, N_("format"), N_("format to
use for the output")),
OPT_END()
};
@@ -399,7 +402,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
run_column_filter(colopts, &copts);
}
filter.name_patterns = argv;
- ret = list_tags(&filter, sorting);
+ ret = list_tags(&filter, sorting, format);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 84153ef..8987fb1 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1519,4 +1519,16 @@ EOF"
test_cmp expect actual
'
+test_expect_success '--format should list tags as per format given' '
+ cat >expect <<-\EOF &&
+ refname : refs/tags/foo1.10
+ refname : refs/tags/foo1.3
+ refname : refs/tags/foo1.6
+ refname : refs/tags/foo1.6-rc1
+ refname : refs/tags/foo1.6-rc2
+ EOF
+ git tag -l --format="refname : %(refname)" "foo*" >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v13 06/12] ref-filter: support printing N lines from tag annotation
From: Karthik Nayak
In 'tag.c' we can print N lines from the annotation of the tag using
the '-n' option. Copy code from 'tag.c' to 'ref-filter' and
modify 'ref-filter' to support printing of N lines from the annotation
of tags.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/for-each-ref.c | 2 +-
builtin/tag.c | 4
ref-filter.c | 51 +-
ref-filter.h | 11 ---
4 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 40f343b..e4a4f8a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -74,7 +74,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char
*prefix)
if (!maxcount || array.nr < maxcount)
maxcount = array.nr;
for (i = 0; i < maxcount; i++)
- show_ref_array_item(array.items[i], format, quote_style);
+ show_ref_array_item(array.items[i], format, quote_style, 0);
ref_array_clear(&array);
return 0;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 471d6b1..0fc7557 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -185,6 +185,10 @@ static enum contains_result contains(struct commit
*candidate,
return contains_test(candidate, want);
}
+/*
+ * Currently duplicated in ref-filter, will eventually be removed as
+ * we port tag.c to use ref-filter APIs.
+ */
static void show_tag_lines(const struct object_id *oid, int lines)
{
int i;
diff --git a/ref-filter.c b/ref-filter.c
index d5fae1a..515147b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1431,7 +1431,51 @@ static void append_literal(const char *cp, const char
*ep, struct ref_formatting
}
}
-void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
+/*
+ * If 'lines' is greater than 0, print that many lines from the given
+ * object_id 'oid'.
+ */
+static void show_tag_lines(const struct object_id *oid, int lines)
+{
+ int i;
+ unsigned long size;
+ enum object_type type;
+ char *buf, *sp, *eol;
+ size_t len;
+
+ buf = read_sha1_file(oid->hash, &type, &size);
+ if (!buf)
+ die_errno("unable to read object %s", oid_to_hex(oid));
+ if (type != OBJ_COMMIT && type != OBJ_TAG)
+ goto free_return;
+ if (!size)
+ die("an empty %s object %s?",
+ typename(type), oid_to_hex(oid));
+
+ /* skip header */
+ sp = strstr(buf, "\n\n");
+ if (!sp)
+ goto free_return;
+
+ /* only take up to "lines" lines, and strip the signature from a tag */
+ if (type == OBJ_TAG)
+ size = parse_signature(buf, size);
+ for (i = 0, sp += 2; i < lines && sp < buf + size; i++) {
+ if (i)
+ printf("\n");
+ eol = memchr(sp, '\n', size - (sp - buf));
+ len = eol ? eol - sp : size - (sp - buf);
+ fwrite(sp, len, 1, stdout);
+ if (!eol)
+ break;
+ sp = eol + 1;
+ }
+free_return:
+ free(buf);
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format,
+int quote_style, unsigned int lines)
{
const char *cp, *sp, *ep;
struct strbuf *final_buf;
@@ -1478,6 +1522,11 @@ void show_ref_array_item(struct ref_array_item *info,
const char *format, int qu
final_buf = &state.stack->output;
fwrite(final_buf->buf, 1, final_buf->len, stdout);
pop_stack_element(&state.stack);
+ if (lines > 0) {
+ struct object_id oid;
+ hashcpy(oid.hash, info->objectname);
+ show_tag_lines(&oid, lines);
+ }
putchar('\n');
}
diff --git a/ref-filter.h b/ref-filter.h
index 99f081b..c599ea2 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -58,7 +58,8 @@ struct ref_filter {
struct commit *merge_commit;
unsigned int with_commit_tag_algo : 1;
- unsigned int kind;
+ unsigned int kind,
+ lines;
};
struct ref_filter_cbdata {
@@ -90,8 +91,12 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
int verify_ref_format(const char *format);
/* Sort the given ref_array as per the ref_sorting provided */
void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
-/* Print the ref using the given format and quote_style */
-void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style);
+/*
+ * Print the ref using the given format and quote_style. If 'lines' > 0,
+ * print that many lines of the the given ref.
+ */
+void show_ref_array_item(struct ref_array_item *info, const char *format,
+int quote_style, unsigned int lines);
/* Callback function for parsing the sort option */
int
[PATCH v13 04/12] ref-filter: implement an `align` atom
Implement an `align` atom which left-, middle-, or right-aligns the
content between %(align:..) and %(end).
It is followed by `:,`, where the `` is
either left, right or middle and `` is the size of the area
into which the content will be placed. If the content between
%(align:) and %(end) is more than the width then no alignment is
performed. e.g. to align a refname atom to the middle with a total
width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
We now have a `handler()` for each atom_value which will be called
when that atom_value is being parsed, and similarly an `at_end`
function for each element of the stack which is to be called when the
`end` atom is encountered. Using this we implement the `align` atom
which aligns the given strbuf by calling `strbuf_utf8_align()` from
utf8.c.
Extract perform_quote_formatting() from append_atom(). Given a string
a quote_value and a strbuf, perform_quote_formatting() formats the
string based on the quote_value and stores it into the strbuf.
Ensure that quote formatting is performed on the whole of
%(align)...%(end) rather than individual atoms. We do this by skipping
individual quote formatting for atoms whenever the stack has more than
one element, and performing formatting for the entire stack element
when the `%(end)` atoms is encountered.
Add documentation and tests for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 9 +++
ref-filter.c | 124 ++---
t/t6302-for-each-ref-filter.sh | 69 +
3 files changed, 192 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index e49d578..943975d 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,15 @@ color::
Change output color. Followed by `:`, where names
are described in `color.branch.*`.
+align::
+ Left-, middle-, or right-align the content between %(align:..)
+ and %(end). Followed by `:,`, where the
+ `` is either left, right or middle and `` is
+ the total length of the content with alignment. If the
+ contents length is more than the width then no alignment is
+ performed. If used with '--quote' everything in between %(align:..)
+ and %(end) is quoted.
+
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index d0d8df0..ffec10a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
#include "quote.h"
#include "ref-filter.h"
#include "revision.h"
+#include "utf8.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -53,6 +54,13 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color" },
+ { "align" },
+ { "end" },
+};
+
+struct align {
+ align_type position;
+ unsigned int width;
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -60,6 +68,8 @@ static struct {
struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
+ void (*at_end)(struct ref_formatting_stack *stack);
+ void *cb_data;
};
struct ref_formatting_state {
@@ -69,6 +79,8 @@ struct ref_formatting_state {
struct atom_value {
const char *s;
+ struct align *align;
+ void (*handler)(struct atom_value *atomv, struct ref_formatting_state
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
};
@@ -632,6 +644,51 @@ static inline char *copy_advance(char *dst, const char
*src)
return dst;
}
+static void align_handler(struct ref_formatting_stack *stack)
+{
+ struct align *align = (struct align *)stack->cb_data;
+ struct strbuf s = STRBUF_INIT;
+
+ strbuf_utf8_align(&s, align->position, align->width, stack->output.buf);
+ strbuf_swap(&stack->output, &s);
+ strbuf_release(&s);
+ free(align);
+}
+
+static void align_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+ struct ref_formatting_stack *new;
+
+ push_new_stack_element(&state->stack);
+ new = state->stack;
+ new->at_end = align_handler;
+ new->cb_data = atomv->align;
+}
+
+static void perform_quote_formatting(struct strbuf *s, const char *str, int
quote_style);
+
+static void end_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+ struct ref_formatting_stack *current = state->stack;
+ struct strbuf s = STRBUF_INIT;
+
+ if (!current->at_end)
+ die(_("format: `end` atom used without a supporting atom"));
+ current->at_end(current);
+ /*
+* Whenever we have more than one stack element that means we
+
[PATCH v13 03/12] utf8: add function to align a string into given strbuf
Add strbuf_utf8_align() which will align a given string into a strbuf
as per given align_type and width. If the width is greater than the
string length then no alignment is performed.
Helped-by: Eric Sunshine
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
utf8.c | 21 +
utf8.h | 15 +++
2 files changed, 36 insertions(+)
diff --git a/utf8.c b/utf8.c
index 28e6d76..00e10c8 100644
--- a/utf8.c
+++ b/utf8.c
@@ -644,3 +644,24 @@ int skip_utf8_bom(char **text, size_t len)
*text += strlen(utf8_bom);
return 1;
}
+
+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int
width,
+ const char *s)
+{
+ int slen = strlen(s);
+ int display_len = utf8_strnwidth(s, slen, 0);
+ int utf8_compensation = slen - display_len;
+
+ if (display_len >= width) {
+ strbuf_addstr(buf, s);
+ return;
+ }
+
+ if (position == ALIGN_LEFT)
+ strbuf_addf(buf, "%-*s", width + utf8_compensation, s);
+ else if (position == ALIGN_MIDDLE) {
+ int left = (width - display_len) / 2;
+ strbuf_addf(buf, "%*s%-*s", left, "", width - left +
utf8_compensation, s);
+ } else if (position == ALIGN_RIGHT)
+ strbuf_addf(buf, "%*s", width + utf8_compensation, s);
+}
diff --git a/utf8.h b/utf8.h
index 5a9e94b..7930b44 100644
--- a/utf8.h
+++ b/utf8.h
@@ -55,4 +55,19 @@ int mbs_chrlen(const char **text, size_t *remainder_p, const
char *encoding);
*/
int is_hfs_dotgit(const char *path);
+typedef enum {
+ ALIGN_LEFT,
+ ALIGN_MIDDLE,
+ ALIGN_RIGHT
+} align_type;
+
+/*
+ * Align the string given and store it into a strbuf as per the
+ * 'position' and 'width'. If the given string length is larger than
+ * 'width' than then the input string is not truncated and no
+ * alignment is done.
+ */
+void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int
width,
+ const char *s);
+
#endif
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v13 00/12] port tag.c to use ref-filter APIs
Part of my GSoC project to port tag.c to use ref-filter APIs. This is
a follow up to porting for-each-ref to use ref-filter APIs.
Version 12 can be found here:
thread.gmane.org/gmane.comp.version-control.git/276133
Changes since v12:
* %(align)...%(end) now quote formats everything in between the atoms
even if they are string literals.
* Changet the structure of ref_formatting_state to hold ref_formatting_stack
as the stack and quote_value, this ensures that we do not need to copy the
quote_value to each element of the stack.
* While checking for %(align) atom, use strbuf_split_str().
* In tag.c support usage of --format with -n.
Karthik Nayak (12):
ref-filter: move `struct atom_value` to ref-filter.c
ref-filter: introduce ref_formatting_state and ref_formatting_stack
utf8: add function to align a string into given strbuf
ref-filter: implement an `align` atom
ref-filter: add option to filter out tags, branches and remotes
ref-filter: support printing N lines from tag annotation
ref-filter: add support to sort by version
ref-filter: add option to match literal pattern
tag.c: use 'ref-filter' data structures
tag.c: use 'ref-filter' APIs
tag.c: implement '--format' option
tag.c: implement '--merged' and '--no-merged' options
Documentation/git-for-each-ref.txt | 12 ++
Documentation/git-tag.txt | 27 ++-
builtin/for-each-ref.c | 3 +-
builtin/tag.c | 365 +++--
ref-filter.c | 350 +++
ref-filter.h | 32 +++-
refs.c | 9 +
refs.h | 1 +
t/t6302-for-each-ref-filter.sh | 105 +++
t/t7004-tag.sh | 47 -
utf8.c | 21 +++
utf8.h | 15 ++
12 files changed, 626 insertions(+), 361 deletions(-)
Interdiff:
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 1997657..06d468e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -133,7 +133,8 @@ align::
`` is either left, right or middle and `` is
the total length of the content with alignment. If the
contents length is more than the width then no alignment is
- performed.
+ performed. If used with '--quote' everything in between %(align:..)
+ and %(end) is quoted.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index c2785d9..3803bf7 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -161,22 +161,15 @@ This option is only applicable when listing tags without
annotation lines.
::
A string that interpolates `%(fieldname)` from the object
- pointed at by a ref being shown. If `fieldname` is prefixed
- with an asterisk (`*`) and the ref points at a tag object, the
- value for the field in the object tag refers is used. When
- unspecified, defaults to `%(refname:short)`. It also
- interpolates `%%` to `%`, and `%xx` where `xx` are hex digits
- interpolates to character with hex code `xx`; for example
- `%00` interpolates to `\0` (NUL), `%09` to `\t` (TAB) and
- `%0a` to `\n` (LF). The fields are same as those in `git
- for-each-ref`.
+ pointed at by a ref being shown. The format is the same as
+ that of linkgit:git-for-each-ref[1]. When unspecified,
+ defaults to `%(refname:short)`.
--[no-]merged []::
Only list tags whose tips are reachable, or not reachable
if '--no-merged' is used, from the specified commit ('HEAD'
if not specified).
-
CONFIGURATION
-
By default, 'git tag' in sign-with-default mode (-s) will use your
diff --git a/ref-filter.c b/ref-filter.c
index 665221b..f8b8fb7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -65,18 +65,24 @@ struct align {
unsigned int width;
};
-struct ref_formatting_state {
- struct ref_formatting_state *prev;
+#define REF_FORMATTING_STATE_INIT { 0, NULL }
+
+struct ref_formatting_stack {
+ struct ref_formatting_stack *prev;
struct strbuf output;
- void (*at_end)(struct ref_formatting_state *state);
+ void (*at_end)(struct ref_formatting_stack *stack);
void *cb_data;
+};
+
+struct ref_formatting_state {
int quote_style;
+ struct ref_formatting_stack *stack;
};
struct atom_value {
const char *s;
struct align *align;
- void (*handler)(struct atom_value *atomv, struct ref_formatting_state
**state);
+ void (*handler)(struct atom_value *atomv, struct ref_formatting_state
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
};
@@ -149,19 +155,19 @@ int parse_re
[PATCH v13 02/12] ref-filter: introduce ref_formatting_state and ref_formatting_stack
Introduce ref_formatting_state which will hold the formatted output
strbuf instead of directly printing to stdout. This will help us in
creating modifier atoms which modify the format specified before
printing to stdout.
Implement a stack machinery for ref_formatting_state, this allows us
to push and pop elements onto the stack. Whenever we pop an element
from the stack, the strbuf from that element is appended to the strbuf
of the next element on the stack, this will allow us to support
nesting of modifier atoms.
Rename some functions to reflect the changes made:
print_value() -> append_atom()
emit()-> append_literal()
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 78 +---
1 file changed, 59 insertions(+), 19 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index e53c77e..d0d8df0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,18 @@ static struct {
{ "color" },
};
+#define REF_FORMATTING_STATE_INIT { 0, NULL }
+
+struct ref_formatting_stack {
+ struct ref_formatting_stack *prev;
+ struct strbuf output;
+};
+
+struct ref_formatting_state {
+ int quote_style;
+ struct ref_formatting_stack *stack;
+};
+
struct atom_value {
const char *s;
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -129,6 +141,27 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
return at;
}
+static void push_new_stack_element(struct ref_formatting_stack **stack)
+{
+ struct ref_formatting_stack *s = xcalloc(1, sizeof(struct
ref_formatting_stack));
+
+ strbuf_init(&s->output, 0);
+ s->prev = *stack;
+ *stack = s;
+}
+
+static void pop_stack_element(struct ref_formatting_stack **stack)
+{
+ struct ref_formatting_stack *current = *stack;
+ struct ref_formatting_stack *prev = current->prev;
+
+ if (prev)
+ strbuf_addbuf(&prev->output, ¤t->output);
+ strbuf_release(¤t->output);
+ free(current);
+ *stack = prev;
+}
+
/*
* In a format string, find the next occurrence of %(atom).
*/
@@ -1195,30 +1228,27 @@ void ref_array_sort(struct ref_sorting *sorting, struct
ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *),
compare_refs);
}
-static void print_value(struct atom_value *v, int quote_style)
+static void append_atom(struct atom_value *v, struct ref_formatting_state
*state)
{
- struct strbuf sb = STRBUF_INIT;
- switch (quote_style) {
+ struct strbuf *s = &state->stack->output;
+
+ switch (state->quote_style) {
case QUOTE_NONE:
- fputs(v->s, stdout);
+ strbuf_addstr(s, v->s);
break;
case QUOTE_SHELL:
- sq_quote_buf(&sb, v->s);
+ sq_quote_buf(s, v->s);
break;
case QUOTE_PERL:
- perl_quote_buf(&sb, v->s);
+ perl_quote_buf(s, v->s);
break;
case QUOTE_PYTHON:
- python_quote_buf(&sb, v->s);
+ python_quote_buf(s, v->s);
break;
case QUOTE_TCL:
- tcl_quote_buf(&sb, v->s);
+ tcl_quote_buf(s, v->s);
break;
}
- if (quote_style != QUOTE_NONE) {
- fputs(sb.buf, stdout);
- strbuf_release(&sb);
- }
}
static int hex1(char ch)
@@ -1239,8 +1269,10 @@ static int hex2(const char *cp)
return -1;
}
-static void emit(const char *cp, const char *ep)
+static void append_literal(const char *cp, const char *ep, struct
ref_formatting_state *state)
{
+ struct strbuf *s = &state->stack->output;
+
while (*cp && (!ep || cp < ep)) {
if (*cp == '%') {
if (cp[1] == '%')
@@ -1248,13 +1280,13 @@ static void emit(const char *cp, const char *ep)
else {
int ch = hex2(cp + 1);
if (0 <= ch) {
- putchar(ch);
+ strbuf_addch(s, ch);
cp += 3;
continue;
}
}
}
- putchar(*cp);
+ strbuf_addch(s, *cp);
cp++;
}
}
@@ -1262,19 +1294,24 @@ static void emit(const char *cp, const char *ep)
void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
{
const char *cp, *sp, *ep;
+ struct strbuf *final_buf;
+ struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+
+ state.quote_style = quote_style;
+ push_new_stack_element(&state.stack);
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
[PATCH v13 08/12] ref-filter: add option to match literal pattern
From: Karthik Nayak
Since 'ref-filter' only has an option to match path names add an
option for plain fnmatch pattern-matching.
This is to support the pattern matching options which are used in `git
tag -l` and `git branch -l` where we can match patterns like `git tag
-l foo*` which would match all tags which has a "foo*" pattern.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/for-each-ref.c | 1 +
ref-filter.c | 40 +---
ref-filter.h | 3 ++-
3 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e4a4f8a..3ad6a64 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -68,6 +68,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char
*prefix)
git_config(git_default_config, NULL);
filter.name_patterns = argv;
+ filter.match_as_path = 1;
filter_refs(&array, &filter, FILTER_REFS_ALL |
FILTER_REFS_INCLUDE_BROKEN);
ref_array_sort(sorting, &array);
diff --git a/ref-filter.c b/ref-filter.c
index b4a7d72..f8b8fb7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1055,9 +1055,33 @@ static int commit_contains(struct ref_filter *filter,
struct commit *commit)
/*
* Return 1 if the refname matches one of the patterns, otherwise 0.
+ * A pattern can be a literal prefix (e.g. a refname "refs/heads/master"
+ * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
+ * matches "refs/heads/mas*", too).
+ */
+static int match_pattern(const char **patterns, const char *refname)
+{
+ /*
+* When no '--format' option is given we need to skip the prefix
+* for matching refs of tags and branches.
+*/
+ (void)(skip_prefix(refname, "refs/tags/", &refname) ||
+ skip_prefix(refname, "refs/heads/", &refname) ||
+ skip_prefix(refname, "refs/remotes/", &refname) ||
+ skip_prefix(refname, "refs/", &refname));
+
+ for (; *patterns; patterns++) {
+ if (!wildmatch(*patterns, refname, 0, NULL))
+ return 1;
+ }
+ return 0;
+}
+
+/*
+ * Return 1 if the refname matches one of the patterns, otherwise 0.
* A pattern can be path prefix (e.g. a refname "refs/heads/master"
- * matches a pattern "refs/heads/") or a wildcard (e.g. the same ref
- * matches "refs/heads/m*",too).
+ * matches a pattern "refs/heads/" but not "refs/heads/m") or a
+ * wildcard (e.g. the same ref matches "refs/heads/m*", too).
*/
static int match_name_as_path(const char **pattern, const char *refname)
{
@@ -1078,6 +1102,16 @@ static int match_name_as_path(const char **pattern,
const char *refname)
return 0;
}
+/* Return 1 if the refname matches one of the patterns, otherwise 0. */
+static int filter_pattern_match(struct ref_filter *filter, const char *refname)
+{
+ if (!*filter->name_patterns)
+ return 1; /* No pattern always matches */
+ if (filter->match_as_path)
+ return match_name_as_path(filter->name_patterns, refname);
+ return match_pattern(filter->name_patterns, refname);
+}
+
/*
* Given a ref (sha1, refname), check if the ref belongs to the array
* of sha1s. If the given ref is a tag, check if the given tag points
@@ -1181,7 +1215,7 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
if (!(kind & filter->kind))
return 0;
- if (*filter->name_patterns &&
!match_name_as_path(filter->name_patterns, refname))
+ if (!filter_pattern_match(filter, refname))
return 0;
if (filter->points_at.nr && !match_points_at(&filter->points_at,
oid->hash, refname))
diff --git a/ref-filter.h b/ref-filter.h
index 5aa2f40..8241066 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -58,7 +58,8 @@ struct ref_filter {
} merge;
struct commit *merge_commit;
- unsigned int with_commit_tag_algo : 1;
+ unsigned int with_commit_tag_algo : 1,
+ match_as_path : 1;
unsigned int kind,
lines;
};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v13 07/12] ref-filter: add support to sort by version
From: Karthik Nayak
Add support to sort by version using the "v:refname" and
"version:refname" option. This is achieved by using the 'versioncmp()'
function as the comparing function for qsort.
This option is included to support sorting by versions in `git tag -l`
which will eventually be ported to use ref-filter APIs.
Add documentation and tests for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 3 +++
ref-filter.c | 15 ++-
ref-filter.h | 3 ++-
t/t6302-for-each-ref-filter.sh | 36
4 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 943975d..06d468e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -154,6 +154,9 @@ For sorting purposes, fields with numeric values sort in
numeric
order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
All other fields are used to sort in their byte-value order.
+There is also an option to sort by versions, this can be done by using
+the fieldname `version:refname` or its alias `v:refname`.
+
In any case, a field name that refers to a field inapplicable to
the object referred by the ref does not cause an error. It
returns an empty string instead.
diff --git a/ref-filter.c b/ref-filter.c
index 515147b..b4a7d72 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -11,6 +11,8 @@
#include "ref-filter.h"
#include "revision.h"
#include "utf8.h"
+#include "git-compat-util.h"
+#include "version.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -1327,19 +1329,19 @@ static int cmp_ref_sorting(struct ref_sorting *s,
struct ref_array_item *a, stru
get_ref_atom_value(a, s->atom, &va);
get_ref_atom_value(b, s->atom, &vb);
- switch (cmp_type) {
- case FIELD_STR:
+ if (s->version)
+ cmp = versioncmp(va->s, vb->s);
+ else if (cmp_type == FIELD_STR)
cmp = strcmp(va->s, vb->s);
- break;
- default:
+ else {
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
cmp = 0;
else
cmp = 1;
- break;
}
+
return (s->reverse) ? -cmp : cmp;
}
@@ -1559,6 +1561,9 @@ int parse_opt_ref_sorting(const struct option *opt, const
char *arg, int unset)
s->reverse = 1;
arg++;
}
+ if (skip_prefix(arg, "version:", &arg) ||
+ skip_prefix(arg, "v:", &arg))
+ s->version = 1;
len = strlen(arg);
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
diff --git a/ref-filter.h b/ref-filter.h
index c599ea2..5aa2f40 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -27,7 +27,8 @@ struct atom_value;
struct ref_sorting {
struct ref_sorting *next;
int atom; /* index into used_atom array (internal) */
- unsigned reverse : 1;
+ unsigned reverse : 1,
+ version : 1;
};
struct ref_array_item {
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 227992b..38c99c9 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -150,4 +150,40 @@ test_expect_success 'alignment with format quote' '
test_cmp expect actual
'
+test_expect_success 'setup for version sort' '
+ test_commit foo1.3 &&
+ test_commit foo1.6 &&
+ test_commit foo1.10
+'
+
+test_expect_success 'version sort' '
+ git for-each-ref --sort=version:refname --format="%(refname:short)"
refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'version sort (shortened)' '
+ git for-each-ref --sort=v:refname --format="%(refname:short)"
refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'reverse version sort' '
+ git for-each-ref --sort=-version:refname --format="%(refname:short)"
refs/tags/ | grep "foo" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.6
+ foo1.3
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v13 05/12] ref-filter: add option to filter out tags, branches and remotes
From: Karthik Nayak
Add a function called 'for_each_reftype_fullpath()' to refs.{c,h}
which iterates through each ref for the given path without trimming
the path and also accounting for broken refs, if mentioned.
Add 'filter_ref_kind()' in ref-filter.c to check the kind of ref being
handled and return the kind to 'ref_filter_handler()', where we
discard refs which we do not need and assign the kind to needed refs.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 59 ++-
ref-filter.h | 12 ++--
refs.c | 9 +
refs.h | 1 +
4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ffec10a..d5fae1a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1123,6 +1123,36 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
return ref;
}
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+ unsigned int i;
+
+ static struct {
+ const char *prefix;
+ unsigned int kind;
+ } ref_kind[] = {
+ { "refs/heads/" , FILTER_REFS_BRANCHES },
+ { "refs/remotes/" , FILTER_REFS_REMOTES },
+ { "refs/tags/", FILTER_REFS_TAGS}
+ };
+
+ if (filter->kind == FILTER_REFS_BRANCHES)
+ return FILTER_REFS_BRANCHES;
+ else if (filter->kind == FILTER_REFS_REMOTES)
+ return FILTER_REFS_REMOTES;
+ else if (filter->kind == FILTER_REFS_TAGS)
+ return FILTER_REFS_TAGS;
+ else if (!strcmp(refname, "HEAD"))
+ return FILTER_REFS_DETACHED_HEAD;
+
+ for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
+ if (starts_with(refname, ref_kind[i].prefix))
+ return ref_kind[i].kind;
+ }
+
+ return FILTER_REFS_OTHERS;
+}
+
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
@@ -1133,6 +1163,7 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
struct ref_filter *filter = ref_cbdata->filter;
struct ref_array_item *ref;
struct commit *commit = NULL;
+ unsigned int kind;
if (flag & REF_BAD_NAME) {
warning("ignoring ref with broken name %s", refname);
@@ -1144,6 +1175,10 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
return 0;
}
+ kind = filter_ref_kind(filter, refname);
+ if (!(kind & filter->kind))
+ return 0;
+
if (*filter->name_patterns &&
!match_name_as_path(filter->name_patterns, refname))
return 0;
@@ -1175,6 +1210,7 @@ static int ref_filter_handler(const char *refname, const
struct object_id *oid,
REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
+ ref->kind = kind;
return 0;
}
@@ -1251,16 +1287,29 @@ int filter_refs(struct ref_array *array, struct
ref_filter *filter, unsigned int
{
struct ref_filter_cbdata ref_cbdata;
int ret = 0;
+ unsigned int broken = 0;
ref_cbdata.array = array;
ref_cbdata.filter = filter;
/* Simple per-ref filtering */
- if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
- ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
- else if (type & FILTER_REFS_ALL)
- ret = for_each_ref(ref_filter_handler, &ref_cbdata);
- else if (type)
+ if (type & FILTER_REFS_INCLUDE_BROKEN) {
+ type &= ~FILTER_REFS_INCLUDE_BROKEN;
+ broken = 1;
+ }
+
+ filter->kind = type;
+ if (type == FILTER_REFS_BRANCHES)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
"refs/heads/", broken, &ref_cbdata);
+ else if (type == FILTER_REFS_REMOTES)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
"refs/remotes/", broken, &ref_cbdata);
+ else if (type == FILTER_REFS_TAGS)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
"refs/tags/", broken, &ref_cbdata);
+ else if (type & FILTER_REFS_ALL) {
+ ret = for_each_reftype_fullpath(ref_filter_handler, "", broken,
&ref_cbdata);
+ if (type & FILTER_REFS_DETACHED_HEAD)
+ head_ref(ref_filter_handler, &ref_cbdata);
+ } else
die("filter_refs: invalid type");
/* Filters that need revision walking */
diff --git a/ref-filter.h b/ref-filter.h
index 45026d0..99f081b 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -13,8 +13,14 @@
#define QUOTE_PYTHON 4
#define QUOTE_TCL 8
-#define FILTER_REFS_INCLUDE_BROKEN 0x1
-#define FILTER_REFS_ALL 0x2
+#define FILTER_REFS_INCLUDE_BROKEN 0x0001
+#define FILTER_REFS_T
[PATCH v13 09/12] tag.c: use 'ref-filter' data structures
From: Karthik Nayak
Make 'tag.c' use 'ref-filter' data structures and make changes to
support the new data structures. This is a part of the process
of porting 'tag.c' to use 'ref-filter' APIs.
This is a temporary step before porting 'tag.c' to use 'ref-filter'
completely. As this is a temporary step, most of the code
introduced here will be removed when 'tag.c' is ported over to use
'ref-filter' APIs.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/tag.c | 106 +++---
1 file changed, 57 insertions(+), 49 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 0fc7557..e96bae2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -17,6 +17,7 @@
#include "gpg-interface.h"
#include "sha1-array.h"
#include "column.h"
+#include "ref-filter.h"
static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m | -F ]
[]"),
@@ -34,15 +35,6 @@ static const char * const git_tag_usage[] = {
static int tag_sort;
-struct tag_filter {
- const char **patterns;
- int lines;
- int sort;
- struct string_list tags;
- struct commit_list *with_commit;
-};
-
-static struct sha1_array points_at;
static unsigned int colopts;
static int match_pattern(const char **patterns, const char *ref)
@@ -61,19 +53,20 @@ static int match_pattern(const char **patterns, const char
*ref)
* removed as we port tag.c to use the ref-filter APIs.
*/
static const unsigned char *match_points_at(const char *refname,
- const unsigned char *sha1)
+ const unsigned char *sha1,
+ struct sha1_array *points_at)
{
const unsigned char *tagged_sha1 = NULL;
struct object *obj;
- if (sha1_array_lookup(&points_at, sha1) >= 0)
+ if (sha1_array_lookup(points_at, sha1) >= 0)
return sha1;
obj = parse_object(sha1);
if (!obj)
die(_("malformed object at '%s'"), refname);
if (obj->type == OBJ_TAG)
tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
- if (tagged_sha1 && sha1_array_lookup(&points_at, tagged_sha1) >= 0)
+ if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
return tagged_sha1;
return NULL;
}
@@ -228,12 +221,24 @@ free_return:
free(buf);
}
+static void ref_array_append(struct ref_array *array, const char *refname)
+{
+ size_t len = strlen(refname);
+ struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) +
len + 1);
+ memcpy(ref->refname, refname, len);
+ ref->refname[len] = '\0';
+ REALLOC_ARRAY(array->items, array->nr + 1);
+ array->items[array->nr++] = ref;
+}
+
static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
- struct tag_filter *filter = cb_data;
+ struct ref_filter_cbdata *data = cb_data;
+ struct ref_array *array = data->array;
+ struct ref_filter *filter = data->filter;
- if (match_pattern(filter->patterns, refname)) {
+ if (match_pattern(filter->name_patterns, refname)) {
if (filter->with_commit) {
struct commit *commit;
@@ -244,12 +249,12 @@ static int show_reference(const char *refname, const
struct object_id *oid,
return 0;
}
- if (points_at.nr && !match_points_at(refname, oid->hash))
+ if (filter->points_at.nr && !match_points_at(refname,
oid->hash, &filter->points_at))
return 0;
if (!filter->lines) {
- if (filter->sort)
- string_list_append(&filter->tags, refname);
+ if (tag_sort)
+ ref_array_append(array, refname);
else
printf("%s\n", refname);
return 0;
@@ -264,36 +269,36 @@ static int show_reference(const char *refname, const
struct object_id *oid,
static int sort_by_version(const void *a_, const void *b_)
{
- const struct string_list_item *a = a_;
- const struct string_list_item *b = b_;
- return versioncmp(a->string, b->string);
+ const struct ref_array_item *a = *((struct ref_array_item **)a_);
+ const struct ref_array_item *b = *((struct ref_array_item **)b_);
+ return versioncmp(a->refname, b->refname);
}
-static int list_tags(const char **patterns, int lines,
-struct commit_list *with_commit, int sort)
+static int list_tags(struct ref_filter *filter, int sort)
{
- struct tag_filter filter;
+ struct ref_array array;
+ struct ref_filter_cbdata data;
+
+ memset(&array, 0, si
[PATCH v13 12/12] tag.c: implement '--merged' and '--no-merged' options
Use 'ref-filter' APIs to implement the '--merged' and '--no-merged'
options into 'tag.c'. The '--merged' option lets the user to only list
tags merged into the named commit. The '--no-merged' option lets the
user to only list tags not merged into the named commit. If no object
is provided it assumes HEAD as the object.
Add documentation and tests for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-tag.txt | 7 ++-
builtin/tag.c | 6 +-
t/t7004-tag.sh| 27 +++
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 0c7f4e6..3803bf7 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -14,7 +14,7 @@ SYNOPSIS
'git tag' -d ...
'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
- [--format=] [...]
+ [--format=] [--[no-]merged []] [...]
'git tag' -v ...
DESCRIPTION
@@ -165,6 +165,11 @@ This option is only applicable when listing tags without
annotation lines.
that of linkgit:git-for-each-ref[1]. When unspecified,
defaults to `%(refname:short)`.
+--[no-]merged []::
+ Only list tags whose tips are reachable, or not reachable
+ if '--no-merged' is used, from the specified commit ('HEAD'
+ if not specified).
+
CONFIGURATION
-
By default, 'git tag' in sign-with-default mode (-s) will use your
diff --git a/builtin/tag.c b/builtin/tag.c
index 4b8d6df..bbbcaed 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {
N_("git tag [-a | -s | -u ] [-f] [-m | -F ]
[]"),
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
- "\n\t\t[--format=] [...]"),
+ "\n\t\t[--format=] [--[no-]merged []]
[...]"),
N_("git tag -v ..."),
NULL
};
@@ -355,6 +355,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_COLUMN(0, "column", &colopts, N_("show tag list in
columns")),
OPT_CONTAINS(&filter.with_commit, N_("print only tags that
contain the commit")),
OPT_WITH(&filter.with_commit, N_("print only tags that contain
the commit")),
+ OPT_MERGED(&filter, N_("print only tags that are merged")),
+ OPT_NO_MERGED(&filter, N_("print only tags that are not
merged")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
N_("field name to sort on"),
&parse_opt_ref_sorting),
{
@@ -413,6 +415,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
die(_("--contains option is only allowed with -l."));
if (filter.points_at.nr)
die(_("--points-at option is only allowed with -l."));
+ 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')
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8987fb1..3dd2f51 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1531,4 +1531,31 @@ test_expect_success '--format should list tags as per
format given' '
test_cmp expect actual
'
+test_expect_success 'setup --merged test tags' '
+ git tag mergetest-1 HEAD~2 &&
+ git tag mergetest-2 HEAD~1 &&
+ git tag mergetest-3 HEAD
+'
+
+test_expect_success '--merged cannot be used in non-list mode' '
+ test_must_fail git tag --merged=mergetest-2 foo
+'
+
+test_expect_success '--merged shows merged tags' '
+ cat >expect <<-\EOF &&
+ mergetest-1
+ mergetest-2
+ EOF
+ git tag -l --merged=mergetest-2 mergetest-* >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--no-merged show unmerged tags' '
+ cat >expect <<-\EOF &&
+ mergetest-3
+ EOF
+ git tag -l --no-merged=mergetest-2 mergetest-* >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v13 10/12] tag.c: use 'ref-filter' APIs
From: Karthik Nayak
Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting
and printing of refs. This removes most of the code used in 'tag.c'
replacing it with calls to the 'ref-filter' library.
Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'
to filter out tags based on the options set.
For printing tags we use 'show_ref_array_item()' function provided by
'ref-filter'.
We improve the sorting option provided by 'tag.c' by using the sorting
options provided by 'ref-filter'. This causes the test 'invalid sort
parameter on command line' in t7004 to fail, as 'ref-filter' throws an
error for all sorting fields which are incorrect. The test is changed
to reflect the same.
Modify documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-tag.txt | 16 ++-
builtin/tag.c | 342 ++
t/t7004-tag.sh| 8 +-
3 files changed, 50 insertions(+), 316 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 84f6496..3ac4a96 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[ | ]
'git tag' -d ...
'git tag' [-n[]] -l [--contains ] [--points-at ]
- [--column[=] | --no-column] [--create-reflog] [...]
+ [--column[=] | --no-column] [--create-reflog] [--sort=]
[...]
'git tag' -v ...
DESCRIPTION
@@ -94,14 +94,16 @@ OPTIONS
using fnmatch(3)). Multiple patterns may be given; if any of
them matches, the tag is shown.
---sort=::
- Sort in a specific order. Supported type is "refname"
- (lexicographic order), "version:refname" or "v:refname" (tag
+--sort=::
+ Sort based on the key given. Prefix `-` to sort in
+ descending order of the value. You may use the --sort= option
+ multiple times, in which case the last key becomes the primary
+ key. Also supports "version:refname" or "v:refname" (tag
names are treated as versions). The "version:refname" sort
order can also be affected by the
- "versionsort.prereleaseSuffix" configuration variable. Prepend
- "-" to reverse sort order. When this option is not given, the
- sort order defaults to the value configured for the 'tag.sort'
+ "versionsort.prereleaseSuffix" configuration variable.
+ The keys supported are the same as those in `git for-each-ref`.
+ Sort order defaults to the value configured for the 'tag.sort'
variable if it exists, or lexicographic order otherwise. See
linkgit:git-config[1].
diff --git a/builtin/tag.c b/builtin/tag.c
index e96bae2..501fc52 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -28,278 +28,32 @@ static const char * const git_tag_usage[] = {
NULL
};
-#define STRCMP_SORT 0 /* must be zero */
-#define VERCMP_SORT 1
-#define SORT_MASK 0x7fff
-#define REVERSE_SORT0x8000
-
-static int tag_sort;
-
static unsigned int colopts;
-static int match_pattern(const char **patterns, const char *ref)
-{
- /* no pattern means match everything */
- if (!*patterns)
- return 1;
- for (; *patterns; patterns++)
- if (!wildmatch(*patterns, ref, 0, NULL))
- return 1;
- return 0;
-}
-
-/*
- * This is currently duplicated in ref-filter.c, and will eventually be
- * removed as we port tag.c to use the ref-filter APIs.
- */
-static const unsigned char *match_points_at(const char *refname,
- const unsigned char *sha1,
- struct sha1_array *points_at)
-{
- const unsigned char *tagged_sha1 = NULL;
- struct object *obj;
-
- if (sha1_array_lookup(points_at, sha1) >= 0)
- return sha1;
- obj = parse_object(sha1);
- if (!obj)
- die(_("malformed object at '%s'"), refname);
- if (obj->type == OBJ_TAG)
- tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
- if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
- return tagged_sha1;
- return NULL;
-}
-
-static int in_commit_list(const struct commit_list *want, struct commit *c)
-{
- for (; want; want = want->next)
- if (!hashcmp(want->item->object.sha1, c->object.sha1))
- return 1;
- return 0;
-}
-
-/*
- * The entire code segment for supporting the --contains option has been
- * copied over to ref-filter.{c,h}. This will be deleted evetually when
- * we port tag.c to use ref-filter APIs.
- */
-enum contains_result {
- CONTAINS_UNKNOWN = -1,
- CONTAINS_NO = 0,
- CONTAINS_YES = 1
-};
-
-/*
- * Test whether the candidate or one of its parents is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
- */
-static enum contains_re
[PATCH v13 01/12] ref-filter: move `struct atom_value` to ref-filter.c
Since atom_value is only required for the internal working of
ref-filter it doesn't belong in the public header.
Helped-by: Eric Sunshine
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 5 +
ref-filter.h | 5 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 46963a5..e53c77e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,6 +55,11 @@ static struct {
{ "color" },
};
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
/*
* An atom is a valid field atom listed above, possibly prefixed with
* a "*" to denote deref_tag().
diff --git a/ref-filter.h b/ref-filter.h
index 6bf27d8..45026d0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -16,10 +16,7 @@
#define FILTER_REFS_INCLUDE_BROKEN 0x1
#define FILTER_REFS_ALL 0x2
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
+struct atom_value;
struct ref_sorting {
struct ref_sorting *next;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] gc: save log from daemonized gc --auto and print it next time
While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.
The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid "gc
--auto" running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.
Daemonized gc now saves stderr to $GIT_DIR/gc.log. Following gc runs
will not be daemonized and gc.log printed out until the user removes
gc.log.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Let's try again. Compared to v2 [1], this version does not delete
gc.log automatically any more. The user needs to take action, then
delete gc.log to bring it background again.
[1] http://thread.gmane.org/gmane.comp.version-control.git/266182/focus=266320
builtin/gc.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index bcc75d9..00a83e1 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -43,9 +43,20 @@ static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
static struct argv_array rerere = ARGV_ARRAY_INIT;
static char *pidfile;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
static void remove_pidfile(void)
{
+ if (daemonized && log_filename.len) {
+ struct stat st;
+
+ close(2);
+ if (stat(log_filename.buf, &st) ||
+ !st.st_size ||
+ rename(log_filename.buf, git_path("gc.log")))
+ unlink(log_filename.buf);
+ }
if (pidfile)
unlink(pidfile);
}
@@ -330,13 +341,24 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
fprintf(stderr, _("See \"git help gc\" for manual
housekeeping.\n"));
}
if (detach_auto) {
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_read_file(&sb, git_path("gc.log"), 0) > 0) {
+ warning(_("last gc run reported:\n"
+ "%s\n"
+ "running in foreground until %s is
removed"),
+ sb.buf, git_path("gc.log"));
+ detach_auto = 0;
+ }
+ strbuf_release(&sb);
+ }
+ if (detach_auto) {
if (gc_before_repack())
return -1;
/*
* failure to daemonize is ok, we'll continue
* in foreground
*/
- daemonize();
+ daemonized = !daemonize();
}
} else
add_repack_all_option();
@@ -349,6 +371,18 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
name, (uintmax_t)pid);
}
+ if (daemonized) {
+ int fd;
+
+ strbuf_addstr(&log_filename, git_path("gc.log_XX"));
+ fd = xmkstemp(log_filename.buf);
+ if (fd >= 0) {
+ dup2(fd, 2);
+ close(fd);
+ } else
+ strbuf_release(&log_filename);
+ }
+
if (gc_before_repack())
return -1;
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] diff.h: extend "flags" field to 64 bits because we're out of bits
I renamed both "flags" and "touched_flags" fields while making this
patch to make sure I was aware of how these flags were manipulated
(besides DIFF_OPT* macros). So hopefully I didn't miss anything.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Resend to the right recipients. I screwed send-email up..
builtin/commit.c | 2 +-
diff-lib.c | 4 ++--
diff.c | 2 +-
diff.h | 8 +---
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 4cbd5ff..95f7296 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -895,7 +895,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
* submodules which were manually staged, which would
* be really confusing.
*/
- int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+ diff_flags_t diff_flags =
DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
if (ignore_submodule_arg &&
!strcmp(ignore_submodule_arg, "all"))
diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
diff --git a/diff-lib.c b/diff-lib.c
index 241a843..ae09034 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options
*diffopt,
{
int changed = ce_match_stat(ce, st, ce_option);
if (S_ISGITLINK(ce->ce_mode)) {
- unsigned orig_flags = diffopt->flags;
+ diff_flags_t orig_flags = diffopt->flags;
if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
set_diffopt_flags_from_submodule_config(diffopt,
ce->name);
if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))
@@ -516,7 +516,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct
diff_options *opt)
return 0;
}
-int index_differs_from(const char *def, int diff_flags)
+int index_differs_from(const char *def, diff_flags_t diff_flags)
{
struct rev_info rev;
struct setup_revision_opt opt;
diff --git a/diff.c b/diff.c
index 7deac90..2485870 100644
--- a/diff.c
+++ b/diff.c
@@ -4912,7 +4912,7 @@ int diff_can_quit_early(struct diff_options *opt)
static int is_submodule_ignored(const char *path, struct diff_options *options)
{
int ignored = 0;
- unsigned orig_flags = options->flags;
+ diff_flags_t orig_flags = options->flags;
if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
set_diffopt_flags_from_submodule_config(options, path);
if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))
diff --git a/diff.h b/diff.h
index f7208ad..4241aa5 100644
--- a/diff.h
+++ b/diff.h
@@ -110,13 +110,15 @@ enum diff_words_type {
DIFF_WORDS_COLOR
};
+typedef uint64_t diff_flags_t;
+
struct diff_options {
const char *orderfile;
const char *pickaxe;
const char *single_follow;
const char *a_prefix, *b_prefix;
- unsigned flags;
- unsigned touched_flags;
+ diff_flags_t flags;
+ diff_flags_t touched_flags;
/* diff-filter bits */
unsigned int filter;
@@ -347,7 +349,7 @@ extern int diff_result_code(struct diff_options *, int);
extern void diff_no_index(struct rev_info *, int, const char **, const char *);
-extern int index_differs_from(const char *def, int diff_flags);
+extern int index_differs_from(const char *def, diff_flags_t diff_flags);
extern size_t fill_textconv(struct userdiff_driver *driver,
struct diff_filespec *df,
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
The original commit d95d728aba06a34394d15466045cbdabdada58a2 was
reverted in commit 78cc1a540ba127b13f2f3fd531777b57f3a9cd46 because we
were (and still are) not ready for a new world order. A lot more
investigation must be done to see what is impacted. See the 78cc1a5 for
details.
This patch takes a smaller and safer step. The new behavior is
controlled by SHIFT_INTENT_TO_ADD flag. We can gradually move more diff
users to the new behavior after we are sure it's safe to do so. This
flag is exposed to outside as "--shift-ita".
"git status" sets this flag unconditionally to display i-t-a entries
correctly. See d95d728 for details about "git status" and i-t-a entries.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/diff-options.txt | 6 ++
diff-lib.c | 14 ++
diff.c | 2 ++
diff.h | 1 +
t/t2203-add-intent.sh | 20 ++--
wt-status.c| 2 ++
6 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index d56ca90..b69db29 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -556,5 +556,11 @@ endif::git-format-patch[]
--no-prefix::
Do not show any source or destination prefix.
+--shift-ita::
+ By default entries added by "git add -N" appear as an existing
+ empty file in "git diff" and a new file in "git diff --cached".
+ This option makes the entry appear as a new file in "git diff"
+ and non-existent in "git diff --cached".
+
For more detailed explanation on these common options, see also
linkgit:gitdiffcore[7].
diff --git a/diff-lib.c b/diff-lib.c
index ae09034..2546f5a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -212,6 +212,12 @@ int run_diff_files(struct rev_info *revs, unsigned int
option)
ce->sha1,
!is_null_sha1(ce->sha1),
ce->name, 0);
continue;
+ } else if (DIFF_OPT_TST(&revs->diffopt,
SHIFT_INTENT_TO_ADD) &&
+ ce_intent_to_add(ce)) {
+ diff_addremove(&revs->diffopt, '+', ce->ce_mode,
+ EMPTY_BLOB_SHA1_BIN, 0,
+ ce->name, 0);
+ continue;
}
changed = match_stat_with_submodule(&revs->diffopt, ce,
&st,
@@ -376,6 +382,14 @@ static void do_oneway_diff(struct unpack_trees_options *o,
struct rev_info *revs = o->unpack_data;
int match_missing, cached;
+ /* i-t-a entries do not actually exist in the index */
+ if (DIFF_OPT_TST(&revs->diffopt, SHIFT_INTENT_TO_ADD) &&
+ idx && ce_intent_to_add(idx)) {
+ idx = NULL;
+ if (!tree)
+ return; /* nothing to diff.. */
+ }
+
/* if the entry is not checked out, don't examine work tree */
cached = o->index_only ||
(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));
diff --git a/diff.c b/diff.c
index 2485870..e554cad 100644
--- a/diff.c
+++ b/diff.c
@@ -3890,6 +3890,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
return parse_ws_error_highlight(options, arg);
+ else if (!strcmp(arg, "--shift-ita"))
+ DIFF_OPT_SET(options, SHIFT_INTENT_TO_ADD);
/* misc options */
else if (!strcmp(arg, "-z"))
diff --git a/diff.h b/diff.h
index 4241aa5..68a4400 100644
--- a/diff.h
+++ b/diff.h
@@ -92,6 +92,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct
diff_options *opt, void *data)
#define DIFF_OPT_FUNCCONTEXT (1 << 29)
#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
+#define DIFF_OPT_SHIFT_INTENT_TO_ADD (1ULL << 32)
#define DIFF_OPT_TST(opts, flag)((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_TOUCHED(opts, flag)((opts)->touched_flags &
DIFF_OPT_##flag)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index d0f36a4..7c90eee 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -5,10 +5,24 @@ test_description='Intent to add'
. ./test-lib.sh
test_expect_success 'intent to add' '
+ test_commit 1 &&
+ git rm 1.t &&
+ echo hello >1.t &&
echo hello >file &&
echo hello >elif &&
git add -N file &&
- git add elif
+ git add elif &&
+ git add -N 1.t
+'
+
+test_expect_success 'git status' '
+ git status --porcelain | grep -v actual >actual &&
+ cat >expect <<-\EOF &&
+ DA 1.t
+ A elif
+A file
+ EOF
+
[PATCH 7/8] diff.h: extend "flags" field to 64 bits because we're out of bits
I renamed both "flags" and "touched_flags" fields while making this
patch to make sure I was aware of how these flags were manipulated
(besides DIFF_OPT* macros). So hopefully I didn't miss anything.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 2 +-
diff-lib.c | 4 ++--
diff.c | 2 +-
diff.h | 8 +---
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 4cbd5ff..95f7296 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -895,7 +895,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
* submodules which were manually staged, which would
* be really confusing.
*/
- int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
+ diff_flags_t diff_flags =
DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG;
if (ignore_submodule_arg &&
!strcmp(ignore_submodule_arg, "all"))
diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
diff --git a/diff-lib.c b/diff-lib.c
index 241a843..ae09034 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -71,7 +71,7 @@ static int match_stat_with_submodule(struct diff_options
*diffopt,
{
int changed = ce_match_stat(ce, st, ce_option);
if (S_ISGITLINK(ce->ce_mode)) {
- unsigned orig_flags = diffopt->flags;
+ diff_flags_t orig_flags = diffopt->flags;
if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
set_diffopt_flags_from_submodule_config(diffopt,
ce->name);
if (DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES))
@@ -516,7 +516,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct
diff_options *opt)
return 0;
}
-int index_differs_from(const char *def, int diff_flags)
+int index_differs_from(const char *def, diff_flags_t diff_flags)
{
struct rev_info rev;
struct setup_revision_opt opt;
diff --git a/diff.c b/diff.c
index 7deac90..2485870 100644
--- a/diff.c
+++ b/diff.c
@@ -4912,7 +4912,7 @@ int diff_can_quit_early(struct diff_options *opt)
static int is_submodule_ignored(const char *path, struct diff_options *options)
{
int ignored = 0;
- unsigned orig_flags = options->flags;
+ diff_flags_t orig_flags = options->flags;
if (!DIFF_OPT_TST(options, OVERRIDE_SUBMODULE_CONFIG))
set_diffopt_flags_from_submodule_config(options, path);
if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))
diff --git a/diff.h b/diff.h
index f7208ad..4241aa5 100644
--- a/diff.h
+++ b/diff.h
@@ -110,13 +110,15 @@ enum diff_words_type {
DIFF_WORDS_COLOR
};
+typedef uint64_t diff_flags_t;
+
struct diff_options {
const char *orderfile;
const char *pickaxe;
const char *single_follow;
const char *a_prefix, *b_prefix;
- unsigned flags;
- unsigned touched_flags;
+ diff_flags_t flags;
+ diff_flags_t touched_flags;
/* diff-filter bits */
unsigned int filter;
@@ -347,7 +349,7 @@ extern int diff_result_code(struct diff_options *, int);
extern void diff_no_index(struct rev_info *, int, const char **, const char *);
-extern int index_differs_from(const char *def, int diff_flags);
+extern int index_differs_from(const char *def, diff_flags_t diff_flags);
extern size_t fill_textconv(struct userdiff_driver *driver,
struct diff_filespec *df,
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] grep: make it clear i-t-a entries are ignored
The expression "!S_ISREG(ce)" covers i-t-a entries as well because
ce->ce_mode would be zero then. I could make a comment saying that, but
it's probably better just to comment with code, in case i-t-a entry
content changes in future.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..f508179 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -375,7 +375,7 @@ static int grep_cache(struct grep_opt *opt, const struct
pathspec *pathspec, int
for (nr = 0; nr < active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
- if (!S_ISREG(ce->ce_mode))
+ if (!S_ISREG(ce->ce_mode) || ce_intent_to_add(ce))
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] checkout(-index): do not checkout i-t-a entries
The cached blob of i-t-a entries are empty blob. By checkout, we delete
the content we have. Don't do it.
This is done higher up instead of inside checkout_entry() because we
would have limited options in there: silently ignore, loudly ignore,
die. At higher level we can do better reporting. For example, "git
checkout -- foo" will complain that "foo" does not match pathspec, just
like when "foo" is not registered with "git add -N"
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout-index.c | 5 -
builtin/checkout.c | 2 ++
t/t2203-add-intent.sh| 34 ++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 8028c37..eca975d 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -56,7 +56,8 @@ static int checkout_file(const char *name, const char *prefix)
while (pos < active_nr) {
struct cache_entry *ce = active_cache[pos];
if (ce_namelen(ce) != namelen ||
- memcmp(ce->name, name, namelen))
+ memcmp(ce->name, name, namelen) ||
+ ce_intent_to_add(ce))
break;
has_same_name = 1;
pos++;
@@ -99,6 +100,8 @@ static void checkout_all(const char *prefix, int
prefix_length)
if (ce_stage(ce) != checkout_stage
&& (CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))
continue;
+ if (ce_intent_to_add(ce))
+ continue;
if (prefix && *prefix &&
(ce_namelen(ce) <= prefix_length ||
memcmp(prefix, ce->name, prefix_length)))
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e1403be..02889d4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -300,6 +300,8 @@ static int checkout_paths(const struct checkout_opts *opts,
* anything to this entry at all.
*/
continue;
+ if (ce_intent_to_add(ce))
+ continue;
/*
* Either this entry came from the tree-ish we are
* checking the paths out of, or we are checking out
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 96c8755..d0f36a4 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -111,5 +111,39 @@ test_expect_success 'apply:check_preimage() not creating
empty file' '
)
'
+test_expect_success 'checkout ignores i-t-a' '
+ git init checkout &&
+ (
+ cd checkout &&
+ echo data >file &&
+ git add -N file &&
+ test_must_fail git checkout -- file &&
+ echo data >expected &&
+ test_cmp expected file
+ )
+'
+
+test_expect_success 'checkout-index ignores i-t-a' '
+ (
+ cd checkout &&
+ git checkout-index file &&
+ echo data >expected &&
+ test_cmp expected file
+ )
+'
+
+test_expect_success 'checkout-index --all ignores i-t-a' '
+ (
+ cd checkout &&
+ echo data >anotherfile &&
+ git add anotherfile &&
+ rm anotherfile &&
+ git checkout-index --all &&
+ echo data >expected &&
+ test_cmp expected file &&
+ test_cmp expected anotherfile
+ )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] apply: fix adding new files on i-t-a entries
Applying a patch that adds a file when that file is registered with "git
add -N" will fail with message "already exists in index" because
git-apply checks, sees those i-t-a entries and aborts. git-apply does
not realize those are for bookkeeping only, they do not really exist in
the index. This patch tightens the "exists in index" check, ignoring
i-t-a entries.
Reported-by: Patrick Higgins
Reported-by: Bjørnar Snoksrud
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/apply.c | 9 +
t/t2203-add-intent.sh | 13 +
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 54aba4e..76b58a1 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3553,10 +3553,11 @@ static int check_to_create(const char *new_name, int
ok_if_exists)
{
struct stat nst;
- if (check_index &&
- cache_name_pos(new_name, strlen(new_name)) >= 0 &&
- !ok_if_exists)
- return EXISTS_IN_INDEX;
+ if (check_index && !ok_if_exists) {
+ int pos = cache_name_pos(new_name, strlen(new_name));
+ if (pos >= 0 && !ce_intent_to_add(active_cache[pos]))
+ return EXISTS_IN_INDEX;
+ }
if (cached)
return 0;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2a4a749..bb5ef2b 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -82,5 +82,18 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
test_cmp expect actual
'
+test_expect_success 'apply adds new file on i-t-a entry' '
+ git init apply &&
+ (
+ cd apply &&
+ echo newcontent >newfile &&
+ git add newfile &&
+ git diff --cached >patch &&
+ rm .git/index &&
+ git add -N newfile &&
+ git apply --cached patch
+ )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] apply: make sure check_preimage() does not leave empty file on error
The test case probably describes the test scenario the best. We have a
patch to modify some file but the base file is gone. Because
check_preimage() finds an index entry with the same old_name, it tries
to restore the on-disk base file with cached content with
checkout_target() and move on.
If this old_name is i-t-a, before this patch "apply -3" will call
checkout_target() which will write an empty file (i-t-a entries always
have "empty blob" SHA-1), then it'll fail at verify_index_match()
(i-t-a entries are always "modified") and the operation is aborted.
This empty file should not be created. Compare to the case where
old_name does not exist in index, "apply -3" fails with a different
error message "...: does not exist" but it does not touch
worktree. This patch makes sure the same happens to i-t-a entries.
load_current() shares the same code pattern (look up an index entry,
if on-disk version is not found, restore using the cached
version). Fix it too (even though it's not exercised in any test case)
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/apply.c | 4 ++--
t/t2203-add-intent.sh | 16
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 76b58a1..b185c97 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3348,7 +3348,7 @@ static int load_current(struct image *image, struct patch
*patch)
die("BUG: patch to %s is not a creation", patch->old_name);
pos = cache_name_pos(name, strlen(name));
- if (pos < 0)
+ if (pos < 0 || ce_intent_to_add(active_cache[pos]))
return error(_("%s: does not exist in index"), name);
ce = active_cache[pos];
if (lstat(name, &st)) {
@@ -3501,7 +3501,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
if (check_index && !previous) {
int pos = cache_name_pos(old_name, strlen(old_name));
- if (pos < 0) {
+ if (pos < 0 || ce_intent_to_add(active_cache[pos])) {
if (patch->is_new < 0)
goto is_new;
return error(_("%s: does not exist in index"),
old_name);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index bb5ef2b..96c8755 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -95,5 +95,21 @@ test_expect_success 'apply adds new file on i-t-a entry' '
)
'
+test_expect_success 'apply:check_preimage() not creating empty file' '
+ git init check-preimage &&
+ (
+ cd check-preimage &&
+ echo oldcontent >newfile &&
+ git add newfile &&
+ echo newcontent >newfile &&
+ git diff >patch &&
+ rm .git/index &&
+ git add -N newfile &&
+ rm newfile &&
+ test_must_fail git apply -3 patch &&
+ ! test -f newfile
+ )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] blame: remove obsolete comment
That "someday" in the comment happened two years later in b65982b (Optimize "diff-index --cached" using cache-tree - 2009-05-20) Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/blame.c | 5 - 1 file changed, 5 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 4db01c1..942f302 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2379,11 +2379,6 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, ce->ce_mode = create_ce_mode(mode); add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE); - /* -* We are not going to write this out, so this does not matter -* right now, but someday we might optimize diff-index --cached -* with cache-tree information. -*/ cache_tree_invalidate_path(&the_index, path); return commit; -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] Add and use convenient macro ce_intent_to_add()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/rm.c | 2 +-
cache-tree.c | 2 +-
cache.h | 1 +
read-cache.c | 4 ++--
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 80b972f..8829b09 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -211,7 +211,7 @@ static int check_local_mod(unsigned char *head, int
index_only)
* "intent to add" entry.
*/
if (local_changes && staged_changes) {
- if (!index_only || !(ce->ce_flags & CE_INTENT_TO_ADD))
+ if (!index_only || !ce_intent_to_add(ce))
string_list_append(&files_staged, name);
}
else if (!index_only) {
diff --git a/cache-tree.c b/cache-tree.c
index feace8b..c865bd2 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -377,7 +377,7 @@ static int update_one(struct cache_tree *it,
* they are not part of generated trees. Invalidate up
* to root to force cache-tree users to read elsewhere.
*/
- if (ce->ce_flags & CE_INTENT_TO_ADD) {
+ if (ce_intent_to_add(ce)) {
to_invalidate = 1;
continue;
}
diff --git a/cache.h b/cache.h
index 4e25271..41d811c 100644
--- a/cache.h
+++ b/cache.h
@@ -241,6 +241,7 @@ static inline unsigned create_ce_flags(unsigned stage)
#define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)
#define ce_skip_worktree(ce) ((ce)->ce_flags & CE_SKIP_WORKTREE)
#define ce_mark_uptodate(ce) ((ce)->ce_flags |= CE_UPTODATE)
+#define ce_intent_to_add(ce) ((ce)->ce_flags & CE_INTENT_TO_ADD)
#define ce_permissions(mode) (((mode) & 0100) ? 0755 : 0644)
static inline unsigned int create_ce_mode(unsigned int mode)
diff --git a/read-cache.c b/read-cache.c
index 89dbc08..f849cbd 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -327,7 +327,7 @@ int ie_match_stat(const struct index_state *istate,
* by definition never matches what is in the work tree until it
* actually gets added.
*/
- if (ce->ce_flags & CE_INTENT_TO_ADD)
+ if (ce_intent_to_add(ce))
return DATA_CHANGED | TYPE_CHANGED | MODE_CHANGED;
changed = ce_match_stat_basic(ce, st);
@@ -1251,7 +1251,7 @@ int refresh_index(struct index_state *istate, unsigned
int flags,
if (cache_errno == ENOENT)
fmt = deleted_fmt;
- else if (ce->ce_flags & CE_INTENT_TO_ADD)
+ else if (ce_intent_to_add(ce))
fmt = added_fmt; /* must be before other checks
*/
else if (changed & TYPE_CHANGED)
fmt = typechange_fmt;
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"
On Thu, Jun 25, 2015 at 8:07 PM, Junio C Hamano wrote: >>> Perhaps a good and safe way forward to resurrect what d95d728a >>> wanted to do is to first add an option to tell run_diff_index() and >>> run_diff_files() which behaviour the caller wants to see, add that >>> only to the caller in wt-status.c? Then incrementally pass that >>> option from more callsites that we are absolutely certain that want >>> this different worldview with respect to i-t-a? >> >> Agreed. > > OK. Perhaps then first I should do that revert and we'll > incrementally rebuild on top. Here comes the rebuild. This adds --shift-ita option (and an internal flag) to enable this behavior. Those only take the last two patches. The remaining is to make sure we handle i-t-a entries correctly in some commands. Nguyễn Thái Ngọc Duy (8): blame: remove obsolete comment Add and use convenient macro ce_intent_to_add() apply: fix adding new files on i-t-a entries apply: make sure check_preimage() does not leave empty file on error checkout(-index): do not checkout i-t-a entries grep: make it clear i-t-a entries are ignored diff.h: extend "flags" field to 64 bits because we're out of bits Resurrect "diff-lib.c: adjust position of i-t-a entries in diff" Documentation/diff-options.txt | 6 +++ builtin/apply.c| 13 --- builtin/blame.c| 5 --- builtin/checkout-index.c | 5 ++- builtin/checkout.c | 2 + builtin/commit.c | 2 +- builtin/grep.c | 2 +- builtin/rm.c | 2 +- cache-tree.c | 2 +- cache.h| 1 + diff-lib.c | 18 - diff.c | 4 +- diff.h | 9 +++-- read-cache.c | 4 +- t/t2203-add-intent.sh | 83 +- wt-status.c| 2 + 16 files changed, 134 insertions(+), 26 deletions(-) -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] clone: better error when --reference is a linked checkout
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/clone.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 836fb64..7a010bb 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -294,9 +294,14 @@ static int add_one_reference(struct string_list_item
*item, void *cb_data)
char *ref_git_git = mkpathdup("%s/.git", ref_git);
free(ref_git);
ref_git = ref_git_git;
- } else if (!is_directory(mkpath("%s/objects", ref_git)))
+ } else if (!is_directory(mkpath("%s/objects", ref_git))) {
+ struct strbuf sb = STRBUF_INIT;
+ if (get_common_dir(&sb, ref_git))
+ die(_("reference repository '%s' as a linked checkout
is not supported yet."),
+ item->string);
die(_("reference repository '%s' is not a local repository."),
item->string);
+ }
if (!access(mkpath("%s/shallow", ref_git), F_OK))
die(_("reference repository '%s' is shallow"), item->string);
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] fix local clone from a linked checkout
On Wed, Jul 15, 2015 at 8:25 PM, Duy Nguyen wrote: > On Wed, Jul 15, 2015 at 11:40:18AM +0200, Bjørnar Snoksrud wrote: >> I reported this before, but now I have a nice topic to hang it on - >> >> I have re-reproduced the bug using a build from master as of today, >> using the new worktree commands. > > Something like the following patch should work if you need it now. > > Because this may conflict (in the test cases) with Eric's series to > move "git checkout --to" to "git worktree add", and because the next > release is already delayed to let "git worktree add" in, I think we > could keep this patch out of tree for now. I will split it up, add > tests and resubmit once the release is out. Please remind me if you > see nothing from me for too long. Here it is. Mostly the same as the previous patch except that the last patch is new. Nguyễn Thái Ngọc Duy (5): path.c: delete an extra space enter_repo: avoid duplicating logic, use is_git_directory() instead enter_repo: allow .git files in strict mode clone: allow --local from a linked checkout clone: better error when --reference is a linked checkout builtin/clone.c | 13 ++--- path.c | 14 +- t/t2025-worktree-add.sh | 5 + 3 files changed, 24 insertions(+), 8 deletions(-) -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead
Signed-off-by: Nguyễn Thái Ngọc Duy
---
path.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/path.c b/path.c
index a536ee3..7340e11 100644
--- a/path.c
+++ b/path.c
@@ -441,8 +441,7 @@ const char *enter_repo(const char *path, int strict)
else if (chdir(path))
return NULL;
- if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
- validate_headref("HEAD") == 0) {
+ if (is_git_directory(".")) {
set_git_dir(".");
check_repository_format();
return path;
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] enter_repo: allow .git files in strict mode
Strict mode is about not guessing where .git is. If the user points to a
.git file, we know exactly where the target .git dir will be.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
path.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/path.c b/path.c
index 7340e11..32d4ca6 100644
--- a/path.c
+++ b/path.c
@@ -438,8 +438,13 @@ const char *enter_repo(const char *path, int strict)
return NULL;
path = validated_path;
}
- else if (chdir(path))
- return NULL;
+ else {
+ const char *gitfile = read_gitfile(used_path);
+ if (gitfile)
+ path = gitfile;
+ if (chdir(path))
+ return NULL;
+ }
if (is_git_directory(".")) {
set_git_dir(".");
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] clone: allow --local from a linked checkout
Noticed-by: Bjørnar Snoksrud
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/clone.c | 6 --
t/t2025-worktree-add.sh | 5 +
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 578da85..836fb64 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -424,8 +424,10 @@ static void clone_local(const char *src_repo, const char
*dest_repo)
} else {
struct strbuf src = STRBUF_INIT;
struct strbuf dest = STRBUF_INIT;
- strbuf_addf(&src, "%s/objects", src_repo);
- strbuf_addf(&dest, "%s/objects", dest_repo);
+ get_common_dir(&src, src_repo);
+ get_common_dir(&dest, dest_repo);
+ strbuf_addstr(&src, "/objects");
+ strbuf_addf(&dest, "/objects");
copy_or_link_directory(&src, &dest, src_repo, src.len);
strbuf_release(&src);
strbuf_release(&dest);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 8267411..3694174 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -193,4 +193,9 @@ test_expect_success '"add" -B/--detach mutually exclusive' '
test_must_fail git worktree add -B poodle --detach bamboo master
'
+test_expect_success 'local clone from linked checkout' '
+ git clone --local here here-clone &&
+ ( cd here-clone && git fsck )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] path.c: delete an extra space
Signed-off-by: Nguyễn Thái Ngọc Duy --- path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path.c b/path.c index 95acbaf..a536ee3 100644 --- a/path.c +++ b/path.c @@ -431,7 +431,7 @@ const char *enter_repo(const char *path, int strict) } if (!suffix[i]) return NULL; - gitfile = read_gitfile(used_path) ; + gitfile = read_gitfile(used_path); if (gitfile) strcpy(used_path, gitfile); if (chdir(used_path)) -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Where to report security vulnerabilities in git?
On Fri, Aug 21, 2015 at 3:55 PM, Guido Vranken wrote: > germane exploitation details. I did find an older thread in the > archive addressing this question ( > http://thread.gmane.org/gmane.comp.version-control.git/260328/ ), but > because I'm unsure if those e-mail addresses are still relevant, I'm > asking again. Indeed that was an old advice. Recent releases of "A note from the maintainer" has this paragraph: If you think you found a security-sensitive issue and want to disclose it to us without announcing it to wider public, please contact us at our security mailing list . -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Submodule, subtree, or something else?
On Fri, Aug 21, 2015 at 3:47 PM, Jānis Rukšāns wrote: > Hello, > > > First of all, I apologise for the wall of text that follows; obviously I > am bad at this. > > My $DAYJOB is switching from Subversion to Git, primarily because of > it's distributed nature (we are scattered all across the globe), and the > ease of branching and merging. One issue that has popped up is how to > manage code shared between multiple projects. > > Our SVN setup used a shared repository for all projects, either using > externals for shared code, or, more often than not, simply merging the > code between projects as needed. Ignoring the fact that merging with > SVN is somewhat cumbersome, overall it has worked quite well for us, > especially when combined with git-svn. > > For external libraries that rarely change, submodules appear to be the > obvious choice when using Git. On the other hand, I've found them > somewhat cumbersome to use, and subtree merging (either using git > subtree, or directly with git merge -s subtree) is closer to what we > were doing in SVN. A major drawback of submodules in my opinion is the > inability to make a full clone from an existing one without having > access to the central repository, which is something I have to do from > time to time. Can you elaborate on that a bit more? git clone --recurse-submodules should do that no matter which remote you contact? > > For internal libraries, the situation is even less clear. For many of > these libraries, most of the development happens within the context of a > single project, with commits to main project being interleaved with > commits to the subproject(s), resulting in histories resembling: > > (using git submodule) > >A---B---S1---S2---C---S3 > ,´ ,´ ,´ > N---OPQ---R > > (using git subtree with --rejoin) > >A---B---N---O---M1---M2---Q---C---R---M3 > /// > N'--O'---PQ'--R' > > (using merge -s subtree) > >A---B---M1---M2---C---M3 > /// > N---OPQ---R > > where A, B and C are changes to the main project, N, O, P, Q and R are > changes to library code, and Sn and Mn are submodule updates and merge > commits, respectively. > > From what I have gathered, submodules have issues with branching and > merging, therefore, unless I'm mistaken, submodules are kinda out of > question. Of the remaining two options, merging directly results in a > nicer history, but requires making all changes to the library repo first > (although I am quite sure that a similar effect can be achieved with > plumbing, similarly to how git subtree split works), and is harder to > use than git subtree. Also, all three options can result in the main > project history being cluttered with extra commits. > > Lastly, there is a particularly painful 3rd party library that has an > enormous amount of local modifications that are never going to make it > upstream, essentially making it a fork, project specific changes that > are required for one project, but would break others, separate language > bindings that access the internals (often requiring bug fixes to be made > simultaneously to both), and, if that wasn't enough, it *requires* > several source files to be modified for each individual project that > uses it. It's a complete mess, but we're stuck with it for the existing > projects, as switching to an alternative would be too time consuming. > > > To sum up, I'm looking for something that would let us share code > between multiple projects, allow for: > > 1) separate histories with relatively easy branching and merging > > 2) distributed workflow without having to set up a multiple repositories > everywhere (eg. work <-> home <-> laptop) > > 3) to work on the shared code within a project using it > > 4) inspection of the complete history > > 5) modifications that are not shared with other projects > > and would not result in lots of clutter in the history. > > Repository size is somewhat less of an issue, because each submodule has > to be checked out anyway. > > Submodules let you have #3, and #1, #2 and #5 to a point, after which it > becomes a pain. git subtree allows #1, #2, #3 and #4, and #5 with some > pain (?), but results in duplicate commits. Using subtree merge > strategy directly gives everything except #3, but is harder to use than > submodules or subtree. > > Are there any other options beside these three for sharing (or in some > cases, not sharing) common code between projects using Git, that would > address the above points better? Or, alternatively, ways to work around > the drawbacks of the existing tools? > > Lastly, I will be grateful for any suggestions about how to handle the > messy case described above better. > > Thanks, > Jānis > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org
Re: Where to report security vulnerabilities in git?
The addresses are still valid. (I think there was a plan to introduce a git-security@... but I am not sure if that happened.) > Current practice is to contact Junio C Hamano pobox.com>. > Cc-ing Jeff King peff.net> isn't a bad idea while at it. Just go for that. On Fri, Aug 21, 2015 at 3:55 PM, Guido Vranken wrote: > List, > > I would like to report security vulnerabilities in git. Due to the > sensitive nature of security-impacting bugs I would like to know if > there's a dedicated e-mail address for this, so that the issues at > play can be patched prior to a coordinated public disclosure of the > germane exploitation details. I did find an older thread in the > archive addressing this question ( > http://thread.gmane.org/gmane.comp.version-control.git/260328/ ), but > because I'm unsure if those e-mail addresses are still relevant, I'm > asking again. > > Thanks. > > Guido > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:44 PM, Jeff King wrote:
> On Fri, Aug 21, 2015 at 12:05:13PM -0700, Junio C Hamano wrote:
>
>> The primary reason I suspect is because you sent to a wrong set of
>> people. Submodule folks have largely been working in the scripted
>> ones, and may not necessarily be the ones who are most familiar with
>> the run-command infrastructure.
>>
>> "shortlog --no-merges" tells me that the obvious suspects are j6t
>> and peff.
>
> No good deed goes unpunished. ;)
>
> Before even looking at the implementation, my first question would be
> whether this pattern is applicable in several places in git (i.e., is it
> worth the extra complexity of abstracting out in the first place). I
> think there are a few task-queue patterns already in git; for example
> the delta search in pack-objects. Is the interface given here sufficient
> to convert pack-objects? Is the result nicer to read? Is it as
> efficient?
I have converted index-pack threading now, and it looks quite easy:
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3f10840..159ee36 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -11,6 +11,7 @@
#include "exec_cmd.h"
#include "streaming.h"
#include "thread-utils.h"
+#include "run-command.h"
static const char index_pack_usage[] =
"git index-pack [-v] [-o ] [--keep | --keep=]
[--verify] [--strict] ( | --stdin [--fix-thin]
[])";
@@ -1075,7 +1076,7 @@ static void resolve_base(struct object_entry *obj)
}
#ifndef NO_PTHREADS
-static void *threaded_second_pass(void *data)
+static int threaded_second_pass(struct task_queue *tq, void *data)
{
set_thread_data(data);
for (;;) {
@@ -1096,7 +1097,7 @@ static void *threaded_second_pass(void *data)
resolve_base(&objects[i]);
}
- return NULL;
+ return 0;
}
#endif
@@ -1195,18 +1196,18 @@ static void resolve_deltas(void)
nr_ref_deltas + nr_ofs_deltas);
#ifndef NO_PTHREADS
- nr_dispatched = 0;
if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
+ nr_dispatched = 0;
init_thread();
- for (i = 0; i < nr_threads; i++) {
- int ret = pthread_create(&thread_data[i].thread, NULL,
-threaded_second_pass,
thread_data + i);
- if (ret)
- die(_("unable to create thread: %s"),
- strerror(ret));
- }
+
+ tq = create_task_queue(nr_threads);
+
for (i = 0; i < nr_threads; i++)
- pthread_join(thread_data[i].thread, NULL);
+ add_task(tq, threaded_second_pass, thread_data + i);
+
+ if (finish_task_queue(tq))
+ die("Not all threads have finished");
+
cleanup_thread();
return;
}
---
(tests pass)
This was cheating as I picked to convert index-pack as opposed to upload-pack
(index-pack is very similar to a queued workload. This was just moving
the thread
creation into the new proposed queue processor.)
I realize now this can be adapted a bit more, to show off the queue features
but would require a larger rewrite. So instead of just creating the threads and
then locking, we get rid of the worker lock like this:
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3f10840..797efea 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -11,6 +11,7 @@
#include "exec_cmd.h"
#include "streaming.h"
#include "thread-utils.h"
+#include "run-command.h"
static const char index_pack_usage[] =
"git index-pack [-v] [-o ] [--keep | --keep=]
[--verify] [--strict] ( | --stdin [--fix-thin]
[])";
@@ -106,10 +107,6 @@ static pthread_mutex_t counter_mutex;
#define counter_lock() lock_mutex(&counter_mutex)
#define counter_unlock() unlock_mutex(&counter_mutex)
-static pthread_mutex_t work_mutex;
-#define work_lock() lock_mutex(&work_mutex)
-#define work_unlock() unlock_mutex(&work_mutex)
-
static pthread_mutex_t deepest_delta_mutex;
#define deepest_delta_lock() lock_mutex(&deepest_delta_mutex)
#define deepest_delta_unlock() unlock_mutex(&deepest_delta_mutex)
@@ -140,7 +137,6 @@ static void init_thread(void)
int i;
init_recursive_mutex(&read_mutex);
pthread_mutex_init(&counter_mutex, NULL);
- pthread_mutex_init(&work_mutex, NULL);
pthread_mutex_init(&type_cas_mutex, NULL);
if (show_stat)
pthread_mutex_init(&deepest_delta_mutex, NULL);
@@ -163,7 +159,6 @@ static void cleanup_thread(void)
threads_active = 0;
pthread_mutex_destroy(&read_mutex);
pthread_mutex_destroy(&counter_mutex);
- pthread_mutex_destroy(&work_mutex);
pthread_mutex_destroy(&type_cas_mutex);
if (show_stat)
pthread_mutex_destroy(&deepest_delta_mutex);
@@ -181,9 +176,6 @@ static void cleanup_thread(void)
#define counter_lock()
#define counter_unlock()
-#define work_lock()
-#
Where to report security vulnerabilities in git?
List, I would like to report security vulnerabilities in git. Due to the sensitive nature of security-impacting bugs I would like to know if there's a dedicated e-mail address for this, so that the issues at play can be patched prior to a coordinated public disclosure of the germane exploitation details. I did find an older thread in the archive addressing this question ( http://thread.gmane.org/gmane.comp.version-control.git/260328/ ), but because I'm unsure if those e-mail addresses are still relevant, I'm asking again. Thanks. Guido -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Submodule, subtree, or something else?
Hello, First of all, I apologise for the wall of text that follows; obviously I am bad at this. My $DAYJOB is switching from Subversion to Git, primarily because of it's distributed nature (we are scattered all across the globe), and the ease of branching and merging. One issue that has popped up is how to manage code shared between multiple projects. Our SVN setup used a shared repository for all projects, either using externals for shared code, or, more often than not, simply merging the code between projects as needed. Ignoring the fact that merging with SVN is somewhat cumbersome, overall it has worked quite well for us, especially when combined with git-svn. For external libraries that rarely change, submodules appear to be the obvious choice when using Git. On the other hand, I've found them somewhat cumbersome to use, and subtree merging (either using git subtree, or directly with git merge -s subtree) is closer to what we were doing in SVN. A major drawback of submodules in my opinion is the inability to make a full clone from an existing one without having access to the central repository, which is something I have to do from time to time. For internal libraries, the situation is even less clear. For many of these libraries, most of the development happens within the context of a single project, with commits to main project being interleaved with commits to the subproject(s), resulting in histories resembling: (using git submodule) A---B---S1---S2---C---S3 ,´ ,´ ,´ N---OPQ---R (using git subtree with --rejoin) A---B---N---O---M1---M2---Q---C---R---M3 /// N'--O'---PQ'--R' (using merge -s subtree) A---B---M1---M2---C---M3 /// N---OPQ---R where A, B and C are changes to the main project, N, O, P, Q and R are changes to library code, and Sn and Mn are submodule updates and merge commits, respectively. >From what I have gathered, submodules have issues with branching and merging, therefore, unless I'm mistaken, submodules are kinda out of question. Of the remaining two options, merging directly results in a nicer history, but requires making all changes to the library repo first (although I am quite sure that a similar effect can be achieved with plumbing, similarly to how git subtree split works), and is harder to use than git subtree. Also, all three options can result in the main project history being cluttered with extra commits. Lastly, there is a particularly painful 3rd party library that has an enormous amount of local modifications that are never going to make it upstream, essentially making it a fork, project specific changes that are required for one project, but would break others, separate language bindings that access the internals (often requiring bug fixes to be made simultaneously to both), and, if that wasn't enough, it *requires* several source files to be modified for each individual project that uses it. It's a complete mess, but we're stuck with it for the existing projects, as switching to an alternative would be too time consuming. To sum up, I'm looking for something that would let us share code between multiple projects, allow for: 1) separate histories with relatively easy branching and merging 2) distributed workflow without having to set up a multiple repositories everywhere (eg. work <-> home <-> laptop) 3) to work on the shared code within a project using it 4) inspection of the complete history 5) modifications that are not shared with other projects and would not result in lots of clutter in the history. Repository size is somewhat less of an issue, because each submodule has to be checked out anyway. Submodules let you have #3, and #1, #2 and #5 to a point, after which it becomes a pain. git subtree allows #1, #2, #3 and #4, and #5 with some pain (?), but results in duplicate commits. Using subtree merge strategy directly gives everything except #3, but is harder to use than submodules or subtree. Are there any other options beside these three for sharing (or in some cases, not sharing) common code between projects using Git, that would address the above points better? Or, alternatively, ways to work around the drawbacks of the existing tools? Lastly, I will be grateful for any suggestions about how to handle the messy case described above better. Thanks, Jānis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Which branch(es) contain certain commits? (was Re: (unknown))
Ivan Chernyavsky writes: > But now I had a look on the source and I can see that builtin/branch.c > builds the list of references and prints them in a single place > (print_ref_list()) so I will have to split that function into two in > order to reuse existing functionality. > > Another problem is that builtin/branch.c currently does not use > setup_revisions(), so I'll have to hook it there as well. > > Then, I assume, I'll need to use the initial ref_list (filled the same > as for the current "list" case) to configure the rev_info structure > after setup_revisions(), and start revision traversal. > > I'm not sure I've got it all right from the source in those few days, > so I apologize in advance if it's stupid in some part or as a whole. Heh, you say "problem" above, but I do not think it is a "problem" per-se. If you want to teach branch some preprocessing based on the revision traversal API, you naturally need to call setup_revisions(). The outlined steps above all feel sensible; one thing you did not mention is that you may probably have to clear object flags after you are done with the initial "--grep" revision traversal, as some features of branch may want to use the object flags (e.g. --merged would use in_merge_bases()). Other than that, all of it sounds easily implementable. Note that "branch --list", "tag --list" and "for-each-ref" are being revamped to share more internal code. If you want to pursue this, you probably would want to build on top of that effort once it is done. That way, you may get "tag --grep=FIX123" for free. > That said, do you think the goal is worth such changes? That is a dangerous question. As Duy already said, > Probably because nobody is interested and steps up to do it. The lack > of response to you mail is a sign. apparently not many people thought it is worth; otherwise we would already have such a feature. If you are asking me personally, I'm sorry but I have to say no. The reason why I personally do not think your "branch --grep=FIX123" would be very useful to me is because I would imagine that I would be interested in learning the exact commit that mentions FIX123 as well as which branches contain it, if I had a need for such a feature. That is, it would inherently be two step process for me anyway, i.e. (1) I'd run "log -p --grep" to find which commits are about FIX123 and check that what they did indeed make sense; and (2) I'd then run "branch --contains" to learn which ones are already up to date with respect to the fix. Your "branch --grep=FIX123" that only tells me the names of branches would have no use in that workflow, as it would not even give me an indication that the request --grep=FIX123 found the right commit in the first place. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Minor bug with help.autocorrect.
Jeff King writes: > I think the plan is: > > 1. squelch the warning message from the config code; even if we change > the config format to pager.*.command, we will have to support > pager.* for historical reasons. > > 2. introduce pager.*.command so that "git foo_bar" can use > pager.foo_bar.command. > > We should do (1) in the near-term. We do not have to do (2) at all (and > people with funny command names are simply out of luck), but it would be > nice in the long run. That sounds sensible. > The patch from Tanay in $gmane/263888 accomplishes (1), but there was a > minor cleanup needed (checking the individual bit in "flags", rather > than the whole variable). Here it is with that fix: Thanks; let's take a look. I have a suspicion that it "accomplishes" a lot more than (1) and may be discarding useful errors. > diff --git a/config.c b/config.c > index 9fd275f..dd0cb52 100644 > --- a/config.c > +++ b/config.c > @@ -1314,7 +1314,7 @@ static struct config_set_element > *configset_find_element(struct config_set *cs, >* `key` may come from the user, so normalize it before using it >* for querying entries from the hashmap. >*/ > - ret = git_config_parse_key(key, &normalized_key, NULL); > + ret = git_config_parse_key(key, &normalized_key, NULL, > CONFIG_ERROR_QUIET); Hmm, I am not sure if this is correct, or it is trying to do things at too low a level. configset_add_value() calls configset_find_element(). A NULL return from find_element() could be because parse_key() errored out due to bad name, or the key genuinely did not exist in the hashmap, and the caller cannot tell. So add_value() can end up adding a new pair under a bogus key, which is not a new problem, but what makes me cautious is that it happens silently with the updated code. In fact, git_configset_add_file() uses git_config_from_file() with configset_add_value() as its callback function, and the error that is squelched with this CONFIG_ERROR_QUIET would be the only thing that tells the user that the config file being read is malformed. Right now, "git config" does not seem to use the full configset API so nobody would notice, but still... I wonder if alias_lookup() and check_pager_config(), two functions that *know* that the string they have, cmd, could be invalid and unusable key to give to the config API, should be doing an extra effort (e.g. call parse_key() with QUIET option and refrain from calling git_config_get_value()). It feels that for existing callers of parse_key(), not passing QUIET would be the right thing to do. Of course, I am OK if git_config_get_value() and friends took the QUIET flag and and passed it all the way down to parse_key(); that would be a much more correct approach to address this issue (these two callers do not have to effectively call parse_key() twice that way), but at the same time, that would be a lot more involved change. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] contrib: teach completion about git-worktree options and arguments
On Fri, Aug 21, 2015 at 4:49 PM, SZEDER Gábor wrote:
> Quoting Junio C Hamano :
>> Eric Sunshine writes:
>>> On Thu, Jul 23, 2015 at 4:49 PM, Eric Sunshine
>>> wrote:
Complete subcommands 'add' and 'prune', as well as their respective
options --force, --detach, --dry-run, --verbose, and --expire. Also
complete 'refname' in "git worktree add [-b ]
".
>>>
>>> Ping[1]?
>>
>> Ping indeed?
>
> Yeah, right, sorry. Non-subscribed occasional gmane-reader here amidst
> job hunting, so thanks for the ping. And the re-ping...
Thanks for the review.
+_git_worktree ()
+{
+ local subcommands='add prune'
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local c=2 n=0 refpos=2
>
> A more descriptive variable name for 'n' would be great.
Indeed. I was planning on resubmitting with better variable names
(even if I didn't get any feedback)...
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+ else
+ case "$subcommand,$cur" in
+ add,--*)
+ __gitcomp "--force --detach"
>
> We usually don't offer '--force', because that option must be
> handled with care.
As a person who never uses git-completion (or git-prompt), I wasn't
aware of that, so thanks for the heads-up. I only installed completion
(and prompt) when I decided to work on this (and I don't think I've
used tab-completion since then).
+ ;;
+ add,*)
+ while [ $c -lt $cword ]; do
+ case "${words[c]}" in
+ --*) ;;
+ -[bB]) ((refpos++)) ;;
+ *) ((n++)) ;;
+ esac
+ ((c++))
+ done
+ if [ $n -eq $refpos ]; then
>
> I suppose here you wanted to calculate where (i.e. at which word on
> the command line) we should offer refs and fall back to bash builtin
> filename completion otherwise. It works well in the common cases,
> but:
>
> - it doesn't offer refs after -b or -B
That was intentional since this is a new branch name, so it didn't
seem sensible to offer completion of existing refs. On the other hand,
it doesn't make much since to offer pathname completion either, but I
didn't see a better alternative. On reflection, I suppose ref
completion can make sense if the new branch name is going to be
similar to an existing one.
> - it gets fooled by options to the git command, e.g. 'git
> --git-dir=.git worktree add ' offers refs instead of files,
> 'git --git-dir=.git worktree add ../some/path ' offers
> refs, etc.
This shortcoming could be addressed by computing relative to the
subcommand-index as your version does, correct?
I don't have enough background with the (git-specific) completion
facility to be able to judge if this patch and approach has merit and
ought to be pursued further, or if it would be better to drop in favor
of (some version of) your patch. I don't care strongly, and am fine
with dropping this patch if it's approach is suboptimal.
+ __gitcomp_nl "$(__git_refs)"
+ fi
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run --verbose --expire"
+ ;;
+ esac
+ fi
+}
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3z/3] log: make '-p' imply '-m --cc'
Junio C Hamano writes:
> The latter was inspired by a recent discussion, most notably
>
>http://thread.gmane.org/gmane.comp.version-control.git/273937/focus=273988
>
> but implements it with a much less UI impact. Tweaking "git log -p"
> has a lot of fallout---interested parties can try it out and how two
> tests in t4xxx series break.
Just for the record, here is what I did before deciding that the
idea of turning '-p' into '--cc -m' is not good and instead did the
"'--cc' should show the patch without '-p' and '-m'" that is the
real [3/3]. The attached patch replaces 3/3, showss what happens if
you tweak 'log -p' to imply '-m --cc'"; the change of behaviours is
illustrated by the updated tests.
Many are good-looking, and some others are questionable.
* "log -G -p" and "log -S -p" that silently turns
"-p" into "-m --cc" shows merges that changed the occurrences of
, but if the merge is a trivial textual merge, the change
itself is not seen (as we are doing "--cc"). This looks very
questionable.
* Similarly, "log -p --stat" shows the diffstat against the first
parent for a merge but does not show the patch text, as "--cc" is
in effect, for a trivial textual merge. This looks strange.
* Even though I very much do like the fact that we see _something_
in the output for a merge commit, "log --patch-with-stat" that
outputs a "patch" that cannot be grokked by "git apply" feels
wrong.
All of the above led me to say "when the user explicitly asks --cc,
show the merges that way, otherwise keeping the 'merges are by
default ignored' is the right thing".
diff --git a/builtin/log.c b/builtin/log.c
index 519f170..1a77804 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -633,6 +633,14 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
rev->prune_data.nr == 1)
DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
+ /* Turn -p without -m to --cc -p -m */
+ if (rev->ignore_merges && !rev->combine_merges &&
+ (rev->diffopt.output_format & DIFF_FORMAT_PATCH)) {
+ rev->ignore_merges = 0;
+ rev->combine_merges = 1;
+ rev->dense_combined_merges = 1;
+ }
+
/* Turn --cc/-c into -p --cc/-c when -p was not given */
if (!rev->diffopt.output_format && rev->combine_merges)
rev->diffopt.output_format = DIFF_FORMAT_PATCH;
diff --git a/t/t4013/diff.log_--patch-with-stat_--summary_master_--_dir_
b/t/t4013/diff.log_--patch-with-stat_--summary_master_--_dir_
index a18f147..04f4eee 100644
--- a/t/t4013/diff.log_--patch-with-stat_--summary_master_--_dir_
+++ b/t/t4013/diff.log_--patch-with-stat_--summary_master_--_dir_
@@ -6,6 +6,23 @@ Date: Mon Jun 26 00:04:00 2006 +
Merge branch 'side'
+ dir/sub | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --cc dir/sub
+index cead32e,7289e35..992913c
+--- a/dir/sub
b/dir/sub
+@@@ -1,6 -1,4 +1,8 @@@
+ A
+ B
+ +C
+ +D
+ +E
+ +F
++ 1
++ 2
+
commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
Author: A U Thor
Date: Mon Jun 26 00:03:00 2006 +
diff --git a/t/t4013/diff.log_--patch-with-stat_master
b/t/t4013/diff.log_--patch-with-stat_master
index ae425c4..ec4b1b9 100644
--- a/t/t4013/diff.log_--patch-with-stat_master
+++ b/t/t4013/diff.log_--patch-with-stat_master
@@ -6,6 +6,38 @@ Date: Mon Jun 26 00:04:00 2006 +
Merge branch 'side'
+ dir/sub | 2 ++
+ file0 | 3 +++
+ 2 files changed, 5 insertions(+)
+
+diff --cc dir/sub
+index cead32e,7289e35..992913c
+--- a/dir/sub
b/dir/sub
+@@@ -1,6 -1,4 +1,8 @@@
+ A
+ B
+ +C
+ +D
+ +E
+ +F
++ 1
++ 2
+diff --cc file0
+index b414108,f4615da..10a8a9f
+--- a/file0
b/file0
+@@@ -1,6 -1,6 +1,9 @@@
+ 1
+ 2
+ 3
+ +4
+ +5
+ +6
++ A
++ B
++ C
+
commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
Author: A U Thor
Date: Mon Jun 26 00:03:00 2006 +
diff --git a/t/t4013/diff.log_--patch-with-stat_master_--_dir_
b/t/t4013/diff.log_--patch-with-stat_master_--_dir_
index d5207ca..d3be9a2 100644
--- a/t/t4013/diff.log_--patch-with-stat_master_--_dir_
+++ b/t/t4013/diff.log_--patch-with-stat_master_--_dir_
@@ -6,6 +6,23 @@ Date: Mon Jun 26 00:04:00 2006 +
Merge branch 'side'
+ dir/sub | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --cc dir/sub
+index cead32e,7289e35..992913c
+--- a/dir/sub
b/dir/sub
+@@@ -1,6 -1,4 +1,8 @@@
+ A
+ B
+ +C
+ +D
+ +E
+ +F
++ 1
++ 2
+
commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
Author: A U Thor
Date: Mon Jun 26 00:03:00 2006 +
diff --git a/t/t4013/diff.log_--root_--patch-with-stat_--summary_master
b/t/t4013/diff.log_--root_--patch-with-stat_--summary_master
index dffc09d..a6aa60a 100644
--- a/t/t4013/diff.log_--root_--patch-with-stat_--summary_master
+++ b/t/t4013/diff.log_--root_--patch-with-stat_--summary_master
@@ -6,6 +6,38 @@ Date: Mon Jun 26 00:04:00 2006 +
Merge branch 'side'
+ dir/sub | 2 ++
+ file0 | 3 +++
+ 2 files changed, 5 insertions(+)
+
+diff --
Which branch(es) contain certain commits? (was Re: (unknown))
17.08.2015, 20:49, "Junio C Hamano" : > Duy Nguyen writes: > >> On Wed, Aug 5, 2015 at 7:47 PM, Ivan Chernyavsky >> wrote: > > That is a dangeous thought. I'd understand if it were internally > two step process, i.e. (1) the first pass finds commits that hits > the --grep criteria and then (2) the second pass does "--contains" > for all the hits found in the first pass using existing code, but > still, this operation is bound to dig all the way through the root > of the history when asked to find something that does not exist. My intention was to use existing git-branch filters such as -a or -r and pattern to limit the scope, then apply --grep machinery. But now I had a look on the source and I can see that builtin/branch.c builds the list of references and prints them in a single place (print_ref_list()) so I will have to split that function into two in order to reuse existing functionality. Another problem is that builtin/branch.c currently does not use setup_revisions(), so I'll have to hook it there as well. Then, I assume, I'll need to use the initial ref_list (filled the same as for the current "list" case) to configure the rev_info structure after setup_revisions(), and start revision traversal. I'm not sure I've got it all right from the source in those few days, so I apologize in advance if it's stupid in some part or as a whole. That said, do you think the goal is worth such changes? Seems like git-branch currently has it's own way of doing things and I'm trying to teach it to use git-rev-list's. Maybe it is more reasonable to add an option to "git log --all --source --grep=..." to print *all* branch tips which are reachable from the found commits, not only the first one encountered? E.g. --decorate-all or --source-all? > You should at least feed all --contains to a single invocation of > "git branch". They are designed to be OR'ed together. Yep thanks I overlooked that. In my repository, this takes 40 seconds instead of 2 minutes 30 seconds for my construct. -- Ivan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 1:47 PM, Junio C Hamano wrote: > > I do not think we are on the same wavelength. What I meant was to > do this: > > aq = xmalloc(...); > set up _everything_ in aq and make it a consistent state; > /* aq->first and aq->last are part of _everything_ in aq */ > for (many times) > pthread_create(...); > > /* No aq->first = aq->last = NULL assignment here */ > > instead of > > aq = xmalloc(...); > set up part of aq; > for (many times) > pthread_create(...); > belatedly initialize aq->first and aq->last and finally > aq becomes a consistent state. > > which is what we see above. The latter works _only_ because the > threads created are blocked waiting on aq->workingcount which is > initialized to block before threads are created to run dispatch, > and one of the early things dispatch does is to try acquiring that > semaphore to block before accessing aq->first and aq->last. I see your point and it makes sense to me as it makes the mental memory model cleaner I viewed the methods as atomic units (you would not call add_task before the constructor has finished, so no harm there. And a dispatcher in the pthread would just block on the `workingcount` semaphore as the semaphores are the main inter thread communication. I viewed the queue as just a secondary thing to distribute the work load. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PoC PATCH] completion: support 'git worktree'
Signed-off-by: SZEDER Gábor
---
> I wrote a completion function for 'git worktree' as well, turns out a week
> or two before you posted this, but I never submitted it as it was way too
> convoluted. Anyway, will send it in reply to this, just for reference.
And here it is.
>From the number of indentation levels and comment lines you can see why
I haven't submitted this patch yet :)
OTOH it offers refs for -b and -B, and there are only fairly narrow
corner cases when 'git --options' can fool it (but that's a general
issue with __git_find_on_cmdline(), I wouldn't go into that).
contrib/completion/git-completion.bash | 59 ++
1 file changed, 59 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648d7e..20a17e2c50 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2564,6 +2564,65 @@ _git_whatchanged ()
_git_log
}
+_git_worktree ()
+{
+ local subcommand subcommand_idx sc c=1
+ local subcommands="add prune"
+
+ while [ $c -lt $cword ] && [ -z "$subcommand" ]; do
+ for sc in $subcommands; do
+ if [ "$sc" = "${words[c]}" ]; then
+ subcommand=$sc
+ subcommand_idx=$c
+ break
+ fi
+ done
+ ((c++))
+ done
+
+ case "$subcommand,$cur" in
+ ,*)
+ __gitcomp "$subcommands"
+ ;;
+ add,--*)
+ __gitcomp "--detach"
+ ;;
+ add,*)
+ case "$prev" in
+ -b|-B)
+ __gitcomp_nl "$(__git_refs)"
+ ;;
+ -*) # $prev is an option without argument: have to complete
+ # the path for the new worktree, fall back to bash
+ # filename completion
+ ;;
+ *) # $prev is not an option, so it must be either the
+ # 'add' subcommand, an argument of an option (e.g.
+ # branch for -b|-B), or the path for the new worktree
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
+ # right after the 'add' subcommand, have to
+ # complete the path
+ :
+ else
+ case "${words[cword-2]}" in
+ -b|-B) # after '-b ', have to complete
+ # the path
+ ;;
+ *) # after the path, have to complete the
+ # branch to be checked out
+ __gitcomp_nl "$(__git_refs)"
+ ;;
+ esac
+ fi
+ ;;
+ esac
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run --verbose --expire"
+ ;;
+ esac
+}
+
__git_main ()
{
local i c=1 command __git_dir
--
2.5.0.418.gdd37a9b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] contrib: teach completion about git-worktree options and arguments
Quoting Junio C Hamano :
Eric Sunshine writes:
On Thu, Jul 23, 2015 at 4:49 PM, Eric Sunshine
wrote:
Complete subcommands 'add' and 'prune', as well as their respective
options --force, --detach, --dry-run, --verbose, and --expire. Also
complete 'refname' in "git worktree add [-b ]
".
Ping[1]?
[1]: http://article.gmane.org/gmane.comp.version-control.git/274526
Ping indeed?
Yeah, right, sorry. Non-subscribed occasional gmane-reader here amidst
job hunting, so thanks for the ping. And the re-ping...
Signed-off-by: Eric Sunshine
---
This is RFC since this is my first foray into the Git completion script,
and there are likely better and more idiomatic ways to accomplish this.
Using __git_find_on_cmdline() to find subcommands and case
"$subcommand,$cur" to limit the number of nested case statements is as
idiomatic as you can get in the completion script.
And I hear you, that " first, second" syntax makes it way
too complicated, especially since they can follow '-b '.
I wrote a completion function for 'git worktree' as well, turns out a week
or two before you posted this, but I never submitted it as it was way too
convoluted. Anyway, will send it in reply to this, just for reference.
contrib/completion/git-completion.bash | 32
1 file changed, 32 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..07c34ef 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2564,6 +2564,38 @@ _git_whatchanged ()
_git_log
}
+_git_worktree ()
+{
+ local subcommands='add prune'
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ local c=2 n=0 refpos=2
A more descriptive variable name for 'n' would be great.
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+ else
+ case "$subcommand,$cur" in
+ add,--*)
+ __gitcomp "--force --detach"
We usually don't offer '--force', because that option must be
handled with care.
+ ;;
+ add,*)
+ while [ $c -lt $cword ]; do
+ case "${words[c]}" in
+ --*) ;;
+ -[bB]) ((refpos++)) ;;
+ *) ((n++)) ;;
+ esac
+ ((c++))
+ done
+ if [ $n -eq $refpos ]; then
I suppose here you wanted to calculate where (i.e. at which word on
the command line) we should offer refs and fall back to bash builtin
filename completion otherwise. It works well in the common cases,
but:
- it doesn't offer refs after -b or -B
- it gets fooled by options to the git command, e.g. 'git
--git-dir=.git worktree add ' offers refs instead of files,
'git --git-dir=.git worktree add ../some/path ' offers
refs, etc.
+ __gitcomp_nl "$(__git_refs)"
+ fi
+ ;;
+ prune,--*)
+ __gitcomp "--dry-run --verbose --expire"
+ ;;
+ esac
+ fi
+}
+
__git_main ()
{
local i c=1 command __git_dir
--
2.5.0.rc3.407.g68aafd0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
Stefan Beller writes:
>>> +struct task_queue *create_task_queue(unsigned max_threads)
>>> +{
>>> + struct task_queue *aq = xmalloc(sizeof(*aq));
>>> +
>>> +#ifndef NO_PTHREADS
>>> + int i;
>>> + if (!max_threads)
>>> + aq->max_threads = online_cpus();
>>> + else
>>> + aq->max_threads = max_threads;
>>> +
>>> + sem_init(&aq->mutex, 0, 1);
>>> + sem_init(&aq->workingcount, 0, 0);
>>> + sem_init(&aq->freecount, 0, aq->max_threads);
>>> + aq->threads = xmalloc(aq->max_threads * sizeof(pthread_t));
>>> +
>>> + for (i = 0; i < aq->max_threads; i++)
>>> + pthread_create(&aq->threads[i], 0, &dispatcher, aq);
>>> +
>>> + aq->first = NULL;
>>> + aq->last = NULL;
>>
>>
>> Shouldn't these be initialized before letting threads call into
>> dispatcher? The workingcount semaphore that is initialized to 0 may
>> prevent them from peeking into these pointers and barfing, but still...
>
> They are initialized to NULL as the empty queue doesn't need a
> container element.
> Do we do queues in another way usually?
I do not think we are on the same wavelength. What I meant was to
do this:
aq = xmalloc(...);
set up _everything_ in aq and make it a consistent state;
/* aq->first and aq->last are part of _everything_ in aq */
for (many times)
pthread_create(...);
/* No aq->first = aq->last = NULL assignment here */
instead of
aq = xmalloc(...);
set up part of aq;
for (many times)
pthread_create(...);
belatedly initialize aq->first and aq->last and finally
aq becomes a consistent state.
which is what we see above. The latter works _only_ because the
threads created are blocked waiting on aq->workingcount which is
initialized to block before threads are created to run dispatch,
and one of the early things dispatch does is to try acquiring that
semaphore to block before accessing aq->first and aq->last.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
Jeff King writes: > On Fri, Aug 21, 2015 at 12:05:13PM -0700, Junio C Hamano wrote: > >> The primary reason I suspect is because you sent to a wrong set of >> people. Submodule folks have largely been working in the scripted >> ones, and may not necessarily be the ones who are most familiar with >> the run-command infrastructure. >> >> "shortlog --no-merges" tells me that the obvious suspects are j6t >> and peff. > > No good deed goes unpunished. ;) > > Before even looking at the implementation, my first question would be > whether this pattern is applicable in several places in git (i.e., is it > worth the extra complexity of abstracting out in the first place). I > think there are a few task-queue patterns already in git; for example > the delta search in pack-objects. Is the interface given here sufficient > to convert pack-objects? Is the result nicer to read? Is it as > efficient? > > We do not need to convert all possible call-sites to the new abstracted > code at once. But I find that converting at least _one_ is a good litmus > test to confirm that a new interface is generally useful. Ah, thanks for saying this. I recall saying something similar earlier and totally agree with you. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Urgent, Dear Invited Author,
Dear Invited Author, You have 6 days until August 25 for your Invited Paper in the WSEAS Conferences in Seoul, South Korea. We have just upgraded the role of Invited Speakers (see below). The extended version of the accepted papers will be published in our collaborating 44 ISI Journals. If August 25 is too early for you to upload your invited paper in our conferences in Seoul, South Korea, you must know that we have also conferences in Michigan State University, East Lansing, MI, USA, September 20-22, 2015 and we would like to invite you to come as Invited Speaker to present an Invited Paper. So, select one of our conferences in Seouls, Korea and/or Michigan State University, USA via http://www.wseas.org and upload your invited paper until until August 25 for Seouls, Korea and September 8, 2015 for Michigan State University, East Lansing, MI (Maximum 10 pages. Format see the web page of the conference) Michigan State University is co-sponsor and co-organizer and the events will be held inside the buildings of the University This is really a unique opportunity with publication in our ISI (indexed by Thomson Reuters) Journals You can publish your paper in one of our 44 ISI (indexed by Thomson Reuters) Journals Upload your invited paper now by selecting the most appropriate conference via www.wseas.org Invited Authors can also have an Interview in our channel (See some recent interviews of our Invited and Plenary Speakers in the third column of our Web Portal under the "Bulletin Board"). You can follow us in Facebook: www.facebook.con/Wseas and you can see several photos and video from our recent conferences here: wseas.org The ISI Journals (with Impact Factor from Thomson Reuters) that publish the accepted papers from WSEAS Conferences are now 44. Several ISI Journals sent us proposals for collaboration in November and December of 2014, so the number of ISI Journals that publish the accepted papers of WSEAS conferences are now 44. Each journal will host approximately 10-15 papers from WSEAS Conferences. You can upload your Invited Paper if you fill-in in the field "Session": invited-by-olga-nikolova-...@vger.kernel.org Do not send papers by email. Invited Authors have == a) Invitation for Additional publication of the extended version in ISI Journals and Positive Recommendation from an Editor for publication in an ISI Journal. You select the Journal where you can publish the extended version of your paper. b) Priority in the conference program c) Double time for presentation (not 15 minutes as the regular papers, but 30 minutes) d) Invited Papers are noted explicitly in the conference program as Invited Papers e) Invited Authors can be included in the Program Committee of our future conferences of 2015 f) Invitation for Co-organizing a future WSEAS Event. g) 20 additional days to upload their invited papers All the authors have Publication in the Conference proceedings and distribution in the conference 10 Books + 1 CD-ROM (total of the 10 books) You must upload your Invited Paper only via the particular conference web site and you must declare in the Field of Special Session this code: invited-by-olga-nikolova-...@vger.kernel.org (in order to consider your paper as Invited paper). Do not send papers by email. Remarks - 1) Invited Authors represent only a 3-5% of our authors, see for example the papers with Italics here: http://www.wseas.org/main/conferences/2014/Salerno/Program.pdf ) 2) Invited Authors are selected very carefully according their recent publications and citations and they must fit to the topics of the conference. 3) If you want, you can give also an Interview in our Web Channel during the days of the conference. Please, check: http://wseas.org/cms.action?id=8322 Do not hesitate to contact me for more questions Indexing === ISI (Thomson Reuters), ELSEVIER, SCOPUS, ACM - Association for Computing Machinery, Zentralblatt MATH, British Library, EBSCO, SWETS, EMBASE, CAS - American Chemical Society, CiteSeerx, Cabell Publishing, Electronic Journals, Library, SAO/NASA Astrophysics Data System EI Compendex, Engineering Village, CSA Cambridge Scientific Abstracts, DoPP, GEOBASE, Biobase, American Mathematical Society (AMS), Inspec - The IET, Ulrich's International Periodicals Directory The ISI Journals (with Impact Factor from Thomson Reuters) that publish the accepted papers from WSEAS Conferences are now 44. Several ISI Journals sent us proposals for collaboration in November and December of 2014, so the number of ISI Journals that publish the accepted papers of WSEAS conferences are now 33. Each journal will host approximately 10-15 papers from WSEAS Conferences in Michigan State University, East Lansing, MI, September 20-22, 2015 Best Regards (more details: publish...@wseas.org ) Dr. Olga Nikolova WSEAS Editorial Office www.wseas.org publish...@w
Re: [WIP/PATCH 3/3] submodule: helper to run foreach in parallel
On Fri, Aug 21, 2015 at 12:23 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> +struct submodule_args {
>> + const char *name;
>> + const char *path;
>> + const char *sha1;
>> + const char *toplevel;
>> + const char *prefix;
>> + const char **cmd;
>> + struct submodule_output *out;
>> + sem_t *mutex;
>> +};
>
> I do not see what submodule_output looks like in the patch, but I
> guess you are capturing _all_ output from a task and emitting
> everything at the end, after the task is done?
It was a leftover and will be removed. Sorry for wasting your time.
So jrnieder and me had a discussion on how to do get the output right.
And we had 2 designs:
1) The worker thread acquires a mutex to be allowed to write to
stdout. This makes early output easy for each thread in case of
error messages. It also requires less resources than 2).
Also the expected code complexity is lower as the decisions
are made locally.
2) Have a single handler which deals with all output of all tasks.
This handler would `select` on a huge number of pipes from
all the tasks, (so we would require a lot of pipes). And this central
handler (which would be submodule_output in this case) would
take care of having no intermingled racy output from tasks, with
prefixes and all bells and whistles. This handler could also
give a progress meter (300 out of 500 submodules updated already)
for all threads, or when the last task is running switch to piping
the output of that task to stdout in real time, so you'd get progress
of the last task as you're already used to.
The outcome of the discussion was to split the worker pool/load
distribution from the output handling in any case as there is no need
to couple them. In my very first designs I had the output handling
as part of the asynchronous worker pool.
This RFC implements 1), that's why there is only the mutex.
I plan on enhancing this to let the last task output in real time (no buffering)
as well as another counter "task n/m completed" after each task
is done.
>
> I have to wonder if that is what people would expect and more
> importantly if that is the most useful. I am sympathetic to the
> desire to avoid the output totally mixed up from different processes
> emitting things in an uncoordinated manner, and "slurp everything
> and then show everything in one go" is certainly _one_ way to do so,
> but early feedback also counts. Besides, because the order in which
> tasks are dispatched and completed is unpredictable, you cannot
> expect a machine parseable output _without_ assuming some help from
> the command invoked by each task (e.g. by prefixing the task's output
> with some string that identifies which submodule the output is about).
I was very focused on the "submodule foreach" output, which (in case of
no -q given), displays a
"Entering %s"
as the first line for each finished task. I suspect that is not enough to
make it a good machine parseable output. So I will prefix each
line with a running number of the task. Maybe like this:
[001/500] Entering 'foo/bar'
[001/500] Here goes the text for task foo/bar
[001/500] Here goes another text for task foo/bar
[002/500] Entering 'foo/baz'
[002/500] Here goes the text for task foo/baz
[002/500] Here goes another text for task foo/bar
[003/500] Entering 'foo/bla'
...
This will make the output for each task distinguishable from
other tasks as well offering some sort of progress meter.
(The fewer submodules you have the less fine grained
the progress bar becomes)
Jonathan suggested to add a capability to the git protocol for a
machine readable progress meter in another channel. so we do not
need to parse the current output Counting/Compressing/Transfer/etc
>
> Once you assume that the command is _aware_ that it needs to help
> the foreach-parallel infrastructure so that the user can sift their
> collective outputs to make sense of them, wouldn't a line-buffered
> intermixing also acceptable, and wouldn't it be a much lower impact
> approach to solve the same problem?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:51 PM, Jeff King wrote: > On Fri, Aug 21, 2015 at 12:48:23PM -0700, Stefan Beller wrote: > >> > Before even looking at the implementation, my first question would be >> > whether this pattern is applicable in several places in git (i.e., is it >> > worth the extra complexity of abstracting out in the first place). I >> > think there are a few task-queue patterns already in git; for example >> > the delta search in pack-objects. Is the interface given here sufficient >> > to convert pack-objects? Is the result nicer to read? Is it as >> > efficient? >> > >> > We do not need to convert all possible call-sites to the new abstracted >> > code at once. But I find that converting at least _one_ is a good litmus >> > test to confirm that a new interface is generally useful. >> >> Ok, I'll head off to convert one place. > > Thanks. By the way, reading over what I wrote, it sounds a little > harsher than I meant. It did not sound harsh at all. I was just reading an internal mailing list, which cites over generalizing as a very bad practice worse than premature optimization, so I totally understand your concern and agree. > It is not "if you do not convert an existing site, > we cannot accept a new interface, period". But I would like at least > some explanation as an alternative, like "I'm pretty sure we can convert > site X to this, but it is not a good time to do so now because of Y". > Where "Y" might be "we need to do this other refactoring work first", or > "it would be disruptive to other work in the area". > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:48:23PM -0700, Stefan Beller wrote: > > Before even looking at the implementation, my first question would be > > whether this pattern is applicable in several places in git (i.e., is it > > worth the extra complexity of abstracting out in the first place). I > > think there are a few task-queue patterns already in git; for example > > the delta search in pack-objects. Is the interface given here sufficient > > to convert pack-objects? Is the result nicer to read? Is it as > > efficient? > > > > We do not need to convert all possible call-sites to the new abstracted > > code at once. But I find that converting at least _one_ is a good litmus > > test to confirm that a new interface is generally useful. > > Ok, I'll head off to convert one place. Thanks. By the way, reading over what I wrote, it sounds a little harsher than I meant. It is not "if you do not convert an existing site, we cannot accept a new interface, period". But I would like at least some explanation as an alternative, like "I'm pretty sure we can convert site X to this, but it is not a good time to do so now because of Y". Where "Y" might be "we need to do this other refactoring work first", or "it would be disruptive to other work in the area". -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:44 PM, Jeff King wrote: > On Fri, Aug 21, 2015 at 12:05:13PM -0700, Junio C Hamano wrote: > >> The primary reason I suspect is because you sent to a wrong set of >> people. Submodule folks have largely been working in the scripted >> ones, and may not necessarily be the ones who are most familiar with >> the run-command infrastructure. >> >> "shortlog --no-merges" tells me that the obvious suspects are j6t >> and peff. > > No good deed goes unpunished. ;) > > Before even looking at the implementation, my first question would be > whether this pattern is applicable in several places in git (i.e., is it > worth the extra complexity of abstracting out in the first place). I > think there are a few task-queue patterns already in git; for example > the delta search in pack-objects. Is the interface given here sufficient > to convert pack-objects? Is the result nicer to read? Is it as > efficient? > > We do not need to convert all possible call-sites to the new abstracted > code at once. But I find that converting at least _one_ is a good litmus > test to confirm that a new interface is generally useful. Ok, I'll head off to convert one place. > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:05 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> This adds functionality to do work in parallel.
>>
>> The whole life cycle of such a thread pool would look like
>>
>> struct task_queue * tq = create_task_queue(32); // no of threads
>> for (...)
>> add_task(tq, process_one_item_function, item); // non blocking
>> ...
>> int ret = finish_task_queue(tq); // blocks until all tasks are done
>> if (!tq)
>> die ("Not all items were be processed");
>>
>> The caller must take care of handling the output.
>>
>> Signed-off-by: Stefan Beller
>> ---
>>
>> I sent this a while ago to the list, no comments on it :(
>
> The primary reason I suspect is because you sent to a wrong set of
> people. Submodule folks have largely been working in the scripted
> ones, and may not necessarily be the ones who are most familiar with
> the run-command infrastructure.
>
> "shortlog --no-merges" tells me that the obvious suspects are j6t
> and peff.
noted.
>
>> The core functionality stayed the same, but I hope to improved naming and
>> location of the code.
>>
>> The WIP is only for the NO_PTHREADS case.
>
>> run-command.c | 212
>> ++
>> run-command.h | 30 +
>> 2 files changed, 230 insertions(+), 12 deletions(-)
>>
>> diff --git a/run-command.c b/run-command.c
>> index 28e1d55..4029011 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -4,6 +4,21 @@
>> #include "sigchain.h"
>> #include "argv-array.h"
>>
>> +#ifdef NO_PTHREADS
>> +
>> +#else
>> +
>> +#include "thread-utils.h"
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#endif
>> +
>> +#include "git-compat-util.h"
>> +
>
> This goes against the way we have been organizing the header files.
>
> http://thread.gmane.org/gmane.comp.version-control.git/276241/focus=276265
ok
>
>> @@ -668,6 +683,22 @@ int git_atexit(void (*handler)(void))
>>
>> #endif
>>
>> +void setup_main_thread()
>
> void setup_main_thread(void)
>
>> @@ -852,3 +872,171 @@ int capture_command(struct child_process *cmd, struct
>> strbuf *buf, size_t hint)
>> close(cmd->out);
>> return finish_command(cmd);
>> }
>> +
>> +#ifndef NO_PTHREADS
>> +struct job_list {
>> + int (*fct)(struct task_queue *aq, void *task);
>> + void *task;
>> + struct job_list *next;
>> +};
>> +#endif
>> +
>> +struct task_queue {
>> +#ifndef NO_PTHREADS
>> + /*
>> + * To avoid deadlocks always aquire the semaphores with lowest priority
>
> acquire.
>
>> + * first, priorites are in descending order as listed.
>> + *
>> + * The `mutex` is a general purpose lock for modifying data in the
>> async
>> + * queue, such as adding a new task or adding a return value from
>> + * an already run task.
>> + *
>> + * `workingcount` and `freecount` are opposing semaphores, the sum of
>> + * their values should equal `max_threads` at any time while the
>> `mutex`
>> + * is available.
>> + */
>> + sem_t mutex;
>> + sem_t workingcount;
>> + sem_t freecount;
>> +
>> + pthread_t *threads;
>> + unsigned max_threads;
>> +
>> + struct job_list *first;
>> + struct job_list *last;
>> +#endif
>> + int early_return;
>> +};
>> +
>> +#ifndef NO_PTHREADS
>> +
>> +static void get_task(struct task_queue *aq,
>> + int (**fct)(struct task_queue *aq, void *task),
>> + void **task,
>> + int *early_return)
>> +{
>> + struct job_list *job;
>> +
>> + sem_wait(&aq->workingcount);
>> + sem_wait(&aq->mutex);
>> +
>> + if (!aq->first)
>> + die("BUG: internal error with dequeuing jobs for threads");
>> + job = aq->first;
>> + *fct = job->fct;
>> + *task = job->task;
>> + aq->early_return |= *early_return;
>> + *early_return = aq->early_return;
>> + aq->first = job->next;
>> + if (!aq->first)
>> + aq->last = NULL;
>> +
>> + sem_post(&aq->freecount);
>> + sem_post(&aq->mutex);
>> +
>> + free(job);
>> +}
>> +
>> +static void* dispatcher(void *args)
>
> static void *dispatcher()
>
>> +{
>> + void *task;
>> + int (*fct)(struct task_queue *aq, void *data);
>
> s/data/task/?
>
>> + int early_return = 0;
>> + struct task_queue *aq = args;
>> +
>> + get_task(aq, &fct, &task, &early_return);
>> + while (fct || early_return != 0) {
>> + early_return = fct(aq, task);
>> + get_task(aq, &fct, &task, &early_return);
>> + }
>
> If the func said "we are done, you may stop dispatching now", do you
> still want to do another get_task()?
This question shows me I messed up readability of this. `get_task` both gets
a new task as well as taking the value from early_return writing it to
aq->early_return,
such that other threads are notified.
Maybe I should do that explicitely here and not get the new task.
>
>> + pthread_exit(0);
>
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
On Fri, Aug 21, 2015 at 12:05:13PM -0700, Junio C Hamano wrote: > The primary reason I suspect is because you sent to a wrong set of > people. Submodule folks have largely been working in the scripted > ones, and may not necessarily be the ones who are most familiar with > the run-command infrastructure. > > "shortlog --no-merges" tells me that the obvious suspects are j6t > and peff. No good deed goes unpunished. ;) Before even looking at the implementation, my first question would be whether this pattern is applicable in several places in git (i.e., is it worth the extra complexity of abstracting out in the first place). I think there are a few task-queue patterns already in git; for example the delta search in pack-objects. Is the interface given here sufficient to convert pack-objects? Is the result nicer to read? Is it as efficient? We do not need to convert all possible call-sites to the new abstracted code at once. But I find that converting at least _one_ is a good litmus test to confirm that a new interface is generally useful. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] format_config: simplify buffer handling
On Fri, Aug 21, 2015 at 10:43:58AM -0700, Junio C Hamano wrote:
> Junio C Hamano writes:
>
> > I wonder if we can do this instead
> >
> > if (!omit_values) {
> > - if (show_keys)
> > + if (show_keys && value_)
> > strbuf_addch(buf, key_delim);
> >
> > though. That would eliminate the need for rolling back.
>
> No we cannot. "config --bool --get-regexp" could get a NULL value_
> and has to turn it to " true".
>
> Sorry for the noise.
Right, I had the same thought and reached the same conclusion.
By the way, I do not think the rollback by itself is gross, it is just
that it has to reproduce the "show_keys" logic. That is, it _really_
wants to be:
else {
/* nothing to show; rollback delim */
if (we_added_delim)
strbuf_setlen(buf, buf->len - 1);
}
and I use "show_keys" as a proxy for "did we add it". Which is
reproducing the logic that you quoted above, and if the two ever get out
of sync, it will be a bug. So you could write it as:
int we_added_delim = 0;
if (show_keys) {
strbuf_addch(buf, key_delim);
we_added_delim = 1;
}
I didn't, because it's ugly, and the function is short enough and
unlikely enough to change that it probably won't matter.
You could perhaps also write it as:
size_t baselen = buf->len;
if (show_keys)
strbuf_addch(buf, key_delim);
...
else {
/* rollback delim */
strbuf_setlen(buf, baselen);
}
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH 3/3] submodule: helper to run foreach in parallel
Stefan Beller writes:
> +struct submodule_args {
> + const char *name;
> + const char *path;
> + const char *sha1;
> + const char *toplevel;
> + const char *prefix;
> + const char **cmd;
> + struct submodule_output *out;
> + sem_t *mutex;
> +};
I do not see what submodule_output looks like in the patch, but I
guess you are capturing _all_ output from a task and emitting
everything at the end, after the task is done?
I have to wonder if that is what people would expect and more
importantly if that is the most useful. I am sympathetic to the
desire to avoid the output totally mixed up from different processes
emitting things in an uncoordinated manner, and "slurp everything
and then show everything in one go" is certainly _one_ way to do so,
but early feedback also counts. Besides, because the order in which
tasks are dispatched and completed is unpredictable, you cannot
expect a machine parseable output _without_ assuming some help from
the command invoked by each task (e.g. by prefixing the task's output
with some string that identifies which submodule the output is about).
Once you assume that the command is _aware_ that it needs to help
the foreach-parallel infrastructure so that the user can sift their
collective outputs to make sense of them, wouldn't a line-buffered
intermixing also acceptable, and wouldn't it be a much lower impact
approach to solve the same problem?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
Stefan Beller writes:
> This adds functionality to do work in parallel.
>
> The whole life cycle of such a thread pool would look like
>
> struct task_queue * tq = create_task_queue(32); // no of threads
> for (...)
> add_task(tq, process_one_item_function, item); // non blocking
> ...
> int ret = finish_task_queue(tq); // blocks until all tasks are done
> if (!tq)
> die ("Not all items were be processed");
>
> The caller must take care of handling the output.
>
> Signed-off-by: Stefan Beller
> ---
>
> I sent this a while ago to the list, no comments on it :(
The primary reason I suspect is because you sent to a wrong set of
people. Submodule folks have largely been working in the scripted
ones, and may not necessarily be the ones who are most familiar with
the run-command infrastructure.
"shortlog --no-merges" tells me that the obvious suspects are j6t
and peff.
> The core functionality stayed the same, but I hope to improved naming and
> location of the code.
>
> The WIP is only for the NO_PTHREADS case.
> run-command.c | 212
> ++
> run-command.h | 30 +
> 2 files changed, 230 insertions(+), 12 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index 28e1d55..4029011 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -4,6 +4,21 @@
> #include "sigchain.h"
> #include "argv-array.h"
>
> +#ifdef NO_PTHREADS
> +
> +#else
> +
> +#include "thread-utils.h"
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#endif
> +
> +#include "git-compat-util.h"
> +
This goes against the way we have been organizing the header files.
http://thread.gmane.org/gmane.comp.version-control.git/276241/focus=276265
> @@ -668,6 +683,22 @@ int git_atexit(void (*handler)(void))
>
> #endif
>
> +void setup_main_thread()
void setup_main_thread(void)
> @@ -852,3 +872,171 @@ int capture_command(struct child_process *cmd, struct
> strbuf *buf, size_t hint)
> close(cmd->out);
> return finish_command(cmd);
> }
> +
> +#ifndef NO_PTHREADS
> +struct job_list {
> + int (*fct)(struct task_queue *aq, void *task);
> + void *task;
> + struct job_list *next;
> +};
> +#endif
> +
> +struct task_queue {
> +#ifndef NO_PTHREADS
> + /*
> + * To avoid deadlocks always aquire the semaphores with lowest priority
acquire.
> + * first, priorites are in descending order as listed.
> + *
> + * The `mutex` is a general purpose lock for modifying data in the async
> + * queue, such as adding a new task or adding a return value from
> + * an already run task.
> + *
> + * `workingcount` and `freecount` are opposing semaphores, the sum of
> + * their values should equal `max_threads` at any time while the `mutex`
> + * is available.
> + */
> + sem_t mutex;
> + sem_t workingcount;
> + sem_t freecount;
> +
> + pthread_t *threads;
> + unsigned max_threads;
> +
> + struct job_list *first;
> + struct job_list *last;
> +#endif
> + int early_return;
> +};
> +
> +#ifndef NO_PTHREADS
> +
> +static void get_task(struct task_queue *aq,
> + int (**fct)(struct task_queue *aq, void *task),
> + void **task,
> + int *early_return)
> +{
> + struct job_list *job;
> +
> + sem_wait(&aq->workingcount);
> + sem_wait(&aq->mutex);
> +
> + if (!aq->first)
> + die("BUG: internal error with dequeuing jobs for threads");
> + job = aq->first;
> + *fct = job->fct;
> + *task = job->task;
> + aq->early_return |= *early_return;
> + *early_return = aq->early_return;
> + aq->first = job->next;
> + if (!aq->first)
> + aq->last = NULL;
> +
> + sem_post(&aq->freecount);
> + sem_post(&aq->mutex);
> +
> + free(job);
> +}
> +
> +static void* dispatcher(void *args)
static void *dispatcher()
> +{
> + void *task;
> + int (*fct)(struct task_queue *aq, void *data);
s/data/task/?
> + int early_return = 0;
> + struct task_queue *aq = args;
> +
> + get_task(aq, &fct, &task, &early_return);
> + while (fct || early_return != 0) {
> + early_return = fct(aq, task);
> + get_task(aq, &fct, &task, &early_return);
> + }
If the func said "we are done, you may stop dispatching now", do you
still want to do another get_task()?
> + pthread_exit(0);
> +}
> +#endif
> +
> +struct task_queue *create_task_queue(unsigned max_threads)
> +{
> + struct task_queue *aq = xmalloc(sizeof(*aq));
> +
> +#ifndef NO_PTHREADS
> + int i;
> + if (!max_threads)
> + aq->max_threads = online_cpus();
> + else
> + aq->max_threads = max_threads;
> +
> + sem_init(&aq->mutex, 0, 1);
> + sem_init(&aq->workingcount, 0, 0);
> + sem_init(&aq->freecount, 0, aq->max_threads);
> + aq->threads = xmalloc(aq->max_threads * sizeof(pthread_t));
> +
> +
[PATCH 4/3] builtin/log.c: minor reformat
Two logical lines that were not overly long was split in the middle,
which made them read worse.
Signed-off-by: Junio C Hamano
---
builtin/log.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 0cdd889..a491d3d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -342,8 +342,7 @@ static int cmd_log_walk(struct rev_info *rev)
* retain that state information if replacing rev->diffopt in this loop
*/
while ((commit = get_revision(rev)) != NULL) {
- if (!log_tree_commit(rev, commit) &&
- rev->max_count >= 0)
+ if (!log_tree_commit(rev, commit) && rev->max_count >= 0)
/*
* We decremented max_count in get_revision,
* but we didn't actually show the commit.
@@ -1464,8 +1463,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
continue;
}
- if (ignore_if_in_upstream &&
- has_commit_patch_id(commit, &ids))
+ if (ignore_if_in_upstream && has_commit_patch_id(commit, &ids))
continue;
nr++;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
larsxschnei...@gmail.com writes: > From: Lars Schneider > > PROBLEM: > We run P4 servers on Linux and P4 clients on Windows. For an unknown > reason the file path for a number of files in P4 does not match the > directory path with respect to case sensitivity. > > E.g. `p4 files` might return > //depot/path/to/file1 > //depot/PATH/to/file2 > > If you use P4/P4V then these files end up in the same directory, e.g. > //depot/path/to/file1 > //depot/path/to/file2 > > If you use git-p4 then all files not matching the correct file path > (e.g. `file2`) will be ignored. > > SOLUTION: > Identify path names that are different with respect to case sensitivity. > If there are any then run `p4 dirs` to build up a dictionary > containing the "correct" cases for each path. It looks like P4 > interprets "correct" here as the existing path of the first file in a > directory. The path dictionary is used later on to fix all paths. > > This is only applied if the parameter "--ignore-path-case" is passed to > the git-p4 clone command. > > > Signed-off-by: Lars Schneider > --- Thanks. A few comments. - Have you checked "git log" on our history and notice how nobody says "PROBLEM:" and "SOLUTION:" in capital letters? Don't try to be original in the form; your contributions are already original and valuable in the substance ;-) - I think I saw v3 yesterday. It would be nice to see a brief description of what has been updated in this version. I do not do Perforce and am not familiar enough with this code to judge myself. Will wait for area experts (you have them CC'ed, which is good) to comment. > diff --git a/git-p4.py b/git-p4.py > index 073f87b..61a587b 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -1931,7 +1931,7 @@ class View(object): > (self.client_prefix, clientFile)) > return clientFile[len(self.client_prefix):] > > -def update_client_spec_path_cache(self, files): > +def update_client_spec_path_cache(self, files, fixPathCase = None): I didn't check the remainder of the file, but I thought it is customery *not* to have spaces around '=' when the parameter is written with its default value in Python? > diff --git a/t/t9821-git-p4-path-variations.sh > b/t/t9821-git-p4-path-variations.sh > ... > +test_expect_success 'Clone the repo and WITHOUT path fixing' ' > + client_view "//depot/One/... //client/..." && > + git p4 clone --use-client-spec --destination="$git" //depot && > + test_when_finished cleanup_git && > + ( > + cd "$git" && > + # This method is used instead of "test -f" to ensure the case is > + # checked even if the test is executed on case-insensitive file > systems. > + cat >expect <<-\EOF && > + two/File2.txt > + EOF I think we usually write the body of the indented here text (i.e. "<<-") indented to the same level as the command and delimiter, i.e. cat >expect <<-\EOF && body of the here document line 1 body of the here document line 2 ... EOF > + git ls-files >actual && > + test_cmp expect actual > + ) > +' I think you used to check the result with "find .", i.e. what the filesystem actually recorded. "ls-files" gives what the index records (i.e. to be committed if you were to make a new commit from that index). They can be different in case on case-insensitive filesystems, which I think are the ones that are most problematic with the issue your patch is trying to address. You are verifying what the set of "canonical" paths should be by checking ls-files output. I think that is what you intended to do (i.e. I am saying "I think the code is more correct than the earlier round that used find"), but I just am double checking. > +test_expect_success 'Add a new file and clone the repo WITH path fixing' ' > + client_view "//depot/... //client/..." && > + ( > + cd "$cli" && > + > + mkdir -p One/two && A blank after 'cd' only in this one but not others. A blank line is a good vehicle to convey that blocks of text above and below it are logically separate, but use it consistently. I _think_ this blank line can go. > + >One/two/File0.txt && > + p4 add One/two/File0.txt && Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] format_config: simplify buffer handling
Junio C Hamano writes:
> I wonder if we can do this instead
>
> if (!omit_values) {
> - if (show_keys)
> + if (show_keys && value_)
> strbuf_addch(buf, key_delim);
>
> though. That would eliminate the need for rolling back.
No we cannot. "config --bool --get-regexp" could get a NULL value_
and has to turn it to " true".
Sorry for the noise.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] format_config: simplify buffer handling
Jeff King writes:
> When formatting a config value into a strbuf, we may end
> up stringifying it into a fixed-size buffer using sprintf,
> and then copying that buffer into the strbuf. We can
> eliminate the middle-man (and drop some calls to sprintf!)
> by writing directly to the strbuf.
>
> The reason it was written this way in the first place is
> that we need to know before writing the value whether to
> insert a delimiter. Instead of delaying the write of the
> value, we speculatively write the delimiter, and roll it
> back in the single case that cares.
>
> Signed-off-by: Jeff King
> ---
> I admit the rollback is a little gross. The other option would be adding
> the delimiter in each of the conditional branches, which is also kind of
> nasty.
I actually am fine with this rollback. The "variable alone stands
for true" is not something a user can produce from the command line
very easily, so having to rollback is a rare event anyway.
I wonder if we can do this instead
if (!omit_values) {
- if (show_keys)
+ if (show_keys && value_)
strbuf_addch(buf, key_delim);
though. That would eliminate the need for rolling back.
I briefly wondered how such a change would interact with
if (types == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
git_config_int64(key_, value_ ? value_ : ""));
that immediately follows it, but this "turn NULL into an empty
string" may be bogus in the first place, in the sense that
git_config_int64() should complain about a NULL value_ the same way
as it would complain about an empty string---both them are not an
integer. And indeed:
- git_parse_int64() that is called from git_config_int64() is
prepared to take both "" and NULL and return failure with EINVAL;
- die_bad_number() that is eventually called when parsing fails by
git_config_int64() is prepared to take NULL and turns it to an
empty string.
So perhaps we could do this squashed in?
builtin/config.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 71acc44..593b1ae 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -111,12 +111,12 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) {
- if (show_keys)
+ if (show_keys && value_)
strbuf_addch(buf, key_delim);
if (types == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
- git_config_int64(key_, value_ ? value_ :
""));
+ git_config_int64(key_, value_));
else if (types == TYPE_BOOL)
strbuf_addstr(buf, git_config_bool(key_, value_) ?
"true" : "false");
@@ -136,9 +136,8 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
} else if (value_) {
strbuf_addstr(buf, value_);
} else {
- /* Just show the key name; back out delimiter */
- if (show_keys)
- strbuf_setlen(buf, buf->len - 1);
+ /* Just show the key name */
+ ;
}
}
strbuf_addch(buf, term);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] contrib: teach completion about git-worktree options and arguments
Eric Sunshine writes:
> On Thu, Jul 23, 2015 at 4:49 PM, Eric Sunshine
> wrote:
>> Complete subcommands 'add' and 'prune', as well as their respective
>> options --force, --detach, --dry-run, --verbose, and --expire. Also
>> complete 'refname' in "git worktree add [-b ]
>> ".
>
> Ping[1]?
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/274526
Ping indeed?
>
>> Signed-off-by: Eric Sunshine
>> ---
>>
>> This is RFC since this is my first foray into the Git completion script,
>> and there are likely better and more idiomatic ways to accomplish this.
>>
>> contrib/completion/git-completion.bash | 32
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index c97c648..07c34ef 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2564,6 +2564,38 @@ _git_whatchanged ()
>> _git_log
>> }
>>
>> +_git_worktree ()
>> +{
>> + local subcommands='add prune'
>> + local subcommand="$(__git_find_on_cmdline "$subcommands")"
>> + local c=2 n=0 refpos=2
>> + if [ -z "$subcommand" ]; then
>> + __gitcomp "$subcommands"
>> + else
>> + case "$subcommand,$cur" in
>> + add,--*)
>> + __gitcomp "--force --detach"
>> + ;;
>> + add,*)
>> + while [ $c -lt $cword ]; do
>> + case "${words[c]}" in
>> + --*) ;;
>> + -[bB]) ((refpos++)) ;;
>> + *) ((n++)) ;;
>> + esac
>> + ((c++))
>> + done
>> + if [ $n -eq $refpos ]; then
>> + __gitcomp_nl "$(__git_refs)"
>> + fi
>> + ;;
>> + prune,--*)
>> + __gitcomp "--dry-run --verbose --expire"
>> + ;;
>> + esac
>> + fi
>> +}
>> +
>> __git_main ()
>> {
>> local i c=1 command __git_dir
>> --
>> 2.5.0.rc3.407.g68aafd0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] git-p4: fix faulty paths for case insensitive systems
From: Lars Schneider
PROBLEM:
We run P4 servers on Linux and P4 clients on Windows. For an unknown
reason the file path for a number of files in P4 does not match the
directory path with respect to case sensitivity.
E.g. `p4 files` might return
//depot/path/to/file1
//depot/PATH/to/file2
If you use P4/P4V then these files end up in the same directory, e.g.
//depot/path/to/file1
//depot/path/to/file2
If you use git-p4 then all files not matching the correct file path
(e.g. `file2`) will be ignored.
SOLUTION:
Identify path names that are different with respect to case sensitivity.
If there are any then run `p4 dirs` to build up a dictionary
containing the "correct" cases for each path. It looks like P4
interprets "correct" here as the existing path of the first file in a
directory. The path dictionary is used later on to fix all paths.
This is only applied if the parameter "--ignore-path-case" is passed to
the git-p4 clone command.
Signed-off-by: Lars Schneider
---
git-p4.py | 82 ++--
t/t9821-git-p4-path-variations.sh | 109 ++
2 files changed, 187 insertions(+), 4 deletions(-)
create mode 100755 t/t9821-git-p4-path-variations.sh
diff --git a/git-p4.py b/git-p4.py
index 073f87b..61a587b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1931,7 +1931,7 @@ class View(object):
(self.client_prefix, clientFile))
return clientFile[len(self.client_prefix):]
-def update_client_spec_path_cache(self, files):
+def update_client_spec_path_cache(self, files, fixPathCase = None):
""" Caching file paths by "p4 where" batch query """
# List depot file paths exclude that already cached
@@ -1950,6 +1950,8 @@ class View(object):
if "unmap" in res:
# it will list all of them, but only one not unmap-ped
continue
+if fixPathCase:
+res['depotFile'] = fixPathCase(res['depotFile'])
self.client_spec_path_cache[res['depotFile']] =
self.convert_client_path(res["clientFile"])
# not found files or unmap files set to ""
@@ -1987,6 +1989,7 @@ class P4Sync(Command, P4UserMap):
help="Maximum number of changes to
import"),
optparse.make_option("--changes-block-size",
dest="changes_block_size", type="int",
help="Internal block size to use when
iteratively calling p4 changes"),
+optparse.make_option("--ignore-path-case",
dest="ignorePathCase", action="store_true"),
optparse.make_option("--keep-path", dest="keepRepoPath",
action='store_true',
help="Keep entire BRANCH/DIR/SUBDIR
prefix during import"),
optparse.make_option("--use-client-spec",
dest="useClientSpec", action='store_true',
@@ -2017,6 +2020,7 @@ class P4Sync(Command, P4UserMap):
self.maxChanges = ""
self.changes_block_size = None
self.keepRepoPath = False
+self.ignorePathCase = False
self.depotPaths = None
self.p4BranchesInGit = []
self.cloneExclude = []
@@ -2049,7 +2053,8 @@ class P4Sync(Command, P4UserMap):
files = []
fnum = 0
while commit.has_key("depotFile%s" % fnum):
-path = commit["depotFile%s" % fnum]
+path = commit["depotFile%s" % fnum]
+path = self.fixPathCase(path)
if [p for p in self.cloneExclude
if p4PathStartsWith(path, p)]:
@@ -2113,7 +2118,9 @@ class P4Sync(Command, P4UserMap):
branches = {}
fnum = 0
while commit.has_key("depotFile%s" % fnum):
-path = commit["depotFile%s" % fnum]
+path = commit["depotFile%s" % fnum]
+path = self.fixPathCase(path)
+
found = [p for p in self.depotPaths
if p4PathStartsWith(path, p)]
if not found:
@@ -2240,6 +2247,10 @@ class P4Sync(Command, P4UserMap):
if marshalled["code"] == "error":
if "data" in marshalled:
err = marshalled["data"].rstrip()
+
+if "depotFile" in marshalled:
+marshalled['depotFile'] = self.fixPathCase(marshalled['depotFile'])
+
if err:
f = None
if self.stream_have_file_info:
@@ -2314,6 +2325,7 @@ class P4Sync(Command, P4UserMap):
# do the last chunk
if self.stream_file.has_key('depotFile'):
+self.stream_file['depotFile'] =
self.fixPathCase(self.stream_file['depotFile'])
self.streamOneP4File(self.stream_file, self.stream_contents)
def make_email(self, userid):
@@ -2371,7 +2383,8 @@ class P4Sync(Command, P4UserMap):
sys.stderr.write("Ignoring file outside of prefix: %s\n" %
f['path'])
if self.clientSpecDirs:
-self.clientS
Re: [PATCH] Documentation/config: fix inconsistent label
Andreas Schwab writes: > Change to in the description of > gc.*.reflogExpireUnreachable, since that is what the text refers to. > > Signed-off-by: Andreas Schwab > --- Makes sense. Thanks. > Documentation/config.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 75ec02e..b4f7c46 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1329,7 +1329,7 @@ gc..reflogExpire:: > the refs that match the . > > gc.reflogExpireUnreachable:: > -gc..reflogExpireUnreachable:: > +gc..reflogExpireUnreachable:: > 'git reflog expire' removes reflog entries older than > this time and are not reachable from the current tip; > defaults to 30 days. The value "now" expires all entries > -- > 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
On Fri, 21 Aug 2015, Jeff King wrote: > - we may still have the opposite problem with renames. That is, a > rename is _also_ a deletion, but will go to the end. So I would > expect renaming the symlink "foo" to "bar" and then adding > "foo/world" would end up with: > >M 100644 :3 foo/world >R foo bar > > (because we push renames to the end in our sort). And indeed, > importing that does seem to get it wrong (we end up with "bar/world" > and no symlink). > > We can't fix the ordering in the second case without breaking the first > case. So I'm not sure it's fixable on the fast-export end. Hmm, renames have a more fundamental ordering problem: swapping two (normal) files and using fast-export -C -B results in R foo bar R bar foo which cannot be reimported correctly without fast-import fixes. Anders -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] am: rename "struct tree_desc t" to "desc" for readability
David Aguilar writes: > Signed-off-by: David Aguilar > --- > builtin/am.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Naming the tree_desc parameter given to unpack_trees() as "t" is (unfortunately) an established convention, just like we often use "i" and "j" in for() loop control. Descriptive names are nicer, especially on the calling sites, but we have quite a many hits from $ git grep unpack_trees\( \*.c so perhaps do this as a clean-up patch after dust settles for all the existing callers in archive, clone, commit, merge, read-tree, reset, diff-lib, and merge-recursive? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] am: do not corrupt the index stat state
David Aguilar writes:
> Reported-by: Linus Torvalds
> Signed-off-by: David Aguilar
> @@ -1975,10 +1977,20 @@ static int clean_index(const unsigned char *head,
> const unsigned char *remote)
>
> memset(&pathspec, 0, sizeof(pathspec));
>
> + memset(&opts, 0, sizeof(opts));
> + opts.fn = oneway_merge;
> + opts.pathspec = &pathspec;
> + opts.src_index = &the_index;
> + opts.dst_index = &the_index;
> + opts.head_idx = 1;
> + opts.merge = 1;
> + opts.reset = 1;
> + init_tree_desc(&desc, remote_tree->buffer, remote_tree->size);
> +
> lock_file = xcalloc(1, sizeof(struct lock_file));
> hold_locked_index(lock_file, 1);
>
> - if (read_tree(remote_tree, 0, &pathspec)) {
> + if (unpack_trees(1, &desc, &opts)) {
I think the same fix was done in 3ecc7040 (am --skip/--abort: merge
HEAD/ORIG_HEAD tree into index, 2015-08-19).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] describe --contains: default to HEAD when no commit-ish is given
SZEDER Gábor writes:
> 'git describe --contains' doesn't default to HEAD when no commit is
> given, and it doesn't produce any output, not even an error:
>
> ~/src/git ((v2.5.0))$ ./git describe --contains
> ~/src/git ((v2.5.0))$ ./git describe --contains HEAD
> v2.5.0^0
Good spotting. I think defaulting to HEAD is a good move.
> diff --git a/builtin/describe.c b/builtin/describe.c
> index ce36032b1f..0e31ac5cb9 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv, const
> char *prefix)
> if (pattern)
> argv_array_pushf(&args, "--refs=refs/tags/%s",
> pattern);
> }
> - while (*argv) {
> - argv_array_push(&args, *argv);
> - argv++;
> - }
> + if (argc)
"What would this code do to 'describe --all --contains'?" was my
knee-jerk reaction, but the options are all parsed by this code and
nothing is delegated to name-rev, so 'if (!argc)' here is truly the
lack of any revisions to be described, which is good.
> + while (*argv) {
> + argv_array_push(&args, *argv);
> + argv++;
> + }
> + else
> + argv_array_push(&args, "HEAD");
By the way, I usually prefer a fatter 'else' clause when everything
else is equal, i.e.
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD */
else {
while (*argv) {
...
}
}
because it is easy to miss tiny else-clause while reading code, but
it is harder to miss tiny then-clause. In this case, however, the
while loop can be replaced with argv_array_pushv() these days, so
perhaps
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD ... */
else
argv_array_pushv(&args, argv); /* or relay what we got */
or something?
> return cmd_name_rev(args.argc, args.argv, prefix);
> }
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 57d50156bb..bf52a0a1cc 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
>
> check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
>
> +test_expect_success 'describe --contains defaults to HEAD without
> commit-ish' '
> + echo "A^0" >expect &&
> + git checkout A &&
> + test_when_finished "git checkout -" &&
> + git describe --contains >actual &&
> + test_cmp expect actual
> +'
> : >err.expect
> check_describe A --all A^0
> test_expect_success 'no warning was displayed for A' '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Minor bug with help.autocorrect.
On Fri, Aug 21, 2015 at 09:10:35AM -0700, Junio C Hamano wrote:
> > $ git \#fetch
> > error: invalid key: pager.#fetch
> > error: invalid key: alias.#fetch
> > git: '#fetch' is not a git command. See 'git --help'.
> >
> > Did you mean this?
> > fetch
>
> Thanks. I somehow thought that we had some discussion on this
> earlier, perhaps
>
> http://thread.gmane.org/gmane.comp.version-control.git/263416/focus=263438
>
> Peff, do you remember what (if anything) we decided to do about
> this? I unfortunately don't X-<.
I think the plan is:
1. squelch the warning message from the config code; even if we change
the config format to pager.*.command, we will have to support
pager.* for historical reasons.
2. introduce pager.*.command so that "git foo_bar" can use
pager.foo_bar.command.
We should do (1) in the near-term. We do not have to do (2) at all (and
people with funny command names are simply out of luck), but it would be
nice in the long run.
The patch from Tanay in $gmane/263888 accomplishes (1), but there was a
minor cleanup needed (checking the individual bit in "flags", rather
than the whole variable). Here it is with that fix:
-- >8 --
From: Tanay Abhra
Subject: [PATCH] add a flag to supress errors in git_config_parse_key()
`git_config_parse_key()` is used to sanitize the input key.
Some callers of the function like `git_config_set_multivar_in_file()`
get the pre-sanitized key directly from the user so it becomes
necessary to raise an error specifying what went wrong when the entered
key is syntactically malformed.
Other callers like `configset_find_element()` get their keys from git
itself so a return value signifying error would be enough. The error
output shown to the user is useless and confusing in this case, so add a
flag to suppress errors in such cases.
Helped-by: Junio C Hamano
Helped-by: Jeff King
Signed-off-by: Tanay Abhra
Signed-off-by: Jeff King
---
builtin/config.c | 2 +-
cache.h | 4 +++-
config.c | 21 ++---
t/t7006-pager.sh | 9 +
4 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..f6cfb10 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -200,7 +200,7 @@ static int get_value(const char *key_, const char *regex_)
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
+ if (git_config_parse_key(key_, &key, NULL, 0)) {
ret = CONFIG_INVALID_KEY;
goto free_strings;
}
diff --git a/cache.h b/cache.h
index 4e25271..4b95d7e 100644
--- a/cache.h
+++ b/cache.h
@@ -1410,6 +1410,8 @@ extern int update_server_info(int);
#define CONFIG_REGEX_NONE ((void *)1)
+#define CONFIG_ERROR_QUIET 0x0001
+
struct git_config_source {
unsigned int use_stdin:1;
const char *file;
@@ -1439,7 +1441,7 @@ extern int git_config_string(const char **, const char *,
const char *);
extern int git_config_pathname(const char **, const char *, const char *);
extern int git_config_set_in_file(const char *, const char *, const char *);
extern int git_config_set(const char *, const char *);
-extern int git_config_parse_key(const char *, char **, int *);
+extern int git_config_parse_key(const char *, char **, int *, unsigned int);
extern int git_config_set_multivar(const char *, const char *, const char *,
int);
extern int git_config_set_multivar_in_file(const char *, const char *, const
char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 9fd275f..dd0cb52 100644
--- a/config.c
+++ b/config.c
@@ -1314,7 +1314,7 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, &normalized_key, NULL);
+ ret = git_config_parse_key(key, &normalized_key, NULL,
CONFIG_ERROR_QUIET);
if (ret)
return NULL;
@@ -1847,11 +1847,14 @@ int git_config_set(const char *key, const char *value)
* lowercase section and variable name
* baselen - pointer to int which will hold the length of the
* section + subsection part, can be NULL
+ * flags - we respect CONFIG_ERROR_QUIET to toggle whether the function raises
+ * an error on a syntactically malformed key
*/
-int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+int git_config_parse_key(const char *key, char **store_key, int *baselen_,
unsigned int flags)
{
int i, dot, baselen;
const char *last_dot = strrchr(key, '.');
+ int quiet = flags & CONFIG_ERROR_QUIET;
/*
* Since "key" actually contains the section name and the real
@@ -1859,12 +1862,14 @@ in
Re: Minor bug with help.autocorrect.
Bjørnar Snoksrud writes: > If you mis-type a git command starting with a non-letter, git > internals will spit out some errors at you. > > $ git 5fetch > error: invalid key: pager.5fetch > error: invalid key: alias.5fetch > git: '5fetch' is not a git command. See 'git --help'. > > Did you mean this? > fetch > > $ git \#fetch > error: invalid key: pager.#fetch > error: invalid key: alias.#fetch > git: '#fetch' is not a git command. See 'git --help'. > > Did you mean this? > fetch Thanks. I somehow thought that we had some discussion on this earlier, perhaps http://thread.gmane.org/gmane.comp.version-control.git/263416/focus=263438 Peff, do you remember what (if anything) we decided to do about this? I unfortunately don't X-<. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] describe: make '--always' fallback work after '--exact-match' failed
SZEDER Gábor writes: > Quoting Junio C Hamano : > >> Well, that can be argued both ways. > ... > > 'git describe' errors out by default if it can't describe the given > commit. Yes. "describe" always fails when it cannot produce an acceptable description. And "--always" and "--exact-match" are two incompatible ways for users to change what is considered an acceptable description. - With "--exact-match" and "--always", the definition of "acceptable" changes: by default, only a name based on some anchor point is acceptable. - "--always" loosens the definition and also allows an abbreviated commit object name as acceptable. - "--exact-match" tightens the definition. In addition to "only a name based on some anchor point is acceptable", it requires that "based on" to be "0 distance from the anchor point". The help text says succinctly: "*ONLY* output exact matches". If you allow a request with "--exact-match" to show something that is not an exact match, that no longer is "--exact-match". Allowing to mix "--always" to that option breaks it, as what the command does no longer is to "only outputs exact matches". - One option "--exact-match" says that it is wrong to show anything but exact matches. - The other option "--always" says it is willing to show a name that is not an exact match. These are competing goals. You give preference to the latter, but that is not the only valid point of view. And that is why I said it can be argued both ways. I think these two are fundamentally incompatible. [Footnote] *1* Other parts of "'describe' only considers a name based on some anchor point as acceptable." are also modified by various options: - With "--contains", the definition of "based on" changes direction: a commit may be described as somewhere ahead of an anchor point by default, but can be described as somewhere behind of an anchor point. - With "--all" and "--tags", the definition of "anchor point" changes: by default, only annotated tags are possible anchor points. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/config: fix inconsistent label
Change to in the description of gc.*.reflogExpireUnreachable, since that is what the text refers to. Signed-off-by: Andreas Schwab --- Documentation/config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 75ec02e..b4f7c46 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1329,7 +1329,7 @@ gc..reflogExpire:: the refs that match the . gc.reflogExpireUnreachable:: -gc..reflogExpireUnreachable:: +gc..reflogExpireUnreachable:: 'git reflog expire' removes reflog entries older than this time and are not reachable from the current tip; defaults to 30 days. The value "now" expires all entries -- 2.5.0 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/9] builtin/send-pack.c: Use option parsing API
On Wed, Aug 19, 2015 at 03:46:25PM -0400, Dave Borowitz wrote: > >> + unsigned dry_run = 0; > >> + unsigned send_mirror = 0; > >> + unsigned force_update = 0; > >> + unsigned quiet = 0; > >> + unsigned push_cert = 0; > >> + unsigned use_thin_pack = 0; > >> + unsigned atomic = 0; > >> + unsigned stateless_rpc = 0; > > > > First I thought: > > You could write to the args flags directly from the options. No > > need to have (most of) > > the variables around here and copy over the values. You'd need to > > use OPT_BIT instead > > for setting a specific bit though > > but then I realized we do not have a direct bit field in args, which > > would make it a bit unreadable. > > Right, and &args->push_cert etc. is invalid, and I didn't know if it > was ok to expand the args struct to be several words longer. But I'm > not a C programmer so I'm happy to take suggestions how to make this > more idiomatic. I think it would be fine to expand it. The reason to use bitfields is to save memory, and there is literally only one of these structs per program. I'm sure we can afford the extra dozen bytes. Making the struct members single-bits also communicates to readers that they are true booleans, but I think a comment in the declaration of send_pack_args could do the same. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG/PATCH] t9350-fast-export: Add failing test for symlink-to-directory
On Wed, Aug 19, 2015 at 03:46:27PM -0400, Anders Kaseorg wrote: > git fast-export | git fast-import fails to preserve a commit that replaces > a symlink with a directory. Add a failing test case demonstrating this > bug. > > The fast-export output for the commit in question looks like > > commit refs/heads/master > mark :4 > author … > committer … > data 4 > two > M 100644 :1 foo/world > D foo > > fast-import deletes the symlink foo and ignores foo/world. Swapping the M > line with the D line would give the correct result. Thanks for providing a patch to the tests. That is my favorite form of bug report. :) The problem seems to be that we output the entries in a "depth first" way; "foo/bar" always comes before "foo", to cover the cases explained in 060df62 (fast-export: Fix output order of D/F changes, 2010-07-09). I'm tempted to say we would want to do all deletions (at any level) first, to make room for new files. That patch looks like: diff --git a/builtin/fast-export.c b/builtin/fast-export.c index d23f3be..336fd6f 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -267,6 +267,14 @@ static int depth_first(const void *a_, const void *b_) const char *name_a, *name_b; int len_a, len_b, len; int cmp; + int deletion; + + /* +* Move all deletions first, to make room for any later modifications. +*/ + deletion = (b->status == 'D') - (a->status == 'D'); + if (deletion) + return deletion; name_a = a->one ? a->one->path : a->two->path; name_b = b->one ? b->one->path : b->two->path; and does indeed pass your test. But I'm not sure that covers all cases, and I'm not sure it doesn't make some cases worse: - if we moved a deletion to the front, is it possible for that path to have been the source side of a copy or rename, that is now broken? I don't _think_ so. If it's a copy, then the file by definition cannot also be deleted (that would make it a rename, not a copy). We could have a copy along with a rename, but again, then we don't have a delete (we have a rename, which is explicitly bumped to the end for this reason). - we may still have the opposite problem with renames. That is, a rename is _also_ a deletion, but will go to the end. So I would expect renaming the symlink "foo" to "bar" and then adding "foo/world" would end up with: M 100644 :3 foo/world R foo bar (because we push renames to the end in our sort). And indeed, importing that does seem to get it wrong (we end up with "bar/world" and no symlink). We can't fix the ordering in the second case without breaking the first case. So I'm not sure it's fixable on the fast-export end. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git log date filter behavior with --date-order
Hi, I’ve faced a strange behavior when filtering git log by date. When used with “--date-order” it gives significantly less results that when used without that option: # git log --pretty=oneline --remotes "--after=Wed May 20 23:00:00 MSK 2015" "--before=Fri May 22 00:00:00 MSK 2015" -- | wc -l 85 # git log --pretty=oneline --remotes "--after=Wed May 20 23:00:00 MSK 2015" "--before=Fri May 22 00:00:00 MSK 2015" --date-order -- | wc -l 25 My assumption is that git log walks from the specified heads down to the past until it finds a commit which commit date matches the filter. Then it continues until it finds a commit which commit date doesn’t match the filter. Is it true? Commits from “matching” and “unmatching” parts of the output are located in different branches which are being developed in parallel. AFAIU the flag “--date-order” is meant to place commits from different branches near each other, if they were performed at the same time. But when I looked both at the “matching" and the “unmatching” commits in the whole `git log --date-order` view, I’ve noticed that they are placed really far away from each other (with about 4K commits between them). Could you please give me a hint, why can git log --date-order behave like that? I’ll be happy to provide more details if needed. Thanks a lot! Kirill.-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] describe --contains: default to HEAD when no commit-ish is given
'git describe --contains' doesn't default to HEAD when no commit is
given, and it doesn't produce any output, not even an error:
~/src/git ((v2.5.0))$ ./git describe --contains
~/src/git ((v2.5.0))$ ./git describe --contains HEAD
v2.5.0^0
Unlike other 'git describe' options, the '--contains' code path is
implemented by calling 'name-rev' with a bunch of options plus all the
commit-ishes that were passed to 'git describe'. If no commit-ish was
present, then 'name-rev' got invoked with none, which then leads to the
behavior illustrated above.
Porcelain commands usually default to HEAD when no commit-ish is given,
and 'git describe' already does so in all other cases, so it should do
so with '--contains' as well.
Pass HEAD to 'name-rev' when no commit-ish is given on the command line
to make '--contains' behave consistently with other 'git describe'
options.
'git describe's short help already indicates that the commit-ish is
optional, but the synopsis in the man page doesn't, so update it
accordingly as well.
Signed-off-by: SZEDER Gábor
---
Documentation/git-describe.txt | 4 ++--
builtin/describe.c | 11 +++
t/t6120-describe.sh| 8
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e045fc73f8..c8f28c8c86 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -9,7 +9,7 @@ git-describe - Describe a commit using the most recent tag
reachable from it
SYNOPSIS
[verse]
-'git describe' [--all] [--tags] [--contains] [--abbrev=] ...
+'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]
'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]
DESCRIPTION
@@ -27,7 +27,7 @@ see the -a and -s options to linkgit:git-tag[1].
OPTIONS
---
...::
- Commit-ish object names to describe.
+ Commit-ish object names to describe. Defaults to HEAD if omitted.
--dirty[=]::
Describe the working tree.
diff --git a/builtin/describe.c b/builtin/describe.c
index ce36032b1f..0e31ac5cb9 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv, const char
*prefix)
if (pattern)
argv_array_pushf(&args, "--refs=refs/tags/%s",
pattern);
}
- while (*argv) {
- argv_array_push(&args, *argv);
- argv++;
- }
+ if (argc)
+ while (*argv) {
+ argv_array_push(&args, *argv);
+ argv++;
+ }
+ else
+ argv_array_push(&args, "HEAD");
return cmd_name_rev(args.argc, args.argv, prefix);
}
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 57d50156bb..bf52a0a1cc 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
+test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
+ echo "A^0" >expect &&
+ git checkout A &&
+ test_when_finished "git checkout -" &&
+ git describe --contains >actual &&
+ test_cmp expect actual
+'
+
: >err.expect
check_describe A --all A^0
test_expect_success 'no warning was displayed for A' '
--
2.5.0.416.g84b07b4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper
On Mon, Aug 17, 2015 at 05:22:03PM -0700, Stefan Beller wrote:
> +static int module_clone(int argc, const char **argv, const char *prefix)
> [...]
> + /* Redirect the worktree of the submodule in the superprojects config */
> + if (!is_absolute_path(sm_gitdir)) {
> + char *s = (char*)sm_gitdir;
> + strbuf_addf(&sb, "%s/%s", xgetcwd(), sm_gitdir);
Coverity noticed that this is a leak; xgetcwd actually returns a newly
allocated buffer. I think you just want:
if (strbuf_getcwd(&sb))
die_errno("unable to get current working directory");
strbuf_addf("/%s", sm_gitdir);
> + sm_gitdir = strbuf_detach(&sb, NULL);
> + strbuf_reset(&sb);
This reset is a noop after you've detached. It's not technically
documented, but I do not think it is reasonable for it to work any other
way. Maybe strbuf.h should make that promise.
> + strbuf_addf(&sb, "%s/%s", xgetcwd(), path);
Ditto on the xgetcwd issue here.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/10] grep/pcre: support utf-8
In the previous change in this function, we add locale support for single-byte encodings only. It looks like pcre only supports utf-* as multibyte encodings, the others are left in the cold (which is fine). We need to enable PCRE_UTF8 so pcre can find character boundary correctly. It's needed for case folding (when --ignore-case is used) or '*', '+' or similar syntax is used. The "has_non_ascii()" check is to be on the conservative side. If there's non-ascii in the pattern, the searched content could still be in utf-8, but we can treat it just like a byte stream and everything should work. If we force utf-8 based on locale only and pcre validates utf-8 and the file content is in non-utf8 encoding, things break. Noticed-by: Plamen Totev Helped-by: Plamen Totev Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 2 ++ t/t7812-grep-icase-non-ascii.sh | 15 +++ 2 files changed, 17 insertions(+) diff --git a/grep.c b/grep.c index f0fbf99..07621c1 100644 --- a/grep.c +++ b/grep.c @@ -329,6 +329,8 @@ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt) p->pcre_tables = pcre_maketables(); options |= PCRE_CASELESS; } + if (is_utf8_locale() && has_non_ascii(p->pattern)) + options |= PCRE_UTF8; p->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset, p->pcre_tables); diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh index aba6b15..8896410 100755 --- a/t/t7812-grep-icase-non-ascii.sh +++ b/t/t7812-grep-icase-non-ascii.sh @@ -20,6 +20,21 @@ test_expect_success REGEX_LOCALE 'grep literal string, no -F' ' git grep -i "TILRAUN: HALLÓ HEIMUR!" ' +test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 icase' ' + git grep --perl-regexp"TILRAUN: H.lló Heimur!" && + git grep --perl-regexp -i "TILRAUN: H.lló Heimur!" && + git grep --perl-regexp -i "TILRAUN: H.LLÓ HEIMUR!" +' + +test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 string with "+"' ' + printf "TILRAUN: Hallóó Heimur!" >file2 && + git add file2 && + git grep -l --perl-regexp "TILRAUN: H.lló+ Heimur!" >actual && + echo file >expected && + echo file2 >>expected && + test_cmp expected actual +' + test_expect_success REGEX_LOCALE 'grep literal string, with -F' ' git grep --debug -i -F "TILRAUN: Halló Heimur!" 2>&1 >/dev/null | grep fixed >debug1 && -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/10] diffcore-pickaxe: support case insensitive match on non-ascii
Similar to the "grep -F -i" case, we can't use kws on icase search
outside ascii range, so we quote the string and pass it to regcomp as
a basic regexp and let regex engine deal with case sensitivity.
The new test is put in t7812 instead of t4209-log-pickaxe because
lib-gettext.sh might cause problems elsewhere, probably..
Noticed-by: Plamen Totev
Signed-off-by: Nguyễn Thái Ngọc Duy
---
diffcore-pickaxe.c | 11 +++
t/t7812-grep-icase-non-ascii.sh | 7 +++
2 files changed, 18 insertions(+)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 7a718fc..6946c15 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -7,6 +7,8 @@
#include "diffcore.h"
#include "xdiff-interface.h"
#include "kwset.h"
+#include "commit.h"
+#include "quote.h"
typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
@@ -212,6 +214,15 @@ void diffcore_pickaxe(struct diff_options *o)
cflags |= REG_ICASE;
err = regcomp(®ex, needle, cflags);
regexp = ®ex;
+ } else if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) &&
+ has_non_ascii(needle)) {
+ struct strbuf sb = STRBUF_INIT;
+ int cflags = REG_NEWLINE | REG_ICASE;
+
+ basic_regex_quote_buf(&sb, needle);
+ err = regcomp(®ex, sb.buf, cflags);
+ strbuf_release(&sb);
+ regexp = ®ex;
} else {
kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
? tolower_trans_tbl : NULL);
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index 8896410..a5475bb 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -61,4 +61,11 @@ test_expect_success REGEX_LOCALE 'grep string with regex,
with -F' '
test_cmp expect2 debug2
'
+test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '
+ git commit -m first &&
+ git log --format=%f -i -S"TILRAUN: HALLÓ HEIMUR!" >actual &&
+ echo first >expected &&
+ test_cmp expected actual
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/10] diffcore-pickaxe: "share" regex error handling code
There's another regcomp code block coming in this function. By moving
the error handling code out of this block, we don't have to add the
same error handling code in the new block.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
diffcore-pickaxe.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 185f86b..7a718fc 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -204,20 +204,13 @@ void diffcore_pickaxe(struct diff_options *o)
int opts = o->pickaxe_opts;
regex_t regex, *regexp = NULL;
kwset_t kws = NULL;
+ int err = 0;
if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
- int err;
int cflags = REG_EXTENDED | REG_NEWLINE;
if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE))
cflags |= REG_ICASE;
err = regcomp(®ex, needle, cflags);
- if (err) {
- /* The POSIX.2 people are surely sick */
- char errbuf[1024];
- regerror(err, ®ex, errbuf, 1024);
- regfree(®ex);
- die("invalid regex: %s", errbuf);
- }
regexp = ®ex;
} else {
kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)
@@ -225,6 +218,13 @@ void diffcore_pickaxe(struct diff_options *o)
kwsincr(kws, needle, strlen(needle));
kwsprep(kws);
}
+ if (err) {
+ /* The POSIX.2 people are surely sick */
+ char errbuf[1024];
+ regerror(err, ®ex, errbuf, 1024);
+ regfree(®ex);
+ die("invalid regex: %s", errbuf);
+ }
/* Might want to warn when both S and G are on; I don't care... */
pickaxe(&diff_queued_diff, o, regexp, kws,
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/10] gettext: add is_utf8_locale()
This function returns true if git is running under an UTF-8
locale. pcre in the next patch will need this.
is_encoding_utf8() is used instead of strcmp() to catch both "utf-8"
and "utf8" suffixes.
When built with no gettext support, we peek in several env variables
to detect UTF-8. pcre library might support utf-8 even if libc is
built without locale support.. The peeking code is a copy from
compat/regex/regcomp.c
Helped-by: Ævar Arnfjörð Bjarmason
Signed-off-by: Nguyễn Thái Ngọc Duy
---
gettext.c | 24 ++--
gettext.h | 1 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/gettext.c b/gettext.c
index a268a2c..db727ea 100644
--- a/gettext.c
+++ b/gettext.c
@@ -18,6 +18,8 @@
# endif
#endif
+static const char *charset;
+
/*
* Guess the user's preferred languages from the value in LANGUAGE environment
* variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
@@ -65,7 +67,6 @@ static int test_vsnprintf(const char *fmt, ...)
return ret;
}
-static const char *charset;
static void init_gettext_charset(const char *domain)
{
/*
@@ -172,8 +173,27 @@ int gettext_width(const char *s)
{
static int is_utf8 = -1;
if (is_utf8 == -1)
- is_utf8 = !strcmp(charset, "UTF-8");
+ is_utf8 = is_utf8_locale();
return is_utf8 ? utf8_strwidth(s) : strlen(s);
}
#endif
+
+int is_utf8_locale(void)
+{
+#ifdef NO_GETTEXT
+ if (!charset) {
+ const char *env = getenv("LC_ALL");
+ if (!env || !*env)
+ env = getenv("LC_CTYPE");
+ if (!env || !*env)
+ env = getenv("LANG");
+ if (!env)
+ env = "";
+ if (strchr(env, '.'))
+ env = strchr(env, '.') + 1;
+ charset = xstrdup(env);
+ }
+#endif
+ return is_encoding_utf8(charset);
+}
diff --git a/gettext.h b/gettext.h
index 33696a4..7eee64a 100644
--- a/gettext.h
+++ b/gettext.h
@@ -90,5 +90,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned
long n)
#endif
const char *get_preferred_languages(void);
+extern int is_utf8_locale(void);
#endif
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/10] grep/pcre: prepare locale-dependent tables for icase matching
The default tables are usually built with C locale and only suitable
for LANG=C or similar. This should make case insensitive search work
correctly for all single-byte charsets.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
grep.c | 8 ++--
grep.h | 1 +
t/t7813-grep-icase-iso.sh (new +x) | 19 +++
3 files changed, 26 insertions(+), 2 deletions(-)
create mode 100755 t/t7813-grep-icase-iso.sh
diff --git a/grep.c b/grep.c
index 8fce54f..f0fbf99 100644
--- a/grep.c
+++ b/grep.c
@@ -324,11 +324,14 @@ static void compile_pcre_regexp(struct grep_pat *p, const
struct grep_opt *opt)
int erroffset;
int options = PCRE_MULTILINE;
- if (opt->ignore_case)
+ if (opt->ignore_case) {
+ if (has_non_ascii(p->pattern))
+ p->pcre_tables = pcre_maketables();
options |= PCRE_CASELESS;
+ }
p->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset,
- NULL);
+ p->pcre_tables);
if (!p->pcre_regexp)
compile_regexp_failed(p, error);
@@ -362,6 +365,7 @@ static void free_pcre_regexp(struct grep_pat *p)
{
pcre_free(p->pcre_regexp);
pcre_free(p->pcre_extra_info);
+ pcre_free((void *)p->pcre_tables);
}
#else /* !USE_LIBPCRE */
static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt)
diff --git a/grep.h b/grep.h
index 95f197a..cee4357 100644
--- a/grep.h
+++ b/grep.h
@@ -48,6 +48,7 @@ struct grep_pat {
regex_t regexp;
pcre *pcre_regexp;
pcre_extra *pcre_extra_info;
+ const unsigned char *pcre_tables;
kwset_t kws;
unsigned fixed:1;
unsigned ignore_case:1;
diff --git a/t/t7813-grep-icase-iso.sh b/t/t7813-grep-icase-iso.sh
new file mode 100755
index 000..efef7fb
--- /dev/null
+++ b/t/t7813-grep-icase-iso.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+test_description='grep icase on non-English locales'
+
+. ./lib-gettext.sh
+
+test_expect_success GETTEXT_ISO_LOCALE 'setup' '
+ printf "TILRAUN: Hall� Heimur!" >file &&
+ git add file &&
+ LC_ALL="$is_IS_iso_locale" &&
+ export LC_ALL
+'
+
+test_expect_success GETTEXT_ISO_LOCALE,LIBPCRE 'grep pcre string' '
+ git grep --perl-regexp -i "TILRAUN: H.ll� Heimur!" &&
+ git grep --perl-regexp -i "TILRAUN: H.LL� HEIMUR!"
+'
+
+test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 05/10] grep/icase: avoid kwsset when -F is specified
Similar to the previous commit, we can't use kws on icase search
outside ascii range. But we can't simply pass the pattern to
regcomp/pcre like the previous commit because it may contain regex
special characters, so we need to quote the regex first.
To avoid misquote traps that could lead to undefined behavior, we
always stick to basic regex engine in this case. We don't need fancy
features for grepping a literal string anyway.
basic_regex_quote_buf() assumes that if the pattern is in a multibyte
encoding, ascii chars must be unambiguously encoded as single
bytes. This is true at least for UTF-8. For others, let's wait until
people yell up. Chances are nobody uses multibyte, non utf-8 charsets
any more..
Helped-by: René Scharfe
Noticed-by: Plamen Totev
Signed-off-by: Nguyễn Thái Ngọc Duy
---
grep.c | 25 -
quote.c | 37 +
quote.h | 1 +
t/t7812-grep-icase-non-ascii.sh | 26 ++
4 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/grep.c b/grep.c
index d795b0e..8fce54f 100644
--- a/grep.c
+++ b/grep.c
@@ -5,6 +5,7 @@
#include "diff.h"
#include "diffcore.h"
#include "commit.h"
+#include "quote.h"
static int grep_source_load(struct grep_source *gs);
static int grep_source_is_binary(struct grep_source *gs);
@@ -397,6 +398,24 @@ static int is_fixed(const char *s, size_t len)
return 1;
}
+static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int err;
+
+ basic_regex_quote_buf(&sb, p->pattern);
+ err = regcomp(&p->regexp, sb.buf, opt->regflags & ~REG_EXTENDED);
+ if (opt->debug)
+ fprintf(stderr, "fixed%s\n", sb.buf);
+ strbuf_release(&sb);
+ if (err) {
+ char errbuf[1024];
+ regerror(err, &p->regexp, errbuf, 1024);
+ regfree(&p->regexp);
+ compile_regexp_failed(p, errbuf);
+ }
+}
+
static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
{
int icase_non_ascii;
@@ -411,7 +430,11 @@ static void compile_regexp(struct grep_pat *p, struct
grep_opt *opt)
if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen))
p->fixed = 1;
else if (opt->fixed) {
- p->fixed = 1;
+ p->fixed = !icase_non_ascii;
+ if (!p->fixed) {
+ compile_fixed_regexp(p, opt);
+ return;
+ }
} else
p->fixed = 0;
diff --git a/quote.c b/quote.c
index 7920e18..43a8057 100644
--- a/quote.c
+++ b/quote.c
@@ -439,3 +439,40 @@ void tcl_quote_buf(struct strbuf *sb, const char *src)
}
strbuf_addch(sb, '"');
}
+
+void basic_regex_quote_buf(struct strbuf *sb, const char *src)
+{
+ char c;
+
+ if (*src == '^') {
+ /* only beginning '^' is special and needs quoting */
+ strbuf_addch(sb, '\\');
+ strbuf_addch(sb, *src++);
+ }
+ if (*src == '*')
+ /* beginning '*' is not special, no quoting */
+ strbuf_addch(sb, *src++);
+
+ while ((c = *src++)) {
+ switch (c) {
+ case '[':
+ case '.':
+ case '\\':
+ case '*':
+ strbuf_addch(sb, '\\');
+ strbuf_addch(sb, c);
+ break;
+
+ case '$':
+ /* only the end '$' is special and needs quoting */
+ if (*src == '\0')
+ strbuf_addch(sb, '\\');
+ strbuf_addch(sb, c);
+ break;
+
+ default:
+ strbuf_addch(sb, c);
+ break;
+ }
+ }
+}
diff --git a/quote.h b/quote.h
index 99e04d3..362d315 100644
--- a/quote.h
+++ b/quote.h
@@ -67,5 +67,6 @@ extern char *quote_path_relative(const char *in, const char
*prefix,
extern void perl_quote_buf(struct strbuf *sb, const char *src);
extern void python_quote_buf(struct strbuf *sb, const char *src);
extern void tcl_quote_buf(struct strbuf *sb, const char *src);
+extern void basic_regex_quote_buf(struct strbuf *sb, const char *src);
#endif
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index 6eff490..aba6b15 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -20,4 +20,30 @@ test_expect_success REGEX_LOCALE 'grep literal string, no
-F' '
git grep -i "TILRAUN: HALLÓ HEIMUR!"
'
+test_expect_success REGEX_LOCALE 'grep literal string, with -F' '
+ git grep --debug -i -F "TILRAUN: Halló Heimur!" 2>&1 >/dev/null |
+grep fixed >debug1 &&
+ echo "fixedTILRAUN: Halló Heimur!" >expect1 &&
+ test_c
[PATCH v4 00/10] icase match on non-ascii
Sorry it took more than a month for a simple reroll. Free time (with
energy left) is rare these days. v4 adds system regex's icase support
detection and only runs tests in these cases. This should fix test
failures on Windows where compat regex does not support icase.
Diff from v3 below
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index d07fa20..a5475bb 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -11,7 +11,11 @@ test_expect_success GETTEXT_LOCALE 'setup' '
export LC_ALL
'
-test_expect_success GETTEXT_LOCALE 'grep literal string, no -F' '
+test_have_prereq GETTEXT_LOCALE &&
+test-regex "HALLÓ" "Halló" ICASE &&
+test_set_prereq REGEX_LOCALE
+
+test_expect_success REGEX_LOCALE 'grep literal string, no -F' '
git grep -i "TILRAUN: Halló Heimur!" &&
git grep -i "TILRAUN: HALLÓ HEIMUR!"
'
@@ -31,7 +35,7 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8
string with "+"' '
test_cmp expected actual
'
-test_expect_success GETTEXT_LOCALE 'grep literal string, with -F' '
+test_expect_success REGEX_LOCALE 'grep literal string, with -F' '
git grep --debug -i -F "TILRAUN: Halló Heimur!" 2>&1 >/dev/null |
grep fixed >debug1 &&
echo "fixedTILRAUN: Halló Heimur!" >expect1 &&
@@ -43,7 +47,7 @@ test_expect_success GETTEXT_LOCALE 'grep literal string, with
-F' '
test_cmp expect2 debug2
'
-test_expect_success GETTEXT_LOCALE 'grep string with regex, with -F' '
+test_expect_success REGEX_LOCALE 'grep string with regex, with -F' '
printf "^*TILR^AUN:.* \\Halló \$He[]imur!\$" >file &&
git grep --debug -i -F "^*TILR^AUN:.* \\Halló \$He[]imur!\$" 2>&1
>/dev/null |
@@ -57,7 +61,7 @@ test_expect_success GETTEXT_LOCALE 'grep string with regex,
with -F' '
test_cmp expect2 debug2
'
-test_expect_success GETTEXT_LOCALE 'pickaxe -i on non-ascii' '
+test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '
git commit -m first &&
git log --format=%f -i -S"TILRAUN: HALLÓ HEIMUR!" >actual &&
echo first >expected &&
diff --git a/test-regex.c b/test-regex.c
index 0dc598e..3b5641c 100644
--- a/test-regex.c
+++ b/test-regex.c
@@ -1,19 +1,63 @@
#include "git-compat-util.h"
+#include "gettext.h"
+
+struct reg_flag {
+ const char *name;
+ int flag;
+};
+
+static struct reg_flag reg_flags[] = {
+ { "EXTENDED",REG_EXTENDED },
+ { "NEWLINE", REG_NEWLINE},
+ { "ICASE", REG_ICASE },
+ { "NOTBOL", REG_NOTBOL },
+#ifdef REG_STARTEND
+ { "STARTEND",REG_STARTEND },
+#endif
+ { NULL, 0 }
+};
int main(int argc, char **argv)
{
- char *pat = "[^={} \t]+";
- char *str = "={}\nfred";
+ const char *pat;
+ const char *str;
+ int flags = 0;
regex_t r;
regmatch_t m[1];
- if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
+ if (argc == 1) {
+ /* special case, bug check */
+ pat = "[^={} \t]+";
+ str = "={}\nfred";
+ flags = REG_EXTENDED | REG_NEWLINE;
+ } else {
+ argv++;
+ pat = *argv++;
+ str = *argv++;
+ while (*argv) {
+ struct reg_flag *rf;
+ for (rf = reg_flags; rf->name; rf++)
+ if (!strcmp(*argv, rf->name)) {
+ flags |= rf->flag;
+ break;
+ }
+ if (!rf->name)
+ die("do not recognize %s", *argv);
+ argv++;
+ }
+ git_setup_gettext();
+ }
+
+ if (regcomp(&r, pat, flags))
die("failed regcomp() for pattern '%s'", pat);
- if (regexec(&r, str, 1, m, 0))
- die("no match of pattern '%s' to string '%s'", pat, str);
+ if (regexec(&r, str, 1, m, 0)) {
+ if (argc == 1)
+ die("no match of pattern '%s' to string '%s'", pat,
str);
+ return 1;
+ }
/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */
- if (m[0].rm_so == 3) /* matches '\n' when it should not */
+ if (argc == 1 && m[0].rm_so == 3) /* matches '\n' when it should not */
die("regex bug confirmed: re-build git with NO_REGEX=1");
exit(0);
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/10] grep: allow -F -i combination
-F means "no regex", not "case sensitive" so it should not override -i
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..2d392e9 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -806,7 +806,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
if (!opt.pattern_list)
die(_("no pattern given."));
- if (!opt.fixed && opt.ignore_case)
+ if (opt.ignore_case)
opt.regflags |= REG_ICASE;
compile_grep_patterns(&opt);
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/10] test-regex: expose full regcomp() to the command line
Signed-off-by: Nguyễn Thái Ngọc Duy
---
test-regex.c | 56 ++--
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/test-regex.c b/test-regex.c
index 0dc598e..3b5641c 100644
--- a/test-regex.c
+++ b/test-regex.c
@@ -1,19 +1,63 @@
#include "git-compat-util.h"
+#include "gettext.h"
+
+struct reg_flag {
+ const char *name;
+ int flag;
+};
+
+static struct reg_flag reg_flags[] = {
+ { "EXTENDED",REG_EXTENDED },
+ { "NEWLINE", REG_NEWLINE},
+ { "ICASE", REG_ICASE },
+ { "NOTBOL", REG_NOTBOL },
+#ifdef REG_STARTEND
+ { "STARTEND",REG_STARTEND },
+#endif
+ { NULL, 0 }
+};
int main(int argc, char **argv)
{
- char *pat = "[^={} \t]+";
- char *str = "={}\nfred";
+ const char *pat;
+ const char *str;
+ int flags = 0;
regex_t r;
regmatch_t m[1];
- if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
+ if (argc == 1) {
+ /* special case, bug check */
+ pat = "[^={} \t]+";
+ str = "={}\nfred";
+ flags = REG_EXTENDED | REG_NEWLINE;
+ } else {
+ argv++;
+ pat = *argv++;
+ str = *argv++;
+ while (*argv) {
+ struct reg_flag *rf;
+ for (rf = reg_flags; rf->name; rf++)
+ if (!strcmp(*argv, rf->name)) {
+ flags |= rf->flag;
+ break;
+ }
+ if (!rf->name)
+ die("do not recognize %s", *argv);
+ argv++;
+ }
+ git_setup_gettext();
+ }
+
+ if (regcomp(&r, pat, flags))
die("failed regcomp() for pattern '%s'", pat);
- if (regexec(&r, str, 1, m, 0))
- die("no match of pattern '%s' to string '%s'", pat, str);
+ if (regexec(&r, str, 1, m, 0)) {
+ if (argc == 1)
+ die("no match of pattern '%s' to string '%s'", pat,
str);
+ return 1;
+ }
/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */
- if (m[0].rm_so == 3) /* matches '\n' when it should not */
+ if (argc == 1 && m[0].rm_so == 3) /* matches '\n' when it should not */
die("regex bug confirmed: re-build git with NO_REGEX=1");
exit(0);
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 04/10] grep/icase: avoid kwsset on literal non-ascii strings
When we detect the pattern is just a literal string, we avoid heavy
regex engine and use fast substring search implemented in kwsset.c.
But kws uses git-ctype which is locale-independent so it does not know
how to fold case properly outside ascii range. Let regcomp or pcre
take care of this case instead. Slower, but accurate.
Helped-by: René Scharfe
Noticed-by: Plamen Totev
Signed-off-by: Nguyễn Thái Ngọc Duy
---
grep.c | 7 ++-
t/t7812-grep-icase-non-ascii.sh (new +x) | 23 +++
2 files changed, 29 insertions(+), 1 deletion(-)
create mode 100755 t/t7812-grep-icase-non-ascii.sh
diff --git a/grep.c b/grep.c
index bd32f66..d795b0e 100644
--- a/grep.c
+++ b/grep.c
@@ -4,6 +4,7 @@
#include "xdiff-interface.h"
#include "diff.h"
#include "diffcore.h"
+#include "commit.h"
static int grep_source_load(struct grep_source *gs);
static int grep_source_is_binary(struct grep_source *gs);
@@ -398,12 +399,16 @@ static int is_fixed(const char *s, size_t len)
static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
{
+ int icase_non_ascii;
int err;
p->word_regexp = opt->word_regexp;
p->ignore_case = opt->ignore_case;
+ icase_non_ascii =
+ (opt->regflags & REG_ICASE || p->ignore_case) &&
+ has_non_ascii(p->pattern);
- if (is_fixed(p->pattern, p->patternlen))
+ if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen))
p->fixed = 1;
else if (opt->fixed) {
p->fixed = 1;
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
new file mode 100755
index 000..6eff490
--- /dev/null
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description='grep icase on non-English locales'
+
+. ./lib-gettext.sh
+
+test_expect_success GETTEXT_LOCALE 'setup' '
+ printf "TILRAUN: Halló Heimur!" >file &&
+ git add file &&
+ LC_ALL="$is_IS_locale" &&
+ export LC_ALL
+'
+
+test_have_prereq GETTEXT_LOCALE &&
+test-regex "HALLÓ" "Halló" ICASE &&
+test_set_prereq REGEX_LOCALE
+
+test_expect_success REGEX_LOCALE 'grep literal string, no -F' '
+ git grep -i "TILRAUN: Halló Heimur!" &&
+ git grep -i "TILRAUN: HALLÓ HEIMUR!"
+'
+
+test_done
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/10] grep: break down an "if" stmt in preparation for next changes
Signed-off-by: Nguyễn Thái Ngọc Duy
---
grep.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index b58c7c6..bd32f66 100644
--- a/grep.c
+++ b/grep.c
@@ -403,9 +403,11 @@ static void compile_regexp(struct grep_pat *p, struct
grep_opt *opt)
p->word_regexp = opt->word_regexp;
p->ignore_case = opt->ignore_case;
- if (opt->fixed || is_fixed(p->pattern, p->patternlen))
+ if (is_fixed(p->pattern, p->patternlen))
p->fixed = 1;
- else
+ else if (opt->fixed) {
+ p->fixed = 1;
+ } else
p->fixed = 0;
if (p->fixed) {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] format_config: simplify buffer handling
Quoting Jeff King :
When formatting a config value into a strbuf, we may end
up stringifying it into a fixed-size buffer using sprintf,
and then copying that buffer into the strbuf. We can
eliminate the middle-man (and drop some calls to sprintf!)
by writing directly to the strbuf.
The reason it was written this way in the first place is
that we need to know before writing the value whether to
insert a delimiter. Instead of delaying the write of the
value, we speculatively write the delimiter, and roll it
back in the single case that cares.
Signed-off-by: Jeff King
---
I admit the rollback is a little gross.
Indeed it is, but I'm for it, as it gets rit of so much more
other grossness, i.e. the fixed-size buffer and vptr stuff
and the two must_do_this variables.
builtin/config.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 91aa56f..04befce 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -111,41 +111,35 @@ static int format_config(struct strbuf *buf,
const char *key_, const char *value
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) {
- int must_free_vptr = 0;
- int must_add_delim = show_keys;
- char value[256];
- const char *vptr = value;
+ if (show_keys)
+ strbuf_addch(buf, key_delim);
if (types == TYPE_INT)
- sprintf(value, "%"PRId64,
- git_config_int64(key_, value_ ? value_ : ""));
+ strbuf_addf(buf, "%"PRId64,
+ git_config_int64(key_, value_ ? value_ :
""));
else if (types == TYPE_BOOL)
- vptr = git_config_bool(key_, value_) ? "true" : "false";
+ strbuf_addstr(buf, git_config_bool(key_, value_) ?
+ "true" : "false");
else if (types == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, &is_bool);
if (is_bool)
- vptr = v ? "true" : "false";
+ strbuf_addstr(buf, v ? "true" : "false");
else
- sprintf(value, "%d", v);
+ strbuf_addf(buf, "%d", v);
} else if (types == TYPE_PATH) {
- if (git_config_pathname(&vptr, key_, value_) < 0)
+ const char *v;
+ if (git_config_pathname(&v, key_, value_) < 0)
return -1;
- must_free_vptr = 1;
+ strbuf_addstr(buf, v);
+ free((char *)v);
} else if (value_) {
- vptr = value_;
+ strbuf_addstr(buf, value_);
} else {
- /* Just show the key name */
- vptr = "";
- must_add_delim = 0;
+ /* Just show the key name; back out delimiter */
+ if (show_keys)
+ strbuf_setlen(buf, buf->len - 1);
}
-
- if (must_add_delim)
- strbuf_addch(buf, key_delim);
- strbuf_addstr(buf, vptr);
-
- if (must_free_vptr)
- free((char *)vptr);
}
strbuf_addch(buf, term);
return 0;
--
2.5.0.680.g69e7703
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to create temporary file '/var/git/tmv3-target-overlay.git/shallow_Un8ZOR': Permission denied
On Fri, 2015-08-21 at 13:36 +0200, Joakim Tjernlund wrote: > I cannot push: > # > git push origin > Login for jo...@git.transmode.se > Password: > Counting objects: 7, done. > Delta compression using up to 4 threads. > Compressing objects: 100% (7/7), done. > Writing objects: 100% (7/7), 13.73 KiB | 0 bytes/s, done. > Total 7 (delta 4), reused 0 (delta 0) > fatal: Unable to create temporary file > '/var/git/tmv3-target-overlay.git/shallow_Un8ZOR': Permission denied > fatal: The remote end hung up unexpectedly > fatal: The remote end hung up unexpectedly > cu-km022 tmv3-target-overlay # ls -l > total 0 > drwxr-xr-x 1 root root 72 Aug 19 17:59 ./ > drwxr-xr-x 1 root root 38 Aug 19 17:59 ../ > drwxr-xr-x 1 root root 216 Aug 21 13:07 .git/ > drwxr-xr-x 1 root root 22 Aug 19 17:59 metadata/ > drwxr-xr-x 1 root root 58 Aug 19 18:27 profiles/ > drwxr-xr-x 1 root root 74 Aug 19 17:59 sys-apps/ > drwxr-xr-x 1 root root 42 Aug 19 17:59 sys-libs/ > > On server I have: > # >ls -la > total 24 > dr-xr-sr-x 6 apache tm-3000 123 Jun 10 15:25 . > drwxr-xr-x 36 root root4096 Jun 25 11:11 .. > -r--r--r-- 1 root tm-3000 263 Jun 10 15:18 config > -r--r--r-- 1 apache tm-3000 73 Jun 10 15:18 description > -rw-r--r-- 1 root tm-30000 Jun 10 15:25 git-daemon-export-ok > -r--r--r-- 1 apache tm-3000 23 Jun 10 15:18 HEAD > drwxr-sr-x 2 root tm-3000 4096 Jun 10 15:18 hooks > drwxrwsr-x 2 apache tm-3000 20 Jun 10 15:18 info > drwxrwsr-x 135 apache tm-3000 4096 Aug 20 19:07 objects > drwxrwsr-x 4 apache tm-3000 29 Jun 10 15:18 refs > > > I suspect this is because no one is allowed to write the repos top dir. > It would be much better if git allowed a tmp dir inside the repo and used > that > for push etc. > Meanwhile, is there some workaround I can use? This error seems to only affect shallow clones. Forgot, git version 2.4.6 Jocke -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Unable to create temporary file '/var/git/tmv3-target-overlay.git/shallow_Un8ZOR': Permission denied
I cannot push: # > git push origin Login for jo...@git.transmode.se Password: Counting objects: 7, done. Delta compression using up to 4 threads. Compressing objects: 100% (7/7), done. Writing objects: 100% (7/7), 13.73 KiB | 0 bytes/s, done. Total 7 (delta 4), reused 0 (delta 0) fatal: Unable to create temporary file '/var/git/tmv3-target-overlay.git/shallow_Un8ZOR': Permission denied fatal: The remote end hung up unexpectedly fatal: The remote end hung up unexpectedly cu-km022 tmv3-target-overlay # ls -l total 0 drwxr-xr-x 1 root root 72 Aug 19 17:59 ./ drwxr-xr-x 1 root root 38 Aug 19 17:59 ../ drwxr-xr-x 1 root root 216 Aug 21 13:07 .git/ drwxr-xr-x 1 root root 22 Aug 19 17:59 metadata/ drwxr-xr-x 1 root root 58 Aug 19 18:27 profiles/ drwxr-xr-x 1 root root 74 Aug 19 17:59 sys-apps/ drwxr-xr-x 1 root root 42 Aug 19 17:59 sys-libs/ On server I have: # >ls -la total 24 dr-xr-sr-x 6 apache tm-3000 123 Jun 10 15:25 . drwxr-xr-x 36 root root4096 Jun 25 11:11 .. -r--r--r-- 1 root tm-3000 263 Jun 10 15:18 config -r--r--r-- 1 apache tm-3000 73 Jun 10 15:18 description -rw-r--r-- 1 root tm-30000 Jun 10 15:25 git-daemon-export-ok -r--r--r-- 1 apache tm-3000 23 Jun 10 15:18 HEAD drwxr-sr-x 2 root tm-3000 4096 Jun 10 15:18 hooks drwxrwsr-x 2 apache tm-3000 20 Jun 10 15:18 info drwxrwsr-x 135 apache tm-3000 4096 Aug 20 19:07 objects drwxrwsr-x 4 apache tm-3000 29 Jun 10 15:18 refs I suspect this is because no one is allowed to write the repos top dir. It would be much better if git allowed a tmp dir inside the repo and used that for push etc. Meanwhile, is there some workaround I can use? Jocke-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] describe: make '--always' fallback work after '--exact-match' failed
Quoting Junio C Hamano : SZEDER Gábor writes: 'git describe [...] --always' should always show the unique abbreviated object name as a fallback when the given commit cannot be described with the given set of options, see da2478dbb0 (describe --always: fall back to showing an abbreviated object name, 2008-03-02). However, this is not the case when the combination '--exact-match --always' is given and the commit cannot be described, because in such cases 'git describe' errors out, as if '--always' were not given at all. Respect '--always' and print the unique abbreviated object name instead of erroring out when the commit cannot be described with '--exact-match --always'. Signed-off-by: SZEDER Gábor Well, that can be argued both ways. Your patch introduces a regression, as "--exact-match" is an instruction to error out when no tag exactly matches, and you deliberately break that. This patch doesn't break '--exact-match', in fact doesn't modify it at all when it's on its own or combined with other options, but it makes '--exact-match --always' finally work. 'git describe' errors out by default if it can't describe the given commit. So if a user wants an exact match or an error otherwise, then he should not give '--always' at all, because that's the instruction to not error out but give the abbreviated object name instead. Why should '--exact-match' be any different from the other options that tell 'git describe' what to use for the description? Why should '--always' not work with '--exact-match', when it works in the other cases? Consider '--contains': it should find a tag that comes after the given commit or error out if such a tag doesn't exist. Now, in current git.git: $ git describe --contains master fatal: cannot describe 'ff86faf2fa02bc21933c9e1dcc75c8d81b3e104a' $ git describe --contains --always master ff86faf2fa Or the default behavior without any options: it should find a tag reachable from the given commit or error out, but what if we pass in a commit before the first tag? It recommends '--always': $ git describe e83c516 fatal: No tags can describe 'e83c5163316f89bfbde7d9ab23ca2e25604af290'. Try --always, or create some tags. $ git describe --always e83c516 e83c516331 My knee-jerk reaction is that the most sensible way forward is to make --exact-match and --always mutually incompatible. That would be wrong, it's perfectly valid to ask for an exactly matching tag or, if there is no such tag, the abbreviated object name as a fallback. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
