* [PATCHv2 30/36] pathspec: move long magic parsing out of prefix_pathspec
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
`prefix_pathspec` is quite a lengthy function and we plan on adding more.
Split it up for better readability. As we want to add code into the
inner loop of the long magic parsing, we also benefit from lower
indentation.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
pathspec.c | 84 +++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 47 insertions(+), 37 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 22ca74a126..ad13556c82 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -88,6 +88,52 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
+static void eat_long_magic(struct pathspec_item *item, const char *elt,
+ unsigned *magic, int *pathspec_prefix,
+ const char **copyfrom_, const char **long_magic_end)
+{
+ int i;
+ const char *copyfrom = *copyfrom_;
+ /* longhand */
+ const char *nextat;
+ for (copyfrom = elt + 2;
+ *copyfrom && *copyfrom != ')';
+ copyfrom = nextat) {
+ size_t len = strcspn(copyfrom, ",)");
+ if (copyfrom[len] == ',')
+ nextat = copyfrom + len + 1;
+ else
+ /* handle ')' and '\0' */
+ nextat = copyfrom + len;
+ if (!len)
+ continue;
+ for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if (strlen(pathspec_magic[i].name) == len &&
+ !strncmp(pathspec_magic[i].name, copyfrom, len)) {
+ *magic |= pathspec_magic[i].bit;
+ break;
+ }
+ if (starts_with(copyfrom, "prefix:")) {
+ char *endptr;
+ *pathspec_prefix = strtol(copyfrom + 7,
+ &endptr, 10);
+ if (endptr - copyfrom != len)
+ die(_("invalid parameter for pathspec magic 'prefix'"));
+ /* "i" would be wrong, but it does not matter */
+ break;
+ }
+ }
+ if (ARRAY_SIZE(pathspec_magic) <= i)
+ die(_("Invalid pathspec magic '%.*s' in '%s'"),
+ (int) len, copyfrom, elt);
+ }
+ if (*copyfrom != ')')
+ die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
+ *long_magic_end = copyfrom;
+ copyfrom++;
+ *copyfrom_ = copyfrom;
+}
+
/*
* Take an element of a pathspec and check for magic signatures.
* Append the result to the prefix. Return the magic bitmap.
@@ -150,43 +196,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
(flags & PATHSPEC_LITERAL_PATH)) {
; /* nothing to do */
} else if (elt[1] == '(') {
- /* longhand */
- const char *nextat;
- for (copyfrom = elt + 2;
- *copyfrom && *copyfrom != ')';
- copyfrom = nextat) {
- size_t len = strcspn(copyfrom, ",)");
- if (copyfrom[len] == ',')
- nextat = copyfrom + len + 1;
- else
- /* handle ')' and '\0' */
- nextat = copyfrom + len;
- if (!len)
- continue;
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
- if (strlen(pathspec_magic[i].name) == len &&
- !strncmp(pathspec_magic[i].name, copyfrom, len)) {
- magic |= pathspec_magic[i].bit;
- break;
- }
- if (starts_with(copyfrom, "prefix:")) {
- char *endptr;
- pathspec_prefix = strtol(copyfrom + 7,
- &endptr, 10);
- if (endptr - copyfrom != len)
- die(_("invalid parameter for pathspec magic 'prefix'"));
- /* "i" would be wrong, but it does not matter */
- break;
- }
- }
- if (ARRAY_SIZE(pathspec_magic) <= i)
- die(_("Invalid pathspec magic '%.*s' in '%s'"),
- (int) len, copyfrom, elt);
- }
- if (*copyfrom != ')')
- die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
- long_magic_end = copyfrom;
- copyfrom++;
+ eat_long_magic(item, elt, &magic, &pathspec_prefix, ©from, &long_magic_end);
} else {
/* shorthand */
for (copyfrom = elt + 1;
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 24/36] attr.c: always pass check[] to collect_some_attrs()
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
This function used to be called with check=NULL to signal it to
collect all attributes in the global check_all_attr[] array.
Because the longer term plan is to allocate check_all_attr[] and
attr_stack data structures per git_attr_check instance (i.e. "check"
here) to make the attr subsystem thread-safe, it is unacceptable.
Pass "Are we grabbing all attributes defined in the system?" bit as
a separate argument and pass it from the callers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/attr.c b/attr.c
index 1098300e54..92b3130f1e 100644
--- a/attr.c
+++ b/attr.c
@@ -760,11 +760,12 @@ static void empty_attr_check_elems(struct git_attr_check *check)
/*
* Collect attributes for path into the array pointed to by
- * check_all_attr. If check is not NULL, only attributes in
- * check[] are collected. Otherwise all attributes are collected.
+ * check_all_attr. If collect_all is zero, only attributes in
+ * check[] are collected. Otherwise, check[] is cleared and
+ * any and all attributes that are visible are collected in it.
*/
static void collect_some_attrs(const char *path, int pathlen,
- struct git_attr_check *check)
+ struct git_attr_check *check, int collect_all)
{
struct attr_stack *stk;
@@ -785,10 +786,11 @@ static void collect_some_attrs(const char *path, int pathlen,
}
prepare_attr_stack(path, dirlen);
+
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- if (check && !cannot_trust_maybe_real) {
+ if (!collect_all && !cannot_trust_maybe_real) {
struct git_attr_check_elem *celem = check->check;
rem = 0;
@@ -807,6 +809,17 @@ static void collect_some_attrs(const char *path, int pathlen,
rem = attr_nr;
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
rem = fill(path, pathlen, basename_offset, stk, rem);
+
+ if (collect_all) {
+ empty_attr_check_elems(check);
+ for (i = 0; i < attr_nr; i++) {
+ const struct git_attr *attr = check_all_attr[i].attr;
+ const char *value = check_all_attr[i].value;
+ if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
+ continue;
+ git_attr_check_append(check, attr)->value = value;
+ }
+ }
}
static int git_check_attrs(const char *path, int pathlen,
@@ -815,7 +828,7 @@ static int git_check_attrs(const char *path, int pathlen,
int i;
struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, check);
+ collect_some_attrs(path, pathlen, check, 0);
for (i = 0; i < check->check_nr; i++) {
const char *value = check_all_attr[celem[i].attr->attr_nr].value;
@@ -829,19 +842,7 @@ static int git_check_attrs(const char *path, int pathlen,
void git_all_attrs(const char *path, struct git_attr_check *check)
{
- int i;
-
- git_attr_check_clear(check);
- collect_some_attrs(path, strlen(path), NULL);
-
- for (i = 0; i < attr_nr; i++) {
- const char *name = check_all_attr[i].attr->name;
- const char *value = check_all_attr[i].value;
- if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
- continue;
- git_attr_check_append(check, git_attr(name));
- check->check[check->check_nr - 1].value = value;
- }
+ collect_some_attrs(path, strlen(path), check, 1);
}
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 13/36] attr: convert git_check_attrs() callers to use the new API
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The remaining callers are all simple "I have N attributes I am
interested in. I'll ask about them with various paths one by one".
After this step, no caller to git_check_attrs() remains. After
removing it, we can extend "struct git_attr_check" struct with data
that can be used in optimizing the query for the specific N
attributes it contains.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/pack-objects.c | 19 +++++--------------
convert.c | 18 +++++++-----------
ll-merge.c | 33 ++++++++++++++-------------------
userdiff.c | 19 ++++++++-----------
ws.c | 19 ++++++-------------
5 files changed, 40 insertions(+), 68 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3cb38ed68e..3918c07188 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -896,24 +896,15 @@ static void write_pack_file(void)
written, nr_result);
}
-static void setup_delta_attr_check(struct git_attr_check_elem *check)
-{
- static struct git_attr *attr_delta;
-
- if (!attr_delta)
- attr_delta = git_attr("delta");
-
- check[0].attr = attr_delta;
-}
-
static int no_try_delta(const char *path)
{
- struct git_attr_check_elem check[1];
+ static struct git_attr_check *check;
- setup_delta_attr_check(check);
- if (git_check_attrs(path, ARRAY_SIZE(check), check))
+ if (!check)
+ check = git_attr_check_initl("delta", NULL);
+ if (git_check_attr(path, check))
return 0;
- if (ATTR_FALSE(check->value))
+ if (ATTR_FALSE(check->check[0].value))
return 1;
return 0;
}
diff --git a/convert.c b/convert.c
index e1e47d2367..4eca0b5dda 100644
--- a/convert.c
+++ b/convert.c
@@ -779,24 +779,20 @@ struct conv_attrs {
int ident;
};
-static const char *conv_attr_name[] = {
- "crlf", "ident", "filter", "eol", "text",
-};
-#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)
-
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
- int i;
- static struct git_attr_check_elem ccheck[NUM_CONV_ATTRS];
+ static struct git_attr_check *check;
- if (!ccheck[0].attr) {
- for (i = 0; i < NUM_CONV_ATTRS; i++)
- ccheck[i].attr = git_attr(conv_attr_name[i]);
+ if (!check) {
+ check = git_attr_check_initl("crlf", "ident",
+ "filter", "eol", "text",
+ NULL);
user_convert_tail = &user_convert;
git_config(read_convert_config, NULL);
}
- if (!git_check_attrs(path, NUM_CONV_ATTRS, ccheck)) {
+ if (!git_check_attr(path, check)) {
+ struct git_attr_check_elem *ccheck = check->check;
ca->crlf_action = git_path_check_crlf(ccheck + 4);
if (ca->crlf_action == CRLF_UNDEFINED)
ca->crlf_action = git_path_check_crlf(ccheck + 0);
diff --git a/ll-merge.c b/ll-merge.c
index eb2c37ea92..bc6479ce7f 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -336,15 +336,6 @@ static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr
return &ll_merge_drv[LL_TEXT_MERGE];
}
-static int git_path_check_merge(const char *path, struct git_attr_check_elem check[2])
-{
- if (!check[0].attr) {
- check[0].attr = git_attr("merge");
- check[1].attr = git_attr("conflict-marker-size");
- }
- return git_check_attrs(path, 2, check);
-}
-
static void normalize_file(mmfile_t *mm, const char *path)
{
struct strbuf strbuf = STRBUF_INIT;
@@ -362,7 +353,7 @@ int ll_merge(mmbuffer_t *result_buf,
mmfile_t *theirs, const char *their_label,
const struct ll_merge_options *opts)
{
- static struct git_attr_check_elem check[2];
+ static struct git_attr_check *check;
static const struct ll_merge_options default_opts;
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
@@ -376,10 +367,14 @@ int ll_merge(mmbuffer_t *result_buf,
normalize_file(ours, path);
normalize_file(theirs, path);
}
- if (!git_path_check_merge(path, check)) {
- ll_driver_name = check[0].value;
- if (check[1].value) {
- marker_size = atoi(check[1].value);
+
+ if (!check)
+ check = git_attr_check_initl("merge", "conflict-marker-size", NULL);
+
+ if (!git_check_attr(path, check)) {
+ ll_driver_name = check->check[0].value;
+ if (check->check[1].value) {
+ marker_size = atoi(check->check[1].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
@@ -398,13 +393,13 @@ int ll_merge(mmbuffer_t *result_buf,
int ll_merge_marker_size(const char *path)
{
- static struct git_attr_check_elem check;
+ static struct git_attr_check *check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
- if (!check.attr)
- check.attr = git_attr("conflict-marker-size");
- if (!git_check_attrs(path, 1, &check) && check.value) {
- marker_size = atoi(check.value);
+ if (!check)
+ check = git_attr_check_initl("conflict-marker-size", NULL);
+ if (!git_check_attr(path, check) && check->check[0].value) {
+ marker_size = atoi(check->check[0].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
diff --git a/userdiff.c b/userdiff.c
index 4de3289af3..46dfd32ad4 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -262,25 +262,22 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {
struct userdiff_driver *userdiff_find_by_path(const char *path)
{
- static struct git_attr *attr;
- struct git_attr_check_elem check;
-
- if (!attr)
- attr = git_attr("diff");
- check.attr = attr;
+ static struct git_attr_check *check;
+ if (!check)
+ check = git_attr_check_initl("diff", NULL);
if (!path)
return NULL;
- if (git_check_attrs(path, 1, &check))
+ if (git_check_attr(path, check))
return NULL;
- if (ATTR_TRUE(check.value))
+ if (ATTR_TRUE(check->check[0].value))
return &driver_true;
- if (ATTR_FALSE(check.value))
+ if (ATTR_FALSE(check->check[0].value))
return &driver_false;
- if (ATTR_UNSET(check.value))
+ if (ATTR_UNSET(check->check[0].value))
return NULL;
- return userdiff_find_by_name(check.value);
+ return userdiff_find_by_name(check->check[0].value);
}
struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver)
diff --git a/ws.c b/ws.c
index 7350905b7f..bb3270c568 100644
--- a/ws.c
+++ b/ws.c
@@ -71,24 +71,17 @@ unsigned parse_whitespace_rule(const char *string)
return rule;
}
-static void setup_whitespace_attr_check(struct git_attr_check_elem *check)
-{
- static struct git_attr *attr_whitespace;
-
- if (!attr_whitespace)
- attr_whitespace = git_attr("whitespace");
- check[0].attr = attr_whitespace;
-}
-
unsigned whitespace_rule(const char *pathname)
{
- struct git_attr_check_elem attr_whitespace_rule;
+ static struct git_attr_check *attr_whitespace_rule;
+
+ if (!attr_whitespace_rule)
+ attr_whitespace_rule = git_attr_check_initl("whitespace", NULL);
- setup_whitespace_attr_check(&attr_whitespace_rule);
- if (!git_check_attrs(pathname, 1, &attr_whitespace_rule)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule)) {
const char *value;
- value = attr_whitespace_rule.value;
+ value = attr_whitespace_rule->check[0].value;
if (ATTR_TRUE(value)) {
/* true (whitespace) */
unsigned all_rule = ws_tab_width(whitespace_rule_cfg);
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 17/36] attr: expose validity check for attribute names
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Export attr_name_valid() function, and a helper function that
returns the message to be given when a given <name, len> pair
is not a good name for an attribute.
We could later update the message to exactly spell out what the
rules for a good attribute name are, etc.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 39 +++++++++++++++++++++++++--------------
attr.h | 10 ++++++++++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/attr.c b/attr.c
index 7058e1c9fa..33021cc857 100644
--- a/attr.c
+++ b/attr.c
@@ -59,23 +59,38 @@ static unsigned hash_name(const char *name, int namelen)
return val;
}
-static int invalid_attr_name(const char *name, int namelen)
+int attr_name_valid(const char *name, size_t namelen)
{
/*
* Attribute name cannot begin with '-' and must consist of
* characters from [-A-Za-z0-9_.].
*/
if (namelen <= 0 || *name == '-')
- return -1;
+ return 0;
while (namelen--) {
char ch = *name++;
if (! (ch == '-' || ch == '.' || ch == '_' ||
('0' <= ch && ch <= '9') ||
('a' <= ch && ch <= 'z') ||
('A' <= ch && ch <= 'Z')) )
- return -1;
+ return 0;
}
- return 0;
+ return 1;
+}
+
+void invalid_attr_name_message(struct strbuf *err, const char *name, int len)
+{
+ strbuf_addf(err, _("%.*s is not a valid attribute name"),
+ len, name);
+}
+
+static void report_invalid_attr(const char *name, size_t len,
+ const char *src, int lineno)
+{
+ struct strbuf err = STRBUF_INIT;
+ invalid_attr_name_message(&err, name, len);
+ fprintf(stderr, "%s: %s:%d\n", err.buf, src, lineno);
+ strbuf_release(&err);
}
struct git_attr *git_attr_counted(const char *name, size_t len)
@@ -90,7 +105,7 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
return a;
}
- if (invalid_attr_name(name, len))
+ if (!attr_name_valid(name, len))
return NULL;
FLEX_ALLOC_MEM(a, name, name, len);
@@ -176,17 +191,15 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
cp++;
len--;
}
- if (invalid_attr_name(cp, len)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- len, cp, src, lineno);
+ if (!attr_name_valid(cp, len)) {
+ report_invalid_attr(cp, len, src, lineno);
return NULL;
}
} else {
/*
* As this function is always called twice, once with
* e == NULL in the first pass and then e != NULL in
- * the second pass, no need for invalid_attr_name()
+ * the second pass, no need for attr_name_valid()
* check here.
*/
if (*cp == '-' || *cp == '!') {
@@ -229,10 +242,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
name += strlen(ATTRIBUTE_MACRO_PREFIX);
name += strspn(name, blank);
namelen = strcspn(name, blank);
- if (invalid_attr_name(name, namelen)) {
- fprintf(stderr,
- "%.*s is not a valid attribute name: %s:%d\n",
- namelen, name, src, lineno);
+ if (!attr_name_valid(name, namelen)) {
+ report_invalid_attr(name, namelen, src, lineno);
goto fail_return;
}
}
diff --git a/attr.h b/attr.h
index bcedf928b5..fe26f3a588 100644
--- a/attr.h
+++ b/attr.h
@@ -13,6 +13,16 @@ extern struct git_attr *git_attr(const char *);
/* The same, but with counted string */
extern struct git_attr *git_attr_counted(const char *, size_t);
+/*
+ * Return the name of the attribute represented by the argument. The
+ * return value is a pointer to a null-delimited string that is part
+ * of the internal data structure; it should not be modified or freed.
+ */
+extern const char *git_attr_name(const struct git_attr *);
+
+extern int attr_name_valid(const char *name, size_t namelen);
+extern void invalid_attr_name_message(struct strbuf *, const char *, int);
+
/* Internal use */
extern const char git_attr__true[];
extern const char git_attr__false[];
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 16/36] attr: add counted string version of git_attr()
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Often a potential caller has <name, namelen> pair that
represents the name it wants to create an attribute out of.
When name[namelen] is not NUL, the caller has to xmemdupz()
only to call git_attr().
Add git_attr_counted() that takes such a counted string instead of
"const char *name".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 8 ++++----
attr.h | 5 ++++-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/attr.c b/attr.c
index dc9d61b411..7058e1c9fa 100644
--- a/attr.c
+++ b/attr.c
@@ -78,7 +78,7 @@ static int invalid_attr_name(const char *name, int namelen)
return 0;
}
-static struct git_attr *git_attr_internal(const char *name, int len)
+struct git_attr *git_attr_counted(const char *name, size_t len)
{
unsigned hval = hash_name(name, len);
unsigned pos = hval % HASHSIZE;
@@ -109,7 +109,7 @@ static struct git_attr *git_attr_internal(const char *name, int len)
struct git_attr *git_attr(const char *name)
{
- return git_attr_internal(name, strlen(name));
+ return git_attr_counted(name, strlen(name));
}
/* What does a matched pattern decide? */
@@ -199,7 +199,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
else {
e->setto = xmemdupz(equals + 1, ep - equals - 1);
}
- e->attr = git_attr_internal(cp, len);
+ e->attr = git_attr_counted(cp, len);
}
return ep + strspn(ep, blank);
}
@@ -254,7 +254,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
sizeof(struct attr_state) * num_attr +
(is_macro ? 0 : namelen + 1));
if (is_macro) {
- res->u.attr = git_attr_internal(name, namelen);
+ res->u.attr = git_attr_counted(name, namelen);
res->u.attr->maybe_macro = 1;
} else {
char *p = (char *)&(res->state[num_attr]);
diff --git a/attr.h b/attr.h
index c84f164b8e..bcedf928b5 100644
--- a/attr.h
+++ b/attr.h
@@ -8,7 +8,10 @@ struct git_attr;
* Given a string, return the gitattribute object that
* corresponds to it.
*/
-struct git_attr *git_attr(const char *);
+extern struct git_attr *git_attr(const char *);
+
+/* The same, but with counted string */
+extern struct git_attr *git_attr_counted(const char *, size_t);
/* Internal use */
extern const char git_attr__true[];
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 15/36] attr: add counted string version of git_check_attr()
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Often a potential caller has <path, pathlen> pair that
represents the path it wants to ask attributes for; when
path[pathlen] is not NUL, the caller has to xmemdupz()
only to call git_check_attr().
Add git_check_attr_counted() that takes such a counted
string instead of "const char *path".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 23 ++++++++++++++---------
attr.h | 1 +
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/attr.c b/attr.c
index 30953c905b..dc9d61b411 100644
--- a/attr.c
+++ b/attr.c
@@ -738,20 +738,19 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
* check_all_attr. If num is non-zero, only attributes in check[] are
* collected. Otherwise all attributes are collected.
*/
-static void collect_some_attrs(const char *path, int num,
+static void collect_some_attrs(const char *path, int pathlen, int num,
struct git_attr_check_elem *check)
{
struct attr_stack *stk;
- int i, pathlen, rem, dirlen;
+ int i, rem, dirlen;
const char *cp, *last_slash = NULL;
int basename_offset;
- for (cp = path; *cp; cp++) {
+ for (cp = path; cp < path + pathlen; cp++) {
if (*cp == '/' && cp[1])
last_slash = cp;
}
- pathlen = cp - path;
if (last_slash) {
basename_offset = last_slash + 1 - path;
dirlen = last_slash - path;
@@ -782,12 +781,12 @@ static void collect_some_attrs(const char *path, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-static int git_check_attrs(const char *path, int num,
+static int git_check_attrs(const char *path, int pathlen, int num,
struct git_attr_check_elem *check)
{
int i;
- collect_some_attrs(path, num, check);
+ collect_some_attrs(path, pathlen, num, check);
for (i = 0; i < num; i++) {
const char *value = check_all_attr[check[i].attr->attr_nr].value;
@@ -804,7 +803,7 @@ void git_all_attrs(const char *path, struct git_attr_check *check)
int i;
git_attr_check_clear(check);
- collect_some_attrs(path, 0, NULL);
+ collect_some_attrs(path, strlen(path), 0, NULL);
for (i = 0; i < attr_nr; i++) {
const char *name = check_all_attr[i].attr->name;
@@ -829,10 +828,16 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
use_index = istate;
}
-int git_check_attr(const char *path, struct git_attr_check *check)
+int git_check_attr_counted(const char *path, int pathlen,
+ struct git_attr_check *check)
{
check->finalized = 1;
- return git_check_attrs(path, check->check_nr, check->check);
+ return git_check_attrs(path, pathlen, check->check_nr, check->check);
+}
+
+int git_check_attr(const char *path, struct git_attr_check *check)
+{
+ return git_check_attr_counted(path, strlen(path), check);
}
struct git_attr_check *git_attr_check_initl(const char *one, ...)
diff --git a/attr.h b/attr.h
index 506db0ca74..c84f164b8e 100644
--- a/attr.h
+++ b/attr.h
@@ -38,6 +38,7 @@ struct git_attr_check {
extern struct git_attr_check *git_attr_check_initl(const char *, ...);
extern int git_check_attr(const char *path, struct git_attr_check *);
+extern int git_check_attr_counted(const char *, int, struct git_attr_check *);
extern struct git_attr_check *git_attr_check_alloc(void);
extern struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *, const struct git_attr *);
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 12/36] attr: convert git_all_attrs() to use "struct git_attr_check"
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
This updates the other two ways the attribute check is done via an
array of "struct git_attr_check_elem" elements. These two niches
appear only in "git check-attr".
* The caller does not know offhand what attributes it wants to ask
about and cannot use git_attr_check_initl() to prepare the
git_attr_check structure.
* The caller may not know what attributes it wants to ask at all,
and instead wants to learn everything that the given path has.
Such a caller can call git_attr_check_alloc() to allocate an empty
git_attr_check, and then call git_attr_check_append() to add
attribute names one by one. A new attribute can be appended until
git_attr_check structure is "finalized", which happens when it is
used to ask for attributes for any path by calling git_check_attr()
or git_all_attrs(). A git_attr_check structure that is initialized
by git_attr_check_initl() is already finalized when it is returned.
I am not at all happy with the way git_all_attrs() API turned out to
be, but it is only to support one niche caller ("check-attr --all"),
so I'll stop here for now.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
attr.c | 75 ++++++++++++++++++++++++++++++++++++++--------------
attr.h | 16 ++++++-----
builtin/check-attr.c | 51 ++++++++++++++++++-----------------
3 files changed, 90 insertions(+), 52 deletions(-)
diff --git a/attr.c b/attr.c
index 20d64b34f4..7ffa19875b 100644
--- a/attr.c
+++ b/attr.c
@@ -728,6 +728,11 @@ static int macroexpand_one(int nr, int rem)
return rem;
}
+static int attr_check_is_dynamic(const struct git_attr_check *check)
+{
+ return (void *)(check->check) != (void *)(check + 1);
+}
+
/*
* Collect attributes for path into the array pointed to by
* check_all_attr. If num is non-zero, only attributes in check[] are
@@ -793,32 +798,21 @@ int git_check_attrs(const char *path, int num, struct git_attr_check_elem *check
return 0;
}
-int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check)
+void git_all_attrs(const char *path, struct git_attr_check *check)
{
- int i, count, j;
+ int i;
+ git_attr_check_clear(check);
collect_some_attrs(path, 0, NULL);
- /* Count the number of attributes that are set. */
- count = 0;
- for (i = 0; i < attr_nr; i++) {
- const char *value = check_all_attr[i].value;
- if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
- ++count;
- }
- *num = count;
- ALLOC_ARRAY(*check, count);
- j = 0;
for (i = 0; i < attr_nr; i++) {
+ const char *name = check_all_attr[i].attr->name;
const char *value = check_all_attr[i].value;
- if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
- (*check)[j].attr = check_all_attr[i].attr;
- (*check)[j].value = value;
- ++j;
- }
+ if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
+ continue;
+ git_attr_check_append(check, git_attr(name));
+ check->check[check->check_nr - 1].value = value;
}
-
- return 0;
}
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
@@ -836,6 +830,7 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
int git_check_attr(const char *path, struct git_attr_check *check)
{
+ check->finalized = 1;
return git_check_attrs(path, check->check_nr, check->check);
}
@@ -853,17 +848,57 @@ struct git_attr_check *git_attr_check_initl(const char *one, ...)
check = xcalloc(1,
sizeof(*check) + cnt * sizeof(*(check->check)));
check->check_nr = cnt;
+ check->finalized = 1;
check->check = (struct git_attr_check_elem *)(check + 1);
check->check[0].attr = git_attr(one);
va_start(params, one);
for (cnt = 1; cnt < check->check_nr; cnt++) {
+ struct git_attr *attr;
param = va_arg(params, const char *);
if (!param)
die("BUG: counted %d != ended at %d",
check->check_nr, cnt);
- check->check[cnt].attr = git_attr(param);
+ attr = git_attr(param);
+ if (!attr)
+ die("BUG: %s: not a valid attribute name", param);
+ check->check[cnt].attr = attr;
}
va_end(params);
return check;
}
+
+struct git_attr_check *git_attr_check_alloc(void)
+{
+ return xcalloc(1, sizeof(struct git_attr_check));
+}
+
+struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
+ const struct git_attr *attr)
+{
+ struct git_attr_check_elem *elem;
+ if (check->finalized)
+ die("BUG: append after git_attr_check structure is finalized");
+ if (!attr_check_is_dynamic(check))
+ die("BUG: appending to a statically initialized git_attr_check");
+ ALLOC_GROW(check->check, check->check_nr + 1, check->check_alloc);
+ elem = &check->check[check->check_nr++];
+ elem->attr = attr;
+ return elem;
+}
+
+void git_attr_check_clear(struct git_attr_check *check)
+{
+ if (!attr_check_is_dynamic(check))
+ die("BUG: clearing a statically initialized git_attr_check");
+ free(check->check);
+ check->check_nr = 0;
+ check->check_alloc = 0;
+ check->finalized = 0;
+}
+
+void git_attr_check_free(struct git_attr_check *check)
+{
+ git_attr_check_clear(check);
+ free(check);
+}
diff --git a/attr.h b/attr.h
index 3fd8690749..0d94077d78 100644
--- a/attr.h
+++ b/attr.h
@@ -30,6 +30,7 @@ struct git_attr_check_elem {
};
struct git_attr_check {
+ int finalized;
int check_nr;
int check_alloc;
struct git_attr_check_elem *check;
@@ -38,6 +39,12 @@ struct git_attr_check {
extern struct git_attr_check *git_attr_check_initl(const char *, ...);
extern int git_check_attr(const char *path, struct git_attr_check *);
+extern struct git_attr_check *git_attr_check_alloc(void);
+extern struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *, const struct git_attr *);
+
+extern void git_attr_check_clear(struct git_attr_check *);
+extern void git_attr_check_free(struct git_attr_check *);
+
/*
* Return the name of the attribute represented by the argument. The
* return value is a pointer to a null-delimited string that is part
@@ -48,13 +55,10 @@ extern const char *git_attr_name(const struct git_attr *);
int git_check_attrs(const char *path, int, struct git_attr_check_elem *);
/*
- * Retrieve all attributes that apply to the specified path. *num
- * will be set to the number of attributes on the path; **check will
- * be set to point at a newly-allocated array of git_attr_check
- * objects describing the attributes and their values. *check must be
- * free()ed by the caller.
+ * Retrieve all attributes that apply to the specified path.
+ * check holds the attributes and their values.
*/
-int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check);
+void git_all_attrs(const char *path, struct git_attr_check *check);
enum git_attr_direction {
GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 97e3837ead..ec61476f68 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
OPT_END()
};
-static void output_attr(int cnt, struct git_attr_check_elem *check,
- const char *file)
+static void output_attr(struct git_attr_check *check, const char *file)
{
int j;
+ int cnt = check->check_nr;
+
for (j = 0; j < cnt; j++) {
- const char *value = check[j].value;
+ const char *value = check->check[j].value;
if (ATTR_TRUE(value))
value = "set";
@@ -42,36 +43,37 @@ static void output_attr(int cnt, struct git_attr_check_elem *check,
printf("%s%c" /* path */
"%s%c" /* attrname */
"%s%c" /* attrvalue */,
- file, 0, git_attr_name(check[j].attr), 0, value, 0);
+ file, 0,
+ git_attr_name(check->check[j].attr), 0, value, 0);
} else {
quote_c_style(file, NULL, stdout, 0);
- printf(": %s: %s\n", git_attr_name(check[j].attr), value);
+ printf(": %s: %s\n",
+ git_attr_name(check->check[j].attr), value);
}
-
}
}
static void check_attr(const char *prefix,
- int cnt, struct git_attr_check_elem *check,
+ struct git_attr_check *check,
const char *file)
{
char *full_path =
prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
if (check != NULL) {
- if (git_check_attrs(full_path, cnt, check))
- die("git_check_attrs died");
- output_attr(cnt, check, file);
+ if (git_check_attr(full_path, check))
+ die("git_check_attr died");
+ output_attr(check, file);
} else {
- if (git_all_attrs(full_path, &cnt, &check))
- die("git_all_attrs died");
- output_attr(cnt, check, file);
- free(check);
+ check = git_attr_check_alloc();
+ git_all_attrs(full_path, check);
+ output_attr(check, file);
+ git_attr_check_free(check);
}
free(full_path);
}
static void check_attr_stdin_paths(const char *prefix,
- int cnt, struct git_attr_check_elem *check)
+ struct git_attr_check *check)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf unquoted = STRBUF_INIT;
@@ -85,7 +87,7 @@ static void check_attr_stdin_paths(const char *prefix,
die("line is badly quoted");
strbuf_swap(&buf, &unquoted);
}
- check_attr(prefix, cnt, check, buf.buf);
+ check_attr(prefix, check, buf.buf);
maybe_flush_or_die(stdout, "attribute to stdout");
}
strbuf_release(&buf);
@@ -100,7 +102,7 @@ static NORETURN void error_with_usage(const char *msg)
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
- struct git_attr_check_elem *check;
+ struct git_attr_check *check;
int cnt, i, doubledash, filei;
if (!is_bare_repository())
@@ -163,24 +165,21 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
if (all_attrs) {
check = NULL;
} else {
- check = xcalloc(cnt, sizeof(*check));
+ check = git_attr_check_alloc();
for (i = 0; i < cnt; i++) {
- const char *name;
- struct git_attr *a;
- name = argv[i];
- a = git_attr(name);
+ struct git_attr *a = git_attr(argv[i]);
if (!a)
return error("%s: not a valid attribute name",
- name);
- check[i].attr = a;
+ argv[i]);
+ git_attr_check_append(check, a);
}
}
if (stdin_paths)
- check_attr_stdin_paths(prefix, cnt, check);
+ check_attr_stdin_paths(prefix, check);
else {
for (i = filei; i < argc; i++)
- check_attr(prefix, cnt, check, argv[i]);
+ check_attr(prefix, check, argv[i]);
maybe_flush_or_die(stdout, "attribute to stdout");
}
return 0;
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* [PATCHv2 11/36] attr: (re)introduce git_check_attr() and struct git_attr_check
From: Stefan Beller @ 2016-10-28 18:54 UTC (permalink / raw)
To: gitster; +Cc: bmwill, pclouds, git, Stefan Beller
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
A common pattern to check N attributes for many paths is to
(1) prepare an array A of N git_attr_check_elem items;
(2) call git_attr() to intern the N attribute names and fill A;
(3) repeatedly call git_check_attrs() for path with N and A;
A look-up for these N attributes for a single path P scans the
entire attr_stack, starting from the .git/info/attributes file and
then .gitattributes file in the directory the path P is in, going
upwards to find .gitattributes file found in parent directories.
An earlier commit 06a604e6 (attr: avoid heavy work when we know the
specified attr is not defined, 2014-12-28) tried to optimize out
this scanning for one trivial special case: when the attribute being
sought is known not to exist, we do not have to scan for it. While
this may be a cheap and effective heuristic, it would not work well
when N is (much) more than 1.
What we would want is a more customized way to skip irrelevant
entries in the attribute stack, and the definition of irrelevance
is tied to the set of attributes passed to git_check_attrs() call,
i.e. the set of attributes being sought. The data necessary for
this optimization needs to live alongside the set of attributes, but
a simple array of git_attr_check_elem simply does not have any place
for that.
Introduce "struct git_attr_check" that contains N, the number of
attributes being sought, and A, the array that holds N
git_attr_check_elem items, and a function git_check_attr() that
takes a path P and this structure as its parameters. This structure
can later be extended to hold extra data necessary for optimization.
Also, to make it easier to write the first two steps in common
cases, introduce git_attr_check_initl() helper function, which takes
a NULL-terminated list of attribute names and initialize this
structure.
As an illustration of this new API, convert archive.c that asks for
export-subst and export-ignore attributes for each paths.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
archive.c | 24 ++++++------------------
attr.c | 34 ++++++++++++++++++++++++++++++++++
attr.h | 9 +++++++++
3 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/archive.c b/archive.c
index 2dc8d6ca57..11e3951371 100644
--- a/archive.c
+++ b/archive.c
@@ -87,19 +87,6 @@ void *sha1_file_to_archive(const struct archiver_args *args,
return buffer;
}
-static void setup_archive_check(struct git_attr_check_elem *check)
-{
- static struct git_attr *attr_export_ignore;
- static struct git_attr *attr_export_subst;
-
- if (!attr_export_ignore) {
- attr_export_ignore = git_attr("export-ignore");
- attr_export_subst = git_attr("export-subst");
- }
- check[0].attr = attr_export_ignore;
- check[1].attr = attr_export_subst;
-}
-
struct directory {
struct directory *up;
struct object_id oid;
@@ -123,7 +110,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
struct archiver_context *c = context;
struct archiver_args *args = c->args;
write_archive_entry_fn_t write_entry = c->write_entry;
- struct git_attr_check_elem check[2];
+ static struct git_attr_check *check;
const char *path_without_prefix;
int err;
@@ -137,11 +124,12 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
strbuf_addch(&path, '/');
path_without_prefix = path.buf + args->baselen;
- setup_archive_check(check);
- if (!git_check_attrs(path_without_prefix, ARRAY_SIZE(check), check)) {
- if (ATTR_TRUE(check[0].value))
+ if (!check)
+ check = git_attr_check_initl("export-ignore", "export-subst", NULL);
+ if (!git_check_attr(path_without_prefix, check)) {
+ if (ATTR_TRUE(check->check[0].value))
return 0;
- args->convert = ATTR_TRUE(check[1].value);
+ args->convert = ATTR_TRUE(check->check[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
diff --git a/attr.c b/attr.c
index ff7f0a59eb..20d64b34f4 100644
--- a/attr.c
+++ b/attr.c
@@ -833,3 +833,37 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
drop_attr_stack();
use_index = istate;
}
+
+int git_check_attr(const char *path, struct git_attr_check *check)
+{
+ return git_check_attrs(path, check->check_nr, check->check);
+}
+
+struct git_attr_check *git_attr_check_initl(const char *one, ...)
+{
+ struct git_attr_check *check;
+ int cnt;
+ va_list params;
+ const char *param;
+
+ va_start(params, one);
+ for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
+ ;
+ va_end(params);
+ check = xcalloc(1,
+ sizeof(*check) + cnt * sizeof(*(check->check)));
+ check->check_nr = cnt;
+ check->check = (struct git_attr_check_elem *)(check + 1);
+
+ check->check[0].attr = git_attr(one);
+ va_start(params, one);
+ for (cnt = 1; cnt < check->check_nr; cnt++) {
+ param = va_arg(params, const char *);
+ if (!param)
+ die("BUG: counted %d != ended at %d",
+ check->check_nr, cnt);
+ check->check[cnt].attr = git_attr(param);
+ }
+ va_end(params);
+ return check;
+}
diff --git a/attr.h b/attr.h
index dd3c4a3aef..3fd8690749 100644
--- a/attr.h
+++ b/attr.h
@@ -29,6 +29,15 @@ struct git_attr_check_elem {
const char *value;
};
+struct git_attr_check {
+ int check_nr;
+ int check_alloc;
+ struct git_attr_check_elem *check;
+};
+
+extern struct git_attr_check *git_attr_check_initl(const char *, ...);
+extern int git_check_attr(const char *path, struct git_attr_check *);
+
/*
* Return the name of the attribute represented by the argument. The
* return value is a pointer to a null-delimited string that is part
--
2.10.1.714.ge3da0db
^ permalink raw reply related
* Re: "git push" says "src refspec XYZ matches more than one" even without explicit XYZ argument.
From: Junio C Hamano @ 2016-10-28 19:13 UTC (permalink / raw)
To: Kannan Goundan; +Cc: git
In-Reply-To: <CAM7aVoUOxx6Fa+yG2JzQhoX-mE4Lgp9ejvwncx5bdL+1tuQQFg@mail.gmail.com>
Kannan Goundan <kannan@cakoose.com> writes:
> 1. My repo has a branch named 'v1' that is tracking 'origin/v1'.
> 2. My repo has a tag named 'v1'.
> 3. I have "push.default" set to "upstream".
>
> I made a commit on branch 'v1' and tried doing a push:
>
> # git push
> error: src refspec v1 matches more than one.
> error: failed to push some refs to 'git@github.com:whatever/ns1-go.git'
>
> If I rename the branch to 'v1-dev', then the push goes through.
>
> I understand why the command "git push origin/v1 v1" is ambiguous.
> But if I do a plain "git push", I thought Git would know to push my
> current branch.
>
> [Git version 2.10.1 from Homebrew on Mac OS 10.11.6.]
Thanks.
You are right that the refspec Git internally create for this case
should not be v1:refs/heads/v1, which would notice that the source
side (e.g. "v1") is ambiguous. Instead we should spell that out.
Perhaps something like this would fix it for you?
builtin/push.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b7e6..02fd235742 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -194,7 +194,7 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,
die_push_simple(branch, remote);
}
- strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
+ strbuf_addf(&refspec, "%s:%s", branch->refname, branch->merge[0]->src);
add_refspec(refspec.buf);
}
^ permalink raw reply related
* [PATCH] push: do not use potentially ambiguous default refspec
From: Junio C Hamano @ 2016-10-28 19:25 UTC (permalink / raw)
To: git; +Cc: Kannan Goundan
When the user does the lazy "git push" with no parameters with
push.default set to either "upstream", "simple" or "current",
we internally generated a refspec that has the current branch name
on the source side and used it to push.
However, the branch name (say "test") may be an ambiguous refname in
the context of the source repository---there may be a tag with the
same name, for example. This would trigger an unnecessary error
without any fault on the end-user's side.
Be explicit and give a full refname as the source side to avoid the
ambiguity. The destination side when pushing with the "current"
sent only the name of the branch and forcing the receiving end to
guess, which is the same issue. Be explicit there as well.
Reported-by: Kannan Goundan <kannan@cakoose.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* It is appreciated if somebody with spare cycles can add a test or
two for this in t/t5523-push-upstream.sh or somewhere nearby.
builtin/push.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 4e9e4dbab2..00a1b68483 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -194,15 +194,18 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,
die_push_simple(branch, remote);
}
- strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
+ strbuf_addf(&refspec, "%s:%s", branch->refname, branch->merge[0]->src);
add_refspec(refspec.buf);
}
static void setup_push_current(struct remote *remote, struct branch *branch)
{
+ struct strbuf refspec = STRBUF_INIT;
+
if (!branch)
die(_(message_detached_head_die), remote->name);
- add_refspec(branch->name);
+ strbuf_addf(&refspec, "%s:%s", branch->refname, branch->refname);
+ add_refspec(refspec.buf);
}
static int is_workflow_triangular(struct remote *remote)
--
2.10.2-722-gd3f6888e25
^ permalink raw reply related
* Re: [PATCH 5/5] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-10-28 19:35 UTC (permalink / raw)
To: git
In-Reply-To: <20161027223834.35312-6-bmwill@google.com>
On 10/27, Brandon Williams wrote:
> diff --git a/tree-walk.c b/tree-walk.c
> index 828f4356b..b3f996174 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -999,10 +999,11 @@ static enum interesting do_match(const struct name_entry *entry,
> return entry_interesting;
>
> /*
> - * Match all directories. We'll try to
> - * match files later on.
> + * Match all directories and gitlinks. We'll
> + * try to match files later on.
> */
> - if (ps->recursive && S_ISDIR(entry->mode))
> + if (ps->recursive && (S_ISDIR(entry->mode) ||
> + S_ISGITLINK(entry->mode)))
> return entry_interesting;
> }
>
> @@ -1043,13 +1044,13 @@ static enum interesting do_match(const struct name_entry *entry,
> strbuf_setlen(base, base_offset + baselen);
>
> /*
> - * Match all directories. We'll try to match files
> - * later on.
> - * max_depth is ignored but we may consider support it
> - * in future, see
> + * Match all directories and gitlinks. We'll try to match files
> + * later on. max_depth is ignored but we may consider support
> + * it in future, see
> * http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
> */
> - if (ps->recursive && S_ISDIR(entry->mode))
> + if (ps->recursive && (S_ISDIR(entry->mode) ||
> + S_ISGITLINK(entry->mode)))
> return entry_interesting;
> }
> return never_interesting; /* No matches */
Looks like this change actually breaks a test in t4010-diff-pathspec.sh.
I think I'll have to add a flag to optionally let through submodules as
apposed to just treating them like directories.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-28 20:26 UTC (permalink / raw)
To: Jacob Keller
Cc: Johannes Schindelin, Stefan Beller, Junio C Hamano,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <CA+P7+xoyF8EG079eC-dfTSj+YrbxhPWx356-jZ90gDs6SwyppA@mail.gmail.com>
Am 28.10.2016 um 20:48 schrieb Jacob Keller:
> On Fri, Oct 28, 2016 at 4:58 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi Jake,
>>
>> On Thu, 27 Oct 2016, Jacob Keller wrote:
>>
>>> I agree with Stefan that there isn't really a great place to put a
>>> dynamic initialization.
>>
>> Ummm. Wait. What???
>>
>> https://github.com/git/git/blob/v2.10.1/common-main.c#L25-L41
>
> I think Stefan has since provided a better suggestion of isolating the
> dynamic initialization into the MINGW startup section. However, you
> are right either place works, though I think platforms which support
> static initialization should use it.
It's a pity that this new patch introduces the first use of
PTHREAD_MUTEX_INITILIZER. (I do not count the one in compat/nedmalloc,
it's in platform specific code.) We have to introduce a dummy definition
on Windows to even compile the code.
Now we have a double-edged sword: Other coders who are not aware of this
thread might think it works. But it does not.
Another problem with the proposed patch is that there is no declaration
for attr_start() before the call in compat/mingw.c. We would have to
move the declaration of attr_start() to cache.h (for example), because
#including attr.h in compat/mingw.c is plainly wrong. However, it would
not be a major offense to #include attr.h in common-main.c. But when we
do that, we can certainly spare the few cycles to call pthread_mutex_init.
-- Hannes
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-28 20:29 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jacob Keller, Johannes Schindelin, Stefan Beller,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <38b70094-5550-8512-7735-a6739f435803@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Another problem with the proposed patch is that there is no
> declaration for attr_start() before the call in compat/mingw.c. We
> would have to move the declaration of attr_start() to cache.h (for
> example), because #including attr.h in compat/mingw.c is plainly
> wrong. However, it would not be a major offense to #include attr.h in
> common-main.c. But when we do that, we can certainly spare the few
> cycles to call pthread_mutex_init.
That sounds like a good argument to have it in common-main.c.
Would it mean that the code that defines the mutex needs to have
#ifdef that defines a no-op attr_start() and defines the mutex with
PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex
without initializatin, with the real attr_start(), though?
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Stefan Beller @ 2016-10-28 20:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, Jacob Keller, Johannes Schindelin,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <xmqqr370z07v.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 28, 2016 at 1:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Another problem with the proposed patch is that there is no
>> declaration for attr_start() before the call in compat/mingw.c. We
>> would have to move the declaration of attr_start() to cache.h (for
>> example), because #including attr.h in compat/mingw.c is plainly
>> wrong. However, it would not be a major offense to #include attr.h in
>> common-main.c. But when we do that, we can certainly spare the few
>> cycles to call pthread_mutex_init.
>
> That sounds like a good argument to have it in common-main.c.
If we're going that route, I would get rid of PTHREAD_MUTEX_INITILIZER
and call a pthread_mutex_init platform independently.
>
> Would it mean that the code that defines the mutex needs to have
> #ifdef that defines a no-op attr_start() and defines the mutex with
> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex
> without initializatin, with the real attr_start(), though?
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-28 20:38 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Sixt, Jacob Keller, Johannes Schindelin,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <CAGZ79kZVivquVDH9S63K8p3bOis+e=b3cKVVrg13zPe_BrL_Sw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Fri, Oct 28, 2016 at 1:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Another problem with the proposed patch is that there is no
>>> declaration for attr_start() before the call in compat/mingw.c. We
>>> would have to move the declaration of attr_start() to cache.h (for
>>> example), because #including attr.h in compat/mingw.c is plainly
>>> wrong. However, it would not be a major offense to #include attr.h in
>>> common-main.c. But when we do that, we can certainly spare the few
>>> cycles to call pthread_mutex_init.
>>
>> That sounds like a good argument to have it in common-main.c.
>
> If we're going that route, I would get rid of PTHREAD_MUTEX_INITILIZER
> and call a pthread_mutex_init platform independently.
Yup, but earlier j6t was trying hard not to penalize any single
platform, and that would certainly penalize the ones with static
initialization, so I was hoping to hear if there is a clever
workaround to avoid that.
>> Would it mean that the code that defines the mutex needs to have
>> #ifdef that defines a no-op attr_start() and defines the mutex with
>> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex
>> without initializatin, with the real attr_start(), though?
^ permalink raw reply
* Can't get git to stop outputting to StdErr
From: Scott R. Chamberlain @ 2016-10-28 20:31 UTC (permalink / raw)
To: git@vger.kernel.org
I am working on some build scripts that get run on TFS. During the build process I need to check in the changes that where done during the build process (A set of binaries other projects depend on).
I would really like to leave the option "Fail on Standard Error" enabled for the script in TFS, however my push keeps writing to standard error even though I told it not to.
The line I do is:
git push -q binaryRepo HEAD:"$Env:BUILD_SOURCEBRANCH"
But I get the following in my log after the build
2016-10-28T20:05:32.3179442Z ##[error]remote:
remote: Analyzing objects... (3/3) (657 ms)
remote: Storing packfile... done (40 ms)
remote: Storing index... done (42 ms)
2016-10-28T20:05:32.3209423Z Done
2016-10-28T20:05:32.4019436Z ##[error]Process completed with exit code 0 and had 1 error(s) written to the error stream.
2016-10-28T20:05:32.4029436Z ##[debug]System.Exception: Process completed with exit code 0 and had 1 error(s) written to the error stream.
Why am I still getting output to standard error when I included the `-q` switch?
For reference, `git version` reports `2.10.0.windows.1` and HEAD is a detached HEAD.
Scott Chamberlain
Software Engineer
ImproMed, LLC
--
Rely On Us.
ImproMed LLC
Henry Schein Animal Health
--
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-28 20:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jacob Keller, Johannes Schindelin, Stefan Beller,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <xmqqr370z07v.fsf@gitster.mtv.corp.google.com>
Am 28.10.2016 um 22:29 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Another problem with the proposed patch is that there is no
>> declaration for attr_start() before the call in compat/mingw.c. We
>> would have to move the declaration of attr_start() to cache.h (for
>> example), because #including attr.h in compat/mingw.c is plainly
>> wrong. However, it would not be a major offense to #include attr.h in
>> common-main.c. But when we do that, we can certainly spare the few
>> cycles to call pthread_mutex_init.
>
> That sounds like a good argument to have it in common-main.c.
>
> Would it mean that the code that defines the mutex needs to have
> #ifdef that defines a no-op attr_start() and defines the mutex with
> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex
> without initializatin, with the real attr_start(), though?
No. My intent was to call pthread_mutex_init for all platforms. We
already call open("/dev/null") unconditionally on every program startup.
The few cycles spent in pthread_mutex_init should be the least of our
worries.
-- Hannes
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Junio C Hamano @ 2016-10-28 20:52 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jacob Keller, Johannes Schindelin, Stefan Beller,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <ab4c8148-2124-f08d-a770-e4f4e49a1c15@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 28.10.2016 um 22:29 schrieb Junio C Hamano:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>
>>> Another problem with the proposed patch is that there is no
>>> declaration for attr_start() before the call in compat/mingw.c. We
>>> would have to move the declaration of attr_start() to cache.h (for
>>> example), because #including attr.h in compat/mingw.c is plainly
>>> wrong. However, it would not be a major offense to #include attr.h in
>>> common-main.c. But when we do that, we can certainly spare the few
>>> cycles to call pthread_mutex_init.
>>
>> That sounds like a good argument to have it in common-main.c.
>>
>> Would it mean that the code that defines the mutex needs to have
>> #ifdef that defines a no-op attr_start() and defines the mutex with
>> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex
>> without initializatin, with the real attr_start(), though?
>
> No. My intent was to call pthread_mutex_init for all platforms.
Ah, OK, so there was no magic plan. That's OK.
^ permalink raw reply
* Re: Can't get git to stop outputting to StdErr
From: Junio C Hamano @ 2016-10-28 21:06 UTC (permalink / raw)
To: Scott R. Chamberlain; +Cc: git@vger.kernel.org
In-Reply-To: <c09d32d8ab97418d98ddf356e20a6ff5@ES4.impromed.com>
"Scott R. Chamberlain" <srchamberlain@impromed.com> writes:
> The line I do is:
>
> git push -q binaryRepo HEAD:"$Env:BUILD_SOURCEBRANCH"
This would
(1) squelch the output from the sending side (i.e. local), and
(2) ask "quiet" to the receiving side (i.e. remote), if they know
how to be quiet.
> But I get the following in my log after the build
>
> 2016-10-28T20:05:32.3179442Z ##[error]remote:
> remote: Analyzing objects... (3/3) (657 ms)
> remote: Storing packfile... done (40 ms)
> remote: Storing index... done (42 ms)
These three lines prefixed with "remote:" are coming from the
software that runs on the remote machine that accepts your push, but
the way it says these three things do not look familiar to me. Is
it possible that the remote machine is running a Git server that is
not ours, which lacks the support for "quiet" capability? If that
is the case, the symptom is understandable.
A quick archive search tells me that you are seeing the same issue
as this one:
https://public-inbox.org/git/20160516133731.GA6903@sigill.intra.peff.net/
where the concluding remark, to which I agree, is:
The server side here is clearly not stock git, from the content
of those progress messages (some googling shows it looks like
whatever visualstudio.com is running, but I don't know what that
is). So either the server implementation doesn't support the
"quiet" protocol extension, or it is ignoring it. It might be
worth filing a bug with them.
^ permalink raw reply
* RE: Can't get git to stop outputting to StdErr
From: Scott R. Chamberlain @ 2016-10-28 21:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqq7f8syyhg.fsf@gitster.mtv.corp.google.com>
This is talking to a visualstudio.com git repo. I will file a bug with them.
Scott Chamberlain
Software Engineer
ImproMed, LLC
(800) 925-7171
www.impromed.com
-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Friday, October 28, 2016 4:07 PM
To: Scott R. Chamberlain <srchamberlain@impromed.com>
Cc: git@vger.kernel.org
Subject: Re: Can't get git to stop outputting to StdErr
"Scott R. Chamberlain" <srchamberlain@impromed.com> writes:
> The line I do is:
>
> git push -q binaryRepo HEAD:"$Env:BUILD_SOURCEBRANCH"
This would
(1) squelch the output from the sending side (i.e. local), and
(2) ask "quiet" to the receiving side (i.e. remote), if they know
how to be quiet.
> But I get the following in my log after the build
>
> 2016-10-28T20:05:32.3179442Z ##[error]remote:
> remote: Analyzing objects... (3/3) (657 ms)
> remote: Storing packfile... done (40 ms)
> remote: Storing index... done (42 ms)
These three lines prefixed with "remote:" are coming from the software that runs on the remote machine that accepts your push, but the way it says these three things do not look familiar to me. Is it possible that the remote machine is running a Git server that is not ours, which lacks the support for "quiet" capability? If that is the case, the symptom is understandable.
A quick archive search tells me that you are seeing the same issue as this one:
https://public-inbox.org/git/20160516133731.GA6903@sigill.intra.peff.net/
where the concluding remark, to which I agree, is:
The server side here is clearly not stock git, from the content
of those progress messages (some googling shows it looks like
whatever visualstudio.com is running, but I don't know what that
is). So either the server implementation doesn't support the
"quiet" protocol extension, or it is ignoring it. It might be
worth filing a bug with them.
--
Rely On Us.
ImproMed LLC
Henry Schein Animal Health
--
^ permalink raw reply
* Re: [PATCH] pre-receive.sample: Mark executable
From: Junio C Hamano @ 2016-10-28 21:19 UTC (permalink / raw)
To: Anders Kaseorg; +Cc: git
In-Reply-To: <alpine.DEB.2.10.1610281438000.60842@buzzword-bingo.mit.edu>
Anders Kaseorg <andersk@mit.edu> writes:
> For consistency with other hooks, allow the hook to be activated by
> renaming pre-receive.sample to pre-receive without a separate step to
> mark it executable.
>
> Signed-off-by: Anders Kaseorg <andersk@mit.edu>
> ---
Makes sense. Thanks.
> templates/hooks--pre-receive.sample | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> mode change 100644 => 100755 templates/hooks--pre-receive.sample
>
> diff --git a/templates/hooks--pre-receive.sample b/templates/hooks--pre-receive.sample
> old mode 100644
> new mode 100755
^ permalink raw reply
* Re: [PATCHv2 28/36] attr: keep attr stack for each check
From: Junio C Hamano @ 2016-10-28 21:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, pclouds, git
In-Reply-To: <20161028185502.8789-29-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Instead of having a global attr stack, attach the stack to each check.
> This allows to use the attr in a multithreaded way.
>
>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> attr.c | 101 +++++++++++++++++++++++++++++++++++++++-----------------------
> attr.h | 4 ++-
> hashmap.h | 2 ++
> 3 files changed, 69 insertions(+), 38 deletions(-)
This looks surprisingly simple ;-) I like it.
I briefly wondered if the addition of lock/unlock surrounding
git_check_attrs() function belongs to [27/36], but that step is not
about making things thread-safe and is primarily to prepare existing
users to use an updated API that can be made thread-safe in later
steps. This [28/36] is the step to have these---so the addition is
not out-of-space at all.
Nicely done.
As this starts to pass a fully populated check object down to the
callchain that begins at bootstrap_attr_stack(), it makes it easier
to add the per-check optimization to read and keep only the relevant
entries from the attribute files later, by passing check also to the
read_attr_from_file() function.
The "set-direction" thing is not yet thread-safe, but I am not sure
what the best way to go there offhand. It somehow feels unnecessary
to allow some thread to be going in the GIT_ATTR_CHECKIN direction
while others to be going in the GIT_ATTR_CHECKOUT direction, so we
probably can leave it at a lower priority for now.
^ permalink raw reply
* Fetch/push lets a malicious server steal the targets of "have" lines
From: Matt McCutchen @ 2016-10-28 21:39 UTC (permalink / raw)
To: git
I was studying the fetch protocol and I realized that in a scenario in
which a client regularly fetches a set of refs from a server and pushes
them back without careful scrutiny, the server can steal the targets of
unrelated refs from the client repository by fabricating its own refs
to the "have" objects specified by the client during the fetch. This
is the reverse of attack #1 described in the "SECURITY" section of the
gitnamespaces(7) man page, with the addition that the server doesn't
have to know the object IDs in advance. Is this supposed to be well-
known? I've been using git since 2006 and it was a surprise to me.
Hopefully it isn't very common for a user to fetch and push with a
server they don't trust to have all the data in their repository. I
don't think I have any such cases myself; I have unfinished work that
isn't meant for scrutiny by others, but nothing really damaging if it
were released to the server. This attack presents no new risks if a
user already runs code fetched from the server in such a way that it
can read the repository. But there might be some users who just review
embargoed security fixes from multiple sources (or something like that)
without running code themselves, and their security expectations might
be violated.
If my analysis is correct, I'd argue for documenting the issue in a
"SECURITY" section in the git-fetch man page. Shall I submit a patch?
Thanks for your attention.
Matt
^ permalink raw reply
* Re: [PATCHv2 00/36] Revamp the attr subsystem!
From: Junio C Hamano @ 2016-10-28 21:43 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, pclouds, git
In-Reply-To: <20161028185502.8789-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> previous discussion at https://public-inbox.org/git/20161022233225.8883-1-sbeller@google.com
>
> This implements the discarded series':
> jc/attr
> jc/attr-more
> sb/pathspec-label
> sb/submodule-default-paths
>
> This includes
> * The fixes for windows
> * Junios latest suggestion to use git_attr_check_initv instead of
> alloc/append.
>
> * I implemented the thread safe attr API in patch 27 (attr: convert to new threadsafe API)
> * patch 28 (attr: keep attr stack for each check) makes it actually possible
> to run in a multithreaded environment.
> * I added a test for the multithreaded when it is introduced in patch 32
> (pathspec: allow querying for attributes) as well as a test to disallow
> multiple "attr"s in a pathspec.
I'd appreciate if you didn't unnecessarily rebase the series. It
would make comparing the new round with the previous one a lot
easier.
Thanks.
^ permalink raw reply
* Re: Fetch/push lets a malicious server steal the targets of "have" lines
From: Junio C Hamano @ 2016-10-28 22:00 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1477690790.2904.22.camel@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
> I was studying the fetch protocol and I realized that in a scenario in
> which a client regularly fetches a set of refs from a server and pushes
> them back without careful scrutiny, the server can steal the targets of
> unrelated refs from the client repository by fabricating its own refs
> to the "have" objects specified by the client during the fetch.
Let me see if I understood your scenario correctly.
Suppose we start from this history where 'O' are common, your victim
has a 'Y' branch with two commits that are private to it, as well as
a 'X' branch on which it has X1 that it previously obtained from the
server. On the other hand, the server does not know about Y1 or Y2,
and it added one commit X2 to the branch 'x' the victim is
following:
victim server
Y1---Y2
/
---O---O---X1 ---O---O---X1---X2
Then when victim wants to fetch 'x' from the server, it would say
have X1, have Y2, have Y1, have O
and gets told to shut up by the server who heard enough. The
histories on these two parties will then become like this:
victim server
Y1---Y2
/
---O---O---X1---X2 ---O---O---X1---X2
Victim wishes to keep Y1 and Y2 private, but pushes some other
branch (perhaps builds X3 on top of X2 and pushes 'x'). On push
protocol, the server would lie to the victim that it has Y2 without
knowing what they are.
Is that how your attack scenario goes?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox