* [PATCH 26/35] attr: convert to new threadsafe API
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
This revamps the API of the attr subsystem to be thread safe.
Before we had the question and its results in one struct type.
The typical usage of the API was
static struct git_attr_check *check;
if (!check)
check = git_attr_check_initl("text", NULL);
git_check_attr(path, check);
act_on(check->value[0]);
This has a couple of issues when it comes to thread safety:
* the initialization is racy in this implementation. To make it
thread safe, we need to acquire a mutex, such that only one
thread is executing the code in git_attr_check_initl.
As we do not want to introduce a mutex at each call site,
this is best done in the attr code. However to do so, we need
to have access to the `check` variable, i.e. the API has to
look like
git_attr_check_initl(struct git_attr_check**, ...);
Then one of the threads calling git_attr_check_initl will
acquire the mutex and init the `check`, while all other threads
will wait on the mutex just to realize they're late to the
party and they'll return with no work done.
* While the check for attributes to be questioned only need to
be initalized once as that part will be read only after its
initialisation, the answer may be different for each path.
Because of that we need to decouple the check and the answer,
such that each thread can obtain an answer for the path it
is currently processing.
This commit changes the API and adds locking in
git_attr_check_initl that provides the thread safety for constructing
`struct git_attr_check`.
The usage of the new API will be:
/*
* The initl call will thread-safely check whether the
* struct git_attr_check has been initialized. We only
* want to do the initialization work once, hence we do
* that work inside a thread safe environment.
*/
static struct git_attr_check *check;
git_attr_check_initl(&check, "text", NULL);
/* We're just asking for one attribute "text". */
git_attr_result myresult[1];
/* Perform the check and act on it: */
git_check_attr(path, check, myresult);
act_on(myresult[0].value);
/*
* No need to free the check as it is static, hence doesn't leak
* memory. The result is also static, so no need to free there either.
*/
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
# Conflicts:
# attr.h
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-gitattributes.txt | 136 +++++++++++---------
archive.c | 11 +-
attr.c | 172 +++++++++++++++++---------
attr.h | 72 ++++++-----
builtin/check-attr.c | 50 ++++----
builtin/pack-objects.c | 16 +--
common-main.c | 2 +
convert.c | 40 +++---
ll-merge.c | 24 ++--
userdiff.c | 16 +--
ws.c | 8 +-
11 files changed, 323 insertions(+), 224 deletions(-)
diff --git a/Documentation/technical/api-gitattributes.txt b/Documentation/technical/api-gitattributes.txt
index 92fc32a..7dd6616 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -10,21 +10,23 @@ Data Structure
`struct git_attr`::
- An attribute is an opaque object that is identified by its name.
- Pass the name to `git_attr()` function to obtain the object of
- this type. The internal representation of this structure is
+ The internal representation of this structure is
of no interest to the calling programs. The name of the
attribute can be retrieved by calling `git_attr_name()`.
-`struct git_attr_check_elem`::
-
- This structure represents one attribute and its value.
-
`struct git_attr_check`::
- This structure represents a collection of `git_attr_check_elem`.
+ This structure represents a collection of `struct git_attrs`.
It is passed to `git_check_attr()` function, specifying the
- attributes to check, and receives their values.
+ attributes to check, and receives their values into a corresponding
+ `struct git_attr_result`.
+
+`struct git_attr_result`::
+
+ This structure represents one results for a check, such that an
+ array of `struct git_attr_results` corresponds to a
+ `struct git_attr_check`. The answers given in that array are in
+ the the same order as the check struct.
Attribute Values
@@ -32,7 +34,7 @@ Attribute Values
An attribute for a path can be in one of four states: Set, Unset,
Unspecified or set to a string, and `.value` member of `struct
-git_attr_check` records it. There are three macros to check these:
+git_attr_result` records it. There are three macros to check these:
`ATTR_TRUE()`::
@@ -53,19 +55,28 @@ value of the attribute for the path.
Querying Specific Attributes
----------------------------
-* Prepare `struct git_attr_check` using git_attr_check_initl()
- function, enumerating the names of attributes whose values you are
- interested in, terminated with a NULL pointer. Alternatively, an
- empty `struct git_attr_check` can be prepared by calling
- `git_attr_check_alloc()` function and then attributes you want to
- ask about can be added to it with `git_attr_check_append()`
- function.
-
-* Call `git_check_attr()` to check the attributes for the path.
-
-* Inspect `git_attr_check` structure to see how each of the
- attribute in the array is defined for the path.
-
+* Prepare a `struct git_attr_check` using `git_attr_check_initl()` or
+ `git_attr_check_initv()` function, enumerating the names of
+ attributes whose values you are interested in, terminated with
+ a NULL.
+ Preparing the `struct git_attr_check` is thread safe, i.e. you
+ can call it from different threads at the same time; when check
+ determines the initialization is still needed, the threads will
+ use a single global mutex to perform the initialization just once,
+ the others will wait on the the thread to actually perform the
+ initialization.
+
+* Allocate an array of `struct git_attr_result` either on the stack
+ or via `git_attr_result_alloc` on the heap when the result size
+ is not known at compile time.
+
+* Call `git_check_attr()` to check the attributes for the path,
+ the given `git_attr_result` will be filled with the result.
+ You must not change the `struct git_attr_check` after calling
+ `git_check_attr()`.
+
+* Inspect each `git_attr_result` structure to see how
+ each of the attribute in the array is defined for the path.
Example
-------
@@ -73,31 +84,22 @@ Example
To see how attributes "crlf" and "ident" are set for different paths.
. Prepare a `struct git_attr_check` with two elements (because
- we are checking two attributes):
-
-------------
-static struct git_attr_check *check;
-static void setup_check(void)
-{
- if (check)
- return; /* already done */
- check = git_attr_check_initl("crlf", "ident", NULL);
-}
-------------
-
-. Call `git_check_attr()` with the prepared `struct git_attr_check`:
+ we are checking two attributes) and then call `git_check_attr()`
+ with the prepared `struct git_attr_check`:
------------
const char *path;
+ struct git_attr_result result[2];
+ static struct git_attr_check *check;
- setup_check();
- git_check_attr(path, check);
+ git_attr_check_initl(&check, "crlf", "ident", NULL);
+ git_check_attr(path, check, result);
------------
-. Act on `.value` member of the result, left in `check->check[]`:
+. Act on `result.value[]`:
------------
- const char *value = check->check[0].value;
+ const char *value = result.value[0];
if (ATTR_TRUE(value)) {
The attribute is Set, by listing only the name of the
@@ -116,19 +118,28 @@ static void setup_check(void)
}
------------
-To see how attributes in argv[] are set for different paths, only
-the first step in the above would be different.
+To see how attributes in argv[] are set for different paths, use
+`git_attr_check_initv` instead. When an invalid attr name is given
+both init functions die unconditionally with a non user friendly message.
+So you want to check the attr values first:
------------
static struct git_attr_check *check;
static void setup_check(const char **argv)
{
- check = git_attr_check_alloc();
- while (*argv) {
- struct git_attr *attr = git_attr(*argv);
- git_attr_check_append(check, attr);
- argv++;
+ const char **argv_copy = argv;
+ while (*argv_copy) {
+ const char *name = argv_copy[0];
+ const int len = strlen(argv_copy[0]);
+ if (!attr_name_valid(name, len)) {
+ struct strbuf sb = STRBUF_INIT;
+ invalid_attr_name_message(&sb, name, len);
+ die ("Oops this is bad: %s", sb.buf);
+ }
+ argv_copy++;
}
+ git_attr_check_initv(&check, argv);
+ struct git_attr_result *result = git_attr_result_alloc(check);
}
------------
@@ -138,17 +149,30 @@ Querying All Attributes
To get the values of all attributes associated with a file:
-* Prepare an empty `git_attr_check` structure by calling
- `git_attr_check_alloc()`.
+* Setup a local variables for the question
+ `struct git_attr_check` as well as a pointer where the result
+ `struct git_attr_result` will be stored. Both should be initialized
+ to NULL.
+
+------------
+ struct git_attr_check *check = NULL;
+ struct git_attr_result *result = NULL;
+------------
+
+* Call `git_all_attrs()`.
-* Call `git_all_attrs()`, which populates the `git_attr_check`
- with the attributes attached to the path.
+------------
+ git_all_attrs(full_path, &check, &result);
+------------
-* Iterate over the `git_attr_check.check[]` array to examine
- the attribute names and values. The name of the attribute
- described by a `git_attr_check.check[]` object can be retrieved via
- `git_attr_name(check->check[i].attr)`. (Please note that no items
+* Iterate over the `git_attr_check.attr[]` array to examine the
+ attribute names. The name of the attribute described by a
+ `git_attr_check.attr[]` object can be retrieved via
+ `git_attr_name(check->attr[i])`. (Please note that no items
will be returned for unset attributes, so `ATTR_UNSET()` will return
false for all returned `git_array_check` objects.)
+ The respective value for an attribute can be found in the same
+ index position in of `git_attr_result`.
-* Free the `git_array_check` by calling `git_attr_check_free()`.
+* Clear the variables by calling `git_attr_check_clear()` and
+ `git_attr_result_free()`.
diff --git a/archive.c b/archive.c
index 11e3951..65bc6b7 100644
--- a/archive.c
+++ b/archive.c
@@ -111,6 +111,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
struct archiver_args *args = c->args;
write_archive_entry_fn_t write_entry = c->write_entry;
static struct git_attr_check *check;
+ struct git_attr_result result[2];
const char *path_without_prefix;
int err;
@@ -124,12 +125,12 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
strbuf_addch(&path, '/');
path_without_prefix = path.buf + args->baselen;
- 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))
+ git_attr_check_initl(&check, "export-ignore", "export-subst", NULL);
+
+ if (!git_check_attr(path_without_prefix, check, result)) {
+ if (ATTR_TRUE(result[0].value))
return 0;
- args->convert = ATTR_TRUE(check->check[1].value);
+ args->convert = ATTR_TRUE(result[1].value);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
diff --git a/attr.c b/attr.c
index 19a3651..32ec453 100644
--- a/attr.c
+++ b/attr.c
@@ -14,6 +14,8 @@
#include "dir.h"
#include "utf8.h"
#include "quote.h"
+#include "thread-utils.h"
+#include "argv-array.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -46,6 +48,21 @@ struct git_attr {
static int attr_nr;
static struct git_attr *(git_attr_hash[HASHSIZE]);
+#ifndef NO_PTHREADS
+
+static pthread_mutex_t attr_mutex;
+#define attr_lock() pthread_mutex_lock(&attr_mutex)
+#define attr_unlock() pthread_mutex_unlock(&attr_mutex)
+void attr_start(void) { pthread_mutex_init(&attr_mutex, NULL); }
+
+#else
+
+#define attr_lock() (void)0
+#define attr_unlock() (void)0
+void attr_start(void) {;}
+
+#endif /* NO_PTHREADS */
+
/*
* NEEDSWORK: maybe-real, maybe-macro are not property of
* an attribute, as it depends on what .gitattributes are
@@ -55,6 +72,16 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);
*/
static int cannot_trust_maybe_real;
+/*
+ * Send one or more git_attr_check to git_check_attrs(), and
+ * each 'value' member tells what its value is.
+ * Unset one is returned as NULL.
+ */
+struct git_attr_check_elem {
+ const struct git_attr *attr;
+ const char *value;
+};
+
/* NEEDSWORK: This will become per git_attr_check */
static struct git_attr_check_elem *check_all_attr;
@@ -781,7 +808,7 @@ static int macroexpand_one(int nr, int rem)
static int attr_check_is_dynamic(const struct git_attr_check *check)
{
- return (void *)(check->check) != (void *)(check + 1);
+ return (void *)(check->attr) != (void *)(check + 1);
}
static void empty_attr_check_elems(struct git_attr_check *check)
@@ -799,7 +826,9 @@ static void empty_attr_check_elems(struct git_attr_check *check)
* 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, int collect_all)
+ struct git_attr_check *check,
+ struct git_attr_result **result,
+ int collect_all)
{
struct attr_stack *stk;
@@ -825,13 +854,11 @@ static void collect_some_attrs(const char *path, int pathlen,
check_all_attr[i].value = ATTR__UNKNOWN;
if (!collect_all && !cannot_trust_maybe_real) {
- struct git_attr_check_elem *celem = check->check;
-
rem = 0;
for (i = 0; i < check->check_nr; i++) {
- if (!celem[i].attr->maybe_real) {
+ if (!check->attr[i]->maybe_real) {
struct git_attr_check_elem *c;
- c = check_all_attr + celem[i].attr->attr_nr;
+ c = check_all_attr + check->attr[i]->attr_nr;
c->value = ATTR__UNSET;
rem++;
}
@@ -845,38 +872,53 @@ static void collect_some_attrs(const char *path, int pathlen,
rem = fill(path, pathlen, basename_offset, stk, rem);
if (collect_all) {
- empty_attr_check_elems(check);
+ int check_nr = 0, check_alloc = 0;
+ const char **res = NULL;
+
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;
+
+ ALLOC_GROW(check->attr, check->check_nr + 1, check->check_alloc);
+ check->attr[check->check_nr++] = attr;
+
+ ALLOC_GROW(res, check_nr + 1, check_alloc);
+ res[check_nr++] = value;
}
+
+ *result = git_attr_result_alloc(check);
+ for (i = 0; i < check->check_nr; i++)
+ (*result)[i].value = res[i];
+
+ free(res);
}
}
static int git_check_attrs(const char *path, int pathlen,
- struct git_attr_check *check)
+ struct git_attr_check *check,
+ struct git_attr_result *result)
{
int i;
- struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, check, 0);
+ collect_some_attrs(path, pathlen, check, &result, 0);
for (i = 0; i < check->check_nr; i++) {
- const char *value = check_all_attr[celem[i].attr->attr_nr].value;
+ const char *value = check_all_attr[check->attr[i]->attr_nr].value;
if (value == ATTR__UNKNOWN)
value = ATTR__UNSET;
- celem[i].value = value;
+ result[i].value = value;
}
return 0;
}
-void git_all_attrs(const char *path, struct git_attr_check *check)
+void git_all_attrs(const char *path,
+ struct git_attr_check *check,
+ struct git_attr_result **result)
{
- collect_some_attrs(path, strlen(path), check, 1);
+ collect_some_attrs(path, strlen(path), check, result, 1);
}
void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
@@ -892,69 +934,79 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
use_index = istate;
}
-static int git_check_attr_counted(const char *path, int pathlen,
- struct git_attr_check *check)
+int git_check_attr(const char *path,
+ struct git_attr_check *check,
+ struct git_attr_result *result)
{
check->finalized = 1;
- return git_check_attrs(path, pathlen, check);
+ return git_check_attrs(path, strlen(path), check, result);
}
-int git_check_attr(const char *path, struct git_attr_check *check)
+static void git_attr_check_initv_unlocked(struct git_attr_check **check_,
+ const char **argv)
{
- return git_check_attr_counted(path, strlen(path), check);
+ int cnt;
+ struct git_attr_check *check;
+
+ for (cnt = 0; argv[cnt] != NULL; cnt++)
+ ;
+
+ check = xcalloc(1, sizeof(*check) + cnt * sizeof(*(check->attr)));
+ check->check_nr = cnt;
+ check->finalized = 1;
+ check->attr = (const struct git_attr **)(check + 1);
+
+ for (cnt = 0; cnt < check->check_nr; cnt++) {
+ struct git_attr *attr = git_attr(argv[cnt]);
+ if (!attr)
+ die("BUG: %s: not a valid attribute name", argv[cnt]);
+ check->attr[cnt] = attr;
+ }
+
+ *check_ = check;
}
-struct git_attr_check *git_attr_check_initl(const char *one, ...)
+void git_attr_check_initl(struct git_attr_check **check,
+ const char *one, ...)
{
- struct git_attr_check *check;
int cnt;
va_list params;
const char *param;
+ struct argv_array attrs = ARGV_ARRAY_INIT;
+
+ attr_lock();
+ if (*check) {
+ attr_unlock();
+ return;
+ }
va_start(params, one);
+ argv_array_push(&attrs, one);
for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
- ;
+ argv_array_push(&attrs, param);
va_end(params);
- 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);
- 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;
+ git_attr_check_initv_unlocked(check, attrs.argv);
+
+ attr_unlock();
+ argv_array_clear(&attrs);
}
-struct git_attr_check *git_attr_check_alloc(void)
+void git_attr_check_initv(struct git_attr_check **check, const char **argv)
{
- return xcalloc(1, sizeof(struct git_attr_check));
+ attr_lock();
+ if (*check) {
+ attr_unlock();
+ return;
+ }
+ git_attr_check_initv_unlocked(check, argv);
+
+ attr_unlock();
}
-struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
- const struct git_attr *attr)
+struct git_attr_result *git_attr_result_alloc(struct git_attr_check *check)
{
- 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;
+ return xcalloc(1, sizeof(struct git_attr_result) * check->check_nr);
}
void git_attr_check_clear(struct git_attr_check *check)
@@ -962,12 +1014,12 @@ void git_attr_check_clear(struct git_attr_check *check)
empty_attr_check_elems(check);
if (!attr_check_is_dynamic(check))
die("BUG: clearing a statically initialized git_attr_check");
- free(check->check);
+ free(check->attr);
check->check_alloc = 0;
}
-void git_attr_check_free(struct git_attr_check *check)
+void git_attr_result_free(struct git_attr_result *result)
{
- git_attr_check_clear(check);
- free(check);
+ /* No need to free values as they are interned. */
+ free(result);
}
diff --git a/attr.h b/attr.h
index 7416b14..1a50288 100644
--- a/attr.h
+++ b/attr.h
@@ -1,14 +1,14 @@
#ifndef ATTR_H
#define ATTR_H
-/* An attribute is a pointer to this opaque structure */
-struct git_attr;
-
/*
- * Given a string, return the gitattribute object that
- * corresponds to it.
+ * Must be called on platforms that do not support static initialization
+ * of mutexes.
*/
-struct git_attr *git_attr(const char *);
+extern void attr_start(void);
+
+/* An attribute is a pointer to this opaque structure */
+struct git_attr;
/*
* Return the name of the attribute represented by the argument. The
@@ -29,44 +29,48 @@ extern const char git_attr__false[];
#define ATTR_FALSE(v) ((v) == git_attr__false)
#define ATTR_UNSET(v) ((v) == NULL)
-/*
- * Send one or more git_attr_check to git_check_attrs(), and
- * each 'value' member tells what its value is.
- * Unset one is returned as NULL.
- */
-struct git_attr_check_elem {
- const struct git_attr *attr;
- const char *value;
-};
-
struct git_attr_check {
int finalized;
int check_nr;
int check_alloc;
- struct git_attr_check_elem *check;
+ const struct git_attr **attr;
};
+#define GIT_ATTR_CHECK_INIT {0, 0, 0, NULL}
-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 *);
+struct git_attr_result {
+ const char *value;
+};
/*
- * 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.
+ * Initialize the `git_attr_check` via one of the following three functions:
+ *
+ * git_all_attrs allocates a check and fills in all attributes and
+ * results that are set for the given path.
+ * git_attr_check_initl takes a pointer to where the check will be initialized,
+ * followed by all attributes that are to be checked.
+ * git_attr_check_initv takes a pointer to where the check will be initialized,
+ * and a NULL terminated array of attributes.
+ *
+ * All initialization methods are thread safe.
+ * To allocate memory for the result of a given check,
+ * use git_attr_result_alloc.
*/
-extern const char *git_attr_name(const struct git_attr *);
+extern void git_attr_check_initl(struct git_attr_check **,
+ const char *, ...);
+extern void git_attr_check_initv(struct git_attr_check **,
+ const char **);
+extern void git_all_attrs(const char *path,
+ struct git_attr_check *,
+ struct git_attr_result **);
+extern struct git_attr_result *git_attr_result_alloc(struct git_attr_check *check);
-/*
- * Retrieve all attributes that apply to the specified path.
- * check holds the attributes and their values.
- */
-void git_all_attrs(const char *path, struct git_attr_check *check);
+/* Query a path for its attributes */
+extern int git_check_attr(const char *path,
+ struct git_attr_check *,
+ struct git_attr_result *result);
+
+extern void git_attr_check_clear(struct git_attr_check *);
+extern void git_attr_result_free(struct git_attr_result *);
enum git_attr_direction {
GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index ec61476..418c3da 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -3,6 +3,7 @@
#include "attr.h"
#include "quote.h"
#include "parse-options.h"
+#include "argv-array.h"
static int all_attrs;
static int cached_attrs;
@@ -24,13 +25,14 @@ static const struct option check_attr_options[] = {
OPT_END()
};
-static void output_attr(struct git_attr_check *check, const char *file)
+static void output_attr(struct git_attr_check *check,
+ struct git_attr_result *result, const char *file)
{
int j;
int cnt = check->check_nr;
for (j = 0; j < cnt; j++) {
- const char *value = check->check[j].value;
+ const char *value = result[j].value;
if (ATTR_TRUE(value))
value = "set";
@@ -44,11 +46,11 @@ static void output_attr(struct git_attr_check *check, const char *file)
"%s%c" /* attrname */
"%s%c" /* attrvalue */,
file, 0,
- git_attr_name(check->check[j].attr), 0, value, 0);
+ git_attr_name(check->attr[j]), 0, value, 0);
} else {
quote_c_style(file, NULL, stdout, 0);
printf(": %s: %s\n",
- git_attr_name(check->check[j].attr), value);
+ git_attr_name(check->attr[j]), value);
}
}
}
@@ -59,16 +61,20 @@ static void check_attr(const char *prefix,
{
char *full_path =
prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
+ struct git_attr_check local_check = GIT_ATTR_CHECK_INIT;
+ struct git_attr_result *result = NULL;
+
if (check != NULL) {
- if (git_check_attr(full_path, check))
- die("git_check_attr died");
- output_attr(check, file);
+ result = git_attr_result_alloc(check);
+ git_check_attr(full_path, check, result);
} else {
- check = git_attr_check_alloc();
- git_all_attrs(full_path, check);
- output_attr(check, file);
- git_attr_check_free(check);
+ git_all_attrs(full_path, &local_check, &result);
+ check = &local_check;
}
+ output_attr(check, result, file);
+ git_attr_check_clear(&local_check);
+
+ git_attr_result_free(result);
free(full_path);
}
@@ -102,7 +108,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 *check;
+ struct git_attr_check *check = NULL;
int cnt, i, doubledash, filei;
if (!is_bare_repository())
@@ -162,17 +168,19 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
error_with_usage("No file specified");
}
- if (all_attrs) {
- check = NULL;
- } else {
- check = git_attr_check_alloc();
+ if (!all_attrs) {
+ struct argv_array attrs = ARGV_ARRAY_INIT;
for (i = 0; i < cnt; i++) {
- struct git_attr *a = git_attr(argv[i]);
- if (!a)
- return error("%s: not a valid attribute name",
- argv[i]);
- git_attr_check_append(check, a);
+ if (!attr_name_valid(argv[i], strlen(argv[i]))) {
+ struct strbuf sb = STRBUF_INIT;
+ invalid_attr_name_message(&sb, argv[i],
+ strlen(argv[i]));
+ return error("%s", strbuf_detach(&sb, NULL));
+ }
+ argv_array_push(&attrs, argv[i]);
}
+ git_attr_check_initv(&check, attrs.argv);
+ argv_array_clear(&attrs);
}
if (stdin_paths)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3918c07..3751836 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -899,14 +899,16 @@ static void write_pack_file(void)
static int no_try_delta(const char *path)
{
static struct git_attr_check *check;
+ int ret = 0;
+ struct git_attr_result result[1];
- if (!check)
- check = git_attr_check_initl("delta", NULL);
- if (git_check_attr(path, check))
- return 0;
- if (ATTR_FALSE(check->check[0].value))
- return 1;
- return 0;
+ git_attr_check_initl(&check, "delta", NULL);
+
+ if (!git_check_attr(path, check, result)) {
+ if (ATTR_FALSE(result[0].value))
+ ret = 1;
+ }
+ return ret;
}
/*
diff --git a/common-main.c b/common-main.c
index 44a29e8..d4699cd 100644
--- a/common-main.c
+++ b/common-main.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "exec_cmd.h"
+#include "attr.h"
/*
* Many parts of Git have subprograms communicate via pipe, expect the
@@ -32,6 +33,7 @@ int main(int argc, const char **argv)
sanitize_stdfds();
git_setup_gettext();
+ attr_start();
argv[0] = git_extract_argv0_path(argv[0]);
diff --git a/convert.c b/convert.c
index bb2435a..fe861ce 100644
--- a/convert.c
+++ b/convert.c
@@ -718,9 +718,9 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-static enum crlf_action git_path_check_crlf(struct git_attr_check_elem *check)
+static enum crlf_action git_path_check_crlf(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
if (ATTR_TRUE(value))
return CRLF_TEXT;
@@ -735,9 +735,9 @@ static enum crlf_action git_path_check_crlf(struct git_attr_check_elem *check)
return CRLF_UNDEFINED;
}
-static enum eol git_path_check_eol(struct git_attr_check_elem *check)
+static enum eol git_path_check_eol(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
if (ATTR_UNSET(value))
;
@@ -748,9 +748,9 @@ static enum eol git_path_check_eol(struct git_attr_check_elem *check)
return EOL_UNSET;
}
-static struct convert_driver *git_path_check_convert(struct git_attr_check_elem *check)
+static struct convert_driver *git_path_check_convert(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
struct convert_driver *drv;
if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))
@@ -761,9 +761,9 @@ static struct convert_driver *git_path_check_convert(struct git_attr_check_elem
return NULL;
}
-static int git_path_check_ident(struct git_attr_check_elem *check)
+static int git_path_check_ident(struct git_attr_result *result)
{
- const char *value = check->value;
+ const char *value = result->value;
return !!ATTR_TRUE(value);
}
@@ -778,25 +778,27 @@ struct conv_attrs {
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
static struct git_attr_check *check;
+ static int init_user_convert_tail;
+ struct git_attr_result result[5];
- if (!check) {
- check = git_attr_check_initl("crlf", "ident",
- "filter", "eol", "text",
- NULL);
+ git_attr_check_initl(&check, "crlf", "ident", "filter",
+ "eol", "text", NULL);
+
+ if (!init_user_convert_tail) {
user_convert_tail = &user_convert;
git_config(read_convert_config, NULL);
+ init_user_convert_tail = 1;
}
- if (!git_check_attr(path, check)) {
- struct git_attr_check_elem *ccheck = check->check;
- ca->crlf_action = git_path_check_crlf(ccheck + 4);
+ if (!git_check_attr(path, check, result)) {
+ ca->crlf_action = git_path_check_crlf(&result[4]);
if (ca->crlf_action == CRLF_UNDEFINED)
- ca->crlf_action = git_path_check_crlf(ccheck + 0);
+ ca->crlf_action = git_path_check_crlf(&result[0]);
ca->attr_action = ca->crlf_action;
- ca->ident = git_path_check_ident(ccheck + 1);
- ca->drv = git_path_check_convert(ccheck + 2);
+ ca->ident = git_path_check_ident(&result[1]);
+ ca->drv = git_path_check_convert(&result[2]);
if (ca->crlf_action != CRLF_BINARY) {
- enum eol eol_attr = git_path_check_eol(ccheck + 3);
+ enum eol eol_attr = git_path_check_eol(&result[3]);
if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_LF)
ca->crlf_action = CRLF_AUTO_INPUT;
else if (ca->crlf_action == CRLF_AUTO && eol_attr == EOL_CRLF)
diff --git a/ll-merge.c b/ll-merge.c
index bc6479c..d71354a 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -353,12 +353,14 @@ 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 *check;
static const struct ll_merge_options default_opts;
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
const struct ll_merge_driver *driver;
+ static struct git_attr_check *check;
+ struct git_attr_result result[2];
+
if (!opts)
opts = &default_opts;
@@ -368,13 +370,12 @@ int ll_merge(mmbuffer_t *result_buf,
normalize_file(theirs, path);
}
- if (!check)
- check = git_attr_check_initl("merge", "conflict-marker-size", NULL);
+ git_attr_check_initl(&check, "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 (!git_check_attr(path, check, result)) {
+ ll_driver_name = result[0].value;
+ if (result[1].value) {
+ marker_size = atoi(result[1].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
@@ -395,11 +396,12 @@ int ll_merge_marker_size(const char *path)
{
static struct git_attr_check *check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
+ struct git_attr_result result[1];
+
+ git_attr_check_initl(&check, "conflict-marker-size", NULL);
- 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 (!git_check_attr(path, check, result) && !ATTR_UNSET(result[0].value)) {
+ marker_size = atoi(result[0].value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
}
diff --git a/userdiff.c b/userdiff.c
index 46dfd32..1d6d363 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -263,21 +263,23 @@ struct userdiff_driver *userdiff_find_by_name(const char *name) {
struct userdiff_driver *userdiff_find_by_path(const char *path)
{
static struct git_attr_check *check;
+ struct git_attr_result result[1];
- if (!check)
- check = git_attr_check_initl("diff", NULL);
if (!path)
return NULL;
- if (git_check_attr(path, check))
+
+ git_attr_check_initl(&check, "diff", NULL);
+
+ if (git_check_attr(path, check, result))
return NULL;
- if (ATTR_TRUE(check->check[0].value))
+ if (ATTR_TRUE(result[0].value))
return &driver_true;
- if (ATTR_FALSE(check->check[0].value))
+ if (ATTR_FALSE(result[0].value))
return &driver_false;
- if (ATTR_UNSET(check->check[0].value))
+ if (ATTR_UNSET(result[0].value))
return NULL;
- return userdiff_find_by_name(check->check[0].value);
+ return userdiff_find_by_name(result[0].value);
}
struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver)
diff --git a/ws.c b/ws.c
index bb3270c..7e355c4 100644
--- a/ws.c
+++ b/ws.c
@@ -74,14 +74,14 @@ unsigned parse_whitespace_rule(const char *string)
unsigned whitespace_rule(const char *pathname)
{
static struct git_attr_check *attr_whitespace_rule;
+ struct git_attr_result result[1];
- if (!attr_whitespace_rule)
- attr_whitespace_rule = git_attr_check_initl("whitespace", NULL);
+ git_attr_check_initl(&attr_whitespace_rule, "whitespace", NULL);
- if (!git_check_attr(pathname, attr_whitespace_rule)) {
+ if (!git_check_attr(pathname, attr_whitespace_rule, result)) {
const char *value;
- value = attr_whitespace_rule->check[0].value;
+ value = result[0].value;
if (ATTR_TRUE(value)) {
/* true (whitespace) */
unsigned all_rule = ws_tab_width(whitespace_rule_cfg);
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 34/35] clone: add --init-submodule=<pathspec> switch
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
The new switch passes the pathspec to `git submodule update --init`
which is called after the actual clone is done.
Additionally this configures the submodule.defaultUpdatePath to
be the given pathspec, such that any future invocation of
`git submodule update --init-default-paths` will keep up
with the pathspec.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 2 +-
Documentation/git-clone.txt | 23 +++++++++----
builtin/clone.c | 36 ++++++++++++++++++--
t/t7400-submodule-basic.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 290de3b..917904e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2887,7 +2887,7 @@ submodule.alternateErrorStrategy
submodule.defaultUpdatePath::
Specifies a set of submodules to initialize when calling
- `git submodule --init-default-group` by using the pathspec
+ `git submodule --init-default-path` by using the pathspec
syntax.
tag.forceSignAnnotated::
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 35cc34b..1089f38 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -15,7 +15,8 @@ SYNOPSIS
[--dissociate] [--separate-git-dir <git dir>]
[--depth <depth>] [--[no-]single-branch]
[--recursive | --recurse-submodules] [--[no-]shallow-submodules]
- [--jobs <n>] [--] <repository> [<directory>]
+ [--init-submodule <pathspec>] [--jobs <n>] [--]
+ <repository> [<directory>]
DESCRIPTION
-----------
@@ -217,12 +218,20 @@ objects from the source repository into a pack in the cloned repository.
--recursive::
--recurse-submodules::
- After the clone is created, initialize all submodules within,
- using their default settings. This is equivalent to running
- `git submodule update --init --recursive` immediately after
- the clone is finished. This option is ignored if the cloned
- repository does not have a worktree/checkout (i.e. if any of
- `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+ After the clone is created, initialize and clone all submodules
+ within, using their default settings. This is equivalent to
+ running `git submodule update --recursive --init `
+ immediately after the clone is finished. This option is ignored
+ if the cloned repository does not have a worktree/checkout (i.e.
+ if any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+
+--init-submodule::
+ After the clone is created, initialize and clone specified
+ submodules within, using their default settings. It is possible
+ to give multiple specifications by giving this argument multiple
+ times. This is equivalent to configure `submodule.defaultUpdateGroup`
+ and then running `git submodule update --init-default-path`
+ immediately after the clone is finished.
--[no-]shallow-submodules::
All submodules which are cloned will be shallow with a depth of 1.
diff --git a/builtin/clone.c b/builtin/clone.c
index 6c80690..ef924c6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,6 +56,16 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
static int option_dissociate;
static int max_jobs = -1;
+static struct string_list init_submodules;
+
+static int init_submodules_cb(const struct option *opt, const char *arg, int unset)
+{
+ if (unset)
+ return -1;
+
+ string_list_append((struct string_list *)opt->value, arg);
+ return 0;
+}
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -112,6 +122,9 @@ static struct option builtin_clone_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+ OPT_CALLBACK(0, "init-submodule", &init_submodules, N_("<pathspec>"),
+ N_("clone specific submodules. Pass multiple times for complex pathspecs"),
+ init_submodules_cb),
OPT_END()
};
@@ -730,13 +743,21 @@ static int checkout(int submodule_progress)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
sha1_to_hex(sha1), "1", NULL);
- if (!err && option_recursive) {
+ if (!err && (option_recursive || init_submodules.nr > 0)) {
struct argv_array args = ARGV_ARRAY_INIT;
- argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
+ argv_array_pushl(&args, "submodule", "update", NULL);
+
+ if (init_submodules.nr > 0)
+ argv_array_pushf(&args, "--init-default-path");
+ else
+ argv_array_pushf(&args, "--init");
if (option_shallow_submodules == 1)
argv_array_push(&args, "--depth=1");
+ if (option_recursive)
+ argv_array_pushf(&args, "--recursive");
+
if (max_jobs != -1)
argv_array_pushf(&args, "--jobs=%d", max_jobs);
@@ -884,6 +905,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_no_checkout = 1;
}
+ if (init_submodules.nr > 0) {
+ struct string_list_item *item;
+ struct strbuf sb = STRBUF_INIT;
+ for_each_string_list_item(item, &init_submodules) {
+ strbuf_addf(&sb, "submodule.defaultUpdatePath=%s",
+ item->string);
+ string_list_append(&option_config,
+ strbuf_detach(&sb, NULL));
+ }
+ }
+
if (!option_origin)
option_origin = "origin";
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 4699b1c..90f9030 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1170,4 +1170,85 @@ test_expect_success 'submodule update --init-default-path' '
test_cmp expect actual
'
+cat <<EOF > expected
+ sub0 (test2)
+-sub1
+-sub2
+-sub3
+EOF
+
+test_expect_success 'clone --init-submodule works' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules --init-submodule="sub0" multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp actual expected
+'
+
+cat <<EOF > expect
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+test_expect_success 'clone with multiple --init-submodule options' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules \
+ --init-submodule="." \
+ --init-submodule ":(exclude)sub0" \
+ --init-submodule ":(exclude)sub2" \
+ multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+cat <<EOF > expect
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+
+cat <<EOF > expect2
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+-sub4
+ sub5 (test2)
+EOF
+
+test_expect_success 'clone and subsequent updates correctly auto-initialize submodules' '
+ test_when_finished "rm -rf multisuper_clone" &&
+ git clone --recurse-submodules --init-submodule="." \
+ --init-submodule ":(exclude)sub0" \
+ --init-submodule ":(exclude)sub2" \
+ --init-submodule ":(exclude)sub4" \
+ multisuper multisuper_clone &&
+ (
+ cd multisuper_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual &&
+ (
+ cd multisuper &&
+ git submodule add ../sub1 sub4 &&
+ git submodule add ../sub1 sub5 &&
+ git commit -m "add more submodules"
+ ) &&
+ (
+ cd multisuper_clone &&
+ # obtain the new superproject
+ git pull &&
+ git submodule update --init-default-path &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect2 actual
+'
+
test_done
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 16/35] attr: expose validity check for attribute names
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 39 +++++++++++++++++++++++++--------------
attr.h | 10 ++++++++++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/attr.c b/attr.c
index 9bec243..14bbfd4 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);
}
static struct git_attr *git_attr_internal(const char *name, int len)
@@ -90,7 +105,7 @@ static struct git_attr *git_attr_internal(const char *name, int 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 c84f164..1c6a6d8 100644
--- a/attr.h
+++ b/attr.h
@@ -10,6 +10,16 @@ struct git_attr;
*/
struct git_attr *git_attr(const char *);
+/*
+ * 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.469.g00a8914
^ permalink raw reply related
* [PATCH 17/35] attr: support quoting pathname patterns in C style
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Full pattern must be quoted. So 'pat"t"ern attr' will give exactly
'pat"t"ern', not 'pattern'. Also clarify that leading whitespaces are
not part of the pattern and document comment syntax.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/gitattributes.txt | 8 +++++---
attr.c | 15 +++++++++++++--
t/t0003-attributes.sh | 26 ++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 7aff940..8a061af 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -21,9 +21,11 @@ Each line in `gitattributes` file is of form:
pattern attr1 attr2 ...
That is, a pattern followed by an attributes list,
-separated by whitespaces. When the pattern matches the
-path in question, the attributes listed on the line are given to
-the path.
+separated by whitespaces. Leading and trailing whitespaces are
+ignored. Lines that begin with '#' are ignored. Patterns
+that begin with a double quote are quoted in C style.
+When the pattern matches the path in question, the attributes
+listed on the line are given to the path.
Each attribute can be in one of these states for a given path:
diff --git a/attr.c b/attr.c
index 14bbfd4..88fcb7c 100644
--- a/attr.c
+++ b/attr.c
@@ -13,6 +13,7 @@
#include "attr.h"
#include "dir.h"
#include "utf8.h"
+#include "quote.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -225,12 +226,21 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
const char *cp, *name, *states;
struct match_attr *res = NULL;
int is_macro;
+ struct strbuf pattern = STRBUF_INIT;
cp = line + strspn(line, blank);
if (!*cp || *cp == '#')
return NULL;
name = cp;
- namelen = strcspn(name, blank);
+
+ if (*cp == '"' && !unquote_c_style(&pattern, name, &states)) {
+ name = pattern.buf;
+ namelen = pattern.len;
+ } else {
+ namelen = strcspn(name, blank);
+ states = name + namelen;
+ }
+
if (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&
starts_with(name, ATTRIBUTE_MACRO_PREFIX)) {
if (!macro_ok) {
@@ -250,7 +260,6 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
else
is_macro = 0;
- states = name + namelen;
states += strspn(states, blank);
/* First pass to count the attr_states */
@@ -293,9 +302,11 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
cannot_trust_maybe_real = 1;
}
+ strbuf_release(&pattern);
return res;
fail_return:
+ strbuf_release(&pattern);
free(res);
return NULL;
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index f0fbb42..f19ae4f 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -13,10 +13,31 @@ attr_check () {
test_line_count = 0 err
}
+attr_check_quote () {
+
+ path="$1"
+ quoted_path="$2"
+ expect="$3"
+
+ git check-attr test -- "$path" >actual &&
+ echo "\"$quoted_path\": test: $expect" >expect &&
+ test_cmp expect actual
+
+}
+
+test_expect_success 'open-quoted pathname' '
+ echo "\"a test=a" >.gitattributes &&
+ test_must_fail attr_check a a
+'
+
+
test_expect_success 'setup' '
mkdir -p a/b/d a/c b &&
(
echo "[attr]notest !test"
+ echo "\" d \" test=d"
+ echo " e test=e"
+ echo " e\" test=e"
echo "f test=f"
echo "a/i test=a/i"
echo "onoff test -test"
@@ -69,6 +90,11 @@ test_expect_success 'command line checks' '
'
test_expect_success 'attribute test' '
+
+ attr_check " d " d &&
+ attr_check e e &&
+ attr_check_quote e\" e\\\" e &&
+
attr_check f f &&
attr_check a/f f &&
attr_check a/c/f f &&
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 19/35] attr.c: pass struct git_attr_check down the callchain
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The callchain that starts from git_check_attrs() down to
collect_some_attrs() used to take an array of git_attr_check_elem
as their parameters. Pass the enclosing git_attr_check instance
instead, so that they will have access to new fields we will add to
the data structure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/attr.c b/attr.c
index a504cb3..219ab11 100644
--- a/attr.c
+++ b/attr.c
@@ -751,14 +751,25 @@ 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 pathlen, int num,
- struct git_attr_check_elem *check)
+static void collect_some_attrs(const char *path, int pathlen,
+ struct git_attr_check *check)
{
struct attr_stack *stk;
int i, rem, dirlen;
const char *cp, *last_slash = NULL;
int basename_offset;
+ int num;
+ struct git_attr_check_elem *celem;
+
+ if (!check) {
+ /* Yuck - ugly git_all_attrs() hack! */
+ celem = NULL;
+ num = 0;
+ } else {
+ celem = check->check;
+ num = check->check_nr;
+ }
for (cp = path; cp < path + pathlen; cp++) {
if (*cp == '/' && cp[1])
@@ -778,9 +789,9 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
if (num && !cannot_trust_maybe_real) {
rem = 0;
for (i = 0; i < num; i++) {
- if (!check[i].attr->maybe_real) {
+ if (!celem[i].attr->maybe_real) {
struct git_attr_check_elem *c;
- c = check_all_attr + check[i].attr->attr_nr;
+ c = check_all_attr + celem[i].attr->attr_nr;
c->value = ATTR__UNSET;
rem++;
}
@@ -794,18 +805,19 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-static int git_check_attrs(const char *path, int pathlen, int num,
- struct git_attr_check_elem *check)
+static int git_check_attrs(const char *path, int pathlen,
+ struct git_attr_check *check)
{
int i;
+ struct git_attr_check_elem *celem = check->check;
- collect_some_attrs(path, pathlen, num, check);
+ collect_some_attrs(path, pathlen, check);
- for (i = 0; i < num; i++) {
- const char *value = check_all_attr[check[i].attr->attr_nr].value;
+ for (i = 0; i < check->check_nr; i++) {
+ const char *value = check_all_attr[celem[i].attr->attr_nr].value;
if (value == ATTR__UNKNOWN)
value = ATTR__UNSET;
- check[i].value = value;
+ celem[i].value = value;
}
return 0;
@@ -816,7 +828,7 @@ 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), 0, NULL);
+ collect_some_attrs(path, strlen(path), NULL);
for (i = 0; i < attr_nr; i++) {
const char *name = check_all_attr[i].attr->name;
@@ -845,7 +857,7 @@ int git_check_attr_counted(const char *path, int pathlen,
struct git_attr_check *check)
{
check->finalized = 1;
- return git_check_attrs(path, pathlen, check->check_nr, check->check);
+ return git_check_attrs(path, pathlen, check);
}
int git_check_attr(const char *path, struct git_attr_check *check)
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 22/35] attr.c: introduce empty_attr_check_elems()
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
One codepath needs to just empty the git_attr_check_elem array in
the git_attr_check structure, without releasing the entire resource.
Introduce a helper to do so and rewrite git_attr_check_clear() using
it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 48460d2..5bb925d 100644
--- a/attr.c
+++ b/attr.c
@@ -746,6 +746,14 @@ static int attr_check_is_dynamic(const struct git_attr_check *check)
return (void *)(check->check) != (void *)(check + 1);
}
+static void empty_attr_check_elems(struct git_attr_check *check)
+{
+ if (!attr_check_is_dynamic(check))
+ die("BUG: emptying a statically initialized git_attr_check");
+ check->check_nr = 0;
+ check->finalized = 0;
+}
+
/*
* Collect attributes for path into the array pointed to by
* check_all_attr. If check is not NULL, only attributes in
@@ -912,12 +920,11 @@ struct git_attr_check_elem *git_attr_check_append(struct git_attr_check *check,
void git_attr_check_clear(struct git_attr_check *check)
{
+ empty_attr_check_elems(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)
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 20/35] attr.c: rename a local variable check
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Throughout this series, we are trying to use "check" to name an
instance of "git_attr_check" structure; let's rename a "check" that
refers to an array whose elements are git_attr_check_elem to avoid
confusion.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 219ab11..e18098c 100644
--- a/attr.c
+++ b/attr.c
@@ -682,12 +682,12 @@ static int macroexpand_one(int attr_nr, int rem);
static int fill_one(const char *what, struct match_attr *a, int rem)
{
- struct git_attr_check_elem *check = check_all_attr;
+ struct git_attr_check_elem *celem = check_all_attr;
int i;
for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
struct git_attr *attr = a->state[i].attr;
- const char **n = &(check[attr->attr_nr].value);
+ const char **n = &(celem[attr->attr_nr].value);
const char *v = a->state[i].setto;
if (*n == ATTR__UNKNOWN) {
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 18/35] attr.c: add push_stack() helper
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
There are too many repetitious "I have this new attr_stack element;
push it at the top of the stack" sequence. The new helper function
push_stack() gives us a way to express what is going on at these
places, and as a side effect, halves the number of times we mention
the attr_stack global variable.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 71 +++++++++++++++++++++++++++++++-----------------------------------
1 file changed, 33 insertions(+), 38 deletions(-)
diff --git a/attr.c b/attr.c
index 88fcb7c..a504cb3 100644
--- a/attr.c
+++ b/attr.c
@@ -521,6 +521,18 @@ static int git_attr_system(void)
static GIT_PATH_FUNC(git_path_info_attributes, INFOATTRIBUTES_FILE)
+static void push_stack(struct attr_stack **attr_stack_p,
+ struct attr_stack *elem, char *origin, size_t originlen)
+{
+ if (elem) {
+ elem->origin = origin;
+ if (origin)
+ elem->originlen = originlen;
+ elem->prev = *attr_stack_p;
+ *attr_stack_p = elem;
+ }
+}
+
static void bootstrap_attr_stack(void)
{
struct attr_stack *elem;
@@ -528,52 +540,35 @@ static void bootstrap_attr_stack(void)
if (attr_stack)
return;
- elem = read_attr_from_array(builtin_attr);
- elem->origin = NULL;
- elem->prev = attr_stack;
- attr_stack = elem;
-
- if (git_attr_system()) {
- elem = read_attr_from_file(git_etc_gitattributes(), 1);
- if (elem) {
- elem->origin = NULL;
- elem->prev = attr_stack;
- attr_stack = elem;
- }
- }
+ push_stack(&attr_stack, read_attr_from_array(builtin_attr), NULL, 0);
+
+ if (git_attr_system())
+ push_stack(&attr_stack,
+ read_attr_from_file(git_etc_gitattributes(), 1),
+ NULL, 0);
if (!git_attributes_file)
git_attributes_file = xdg_config_home("attributes");
- if (git_attributes_file) {
- elem = read_attr_from_file(git_attributes_file, 1);
- if (elem) {
- elem->origin = NULL;
- elem->prev = attr_stack;
- attr_stack = elem;
- }
- }
+ if (git_attributes_file)
+ push_stack(&attr_stack,
+ read_attr_from_file(git_attributes_file, 1),
+ NULL, 0);
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
elem = read_attr(GITATTRIBUTES_FILE, 1);
- elem->origin = xstrdup("");
- elem->originlen = 0;
- elem->prev = attr_stack;
- attr_stack = elem;
+ push_stack(&attr_stack, elem, xstrdup(""), 0);
debug_push(elem);
}
elem = read_attr_from_file(git_path_info_attributes(), 1);
if (!elem)
elem = xcalloc(1, sizeof(*elem));
- elem->origin = NULL;
- elem->prev = attr_stack;
- attr_stack = elem;
+ push_stack(&attr_stack, elem, NULL, 0);
}
static void prepare_attr_stack(const char *path, int dirlen)
{
struct attr_stack *elem, *info;
- int len;
const char *cp;
/*
@@ -633,20 +628,21 @@ static void prepare_attr_stack(const char *path, int dirlen)
assert(attr_stack->origin);
while (1) {
- len = strlen(attr_stack->origin);
+ size_t len = strlen(attr_stack->origin);
+ char *origin;
+
if (dirlen <= len)
break;
cp = memchr(path + len + 1, '/', dirlen - len - 1);
if (!cp)
cp = path + dirlen;
- strbuf_add(&pathbuf, path, cp - path);
- strbuf_addch(&pathbuf, '/');
- strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
+ strbuf_addf(&pathbuf,
+ "%.*s/%s", (int)(cp - path), path,
+ GITATTRIBUTES_FILE);
elem = read_attr(pathbuf.buf, 0);
strbuf_setlen(&pathbuf, cp - path);
- elem->origin = strbuf_detach(&pathbuf, &elem->originlen);
- elem->prev = attr_stack;
- attr_stack = elem;
+ origin = strbuf_detach(&pathbuf, &len);
+ push_stack(&attr_stack, elem, origin, len);
debug_push(elem);
}
@@ -656,8 +652,7 @@ static void prepare_attr_stack(const char *path, int dirlen)
/*
* Finally push the "info" one at the top of the stack.
*/
- info->prev = attr_stack;
- attr_stack = info;
+ push_stack(&attr_stack, info, NULL, 0);
}
static int path_matches(const char *pathname, int pathlen,
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 13/35] attr: convert git_check_attrs() callers to use the new API
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-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>
Signed-off-by: Junio C Hamano <gitster@pobox.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 3cb38ed..3918c07 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 c95ae71..bb2435a 100644
--- a/convert.c
+++ b/convert.c
@@ -775,24 +775,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 eb2c37e..bc6479c 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 4de3289..46dfd32 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 7350905..bb3270c 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.469.g00a8914
^ permalink raw reply related
* [PATCH 11/35] attr: (re)introduce git_check_attr() and struct git_attr_check
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-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>
Signed-off-by: Junio C Hamano <gitster@pobox.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 2dc8d6c..11e3951 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 c99e23a..861e1a2 100644
--- a/attr.c
+++ b/attr.c
@@ -829,3 +829,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 dd3c4a3..3fd8690 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.469.g00a8914
^ permalink raw reply related
* [PATCH 12/35] attr: convert git_all_attrs() to use "struct git_attr_check"
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-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>
Signed-off-by: Junio C Hamano <gitster@pobox.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 861e1a2..76f0d6b 100644
--- a/attr.c
+++ b/attr.c
@@ -724,6 +724,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
@@ -789,32 +794,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)
@@ -832,6 +826,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);
}
@@ -849,17 +844,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 3fd8690..0d94077 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 97e3837..ec61476 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.469.g00a8914
^ permalink raw reply related
* [PATCH 10/35] attr: rename function and struct related to checking attributes
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
The traditional API to check attributes is to prepare an N-element
array of "struct git_attr_check" and pass N and the array to the
function "git_check_attr()" as arguments.
In preparation to revamp the API to pass a single structure, in
which these N elements are held, rename the type used for these
individual array elements to "struct git_attr_check_elem" and rename
the function to "git_check_attrs()".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
archive.c | 6 +++---
attr.c | 12 ++++++------
attr.h | 8 ++++----
builtin/check-attr.c | 19 ++++++++++---------
builtin/pack-objects.c | 6 +++---
convert.c | 12 ++++++------
ll-merge.c | 10 +++++-----
userdiff.c | 4 ++--
ws.c | 6 +++---
9 files changed, 42 insertions(+), 41 deletions(-)
diff --git a/archive.c b/archive.c
index dde1ab4..2dc8d6c 100644
--- a/archive.c
+++ b/archive.c
@@ -87,7 +87,7 @@ void *sha1_file_to_archive(const struct archiver_args *args,
return buffer;
}
-static void setup_archive_check(struct git_attr_check *check)
+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;
@@ -123,7 +123,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 check[2];
+ struct git_attr_check_elem check[2];
const char *path_without_prefix;
int err;
@@ -138,7 +138,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
path_without_prefix = path.buf + args->baselen;
setup_archive_check(check);
- if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
+ if (!git_check_attrs(path_without_prefix, ARRAY_SIZE(check), check)) {
if (ATTR_TRUE(check[0].value))
return 0;
args->convert = ATTR_TRUE(check[1].value);
diff --git a/attr.c b/attr.c
index 1877f7a..c99e23a 100644
--- a/attr.c
+++ b/attr.c
@@ -40,7 +40,7 @@ struct git_attr {
static int attr_nr;
static int cannot_trust_maybe_real;
-static struct git_attr_check *check_all_attr;
+static struct git_attr_check_elem *check_all_attr;
static struct git_attr *(git_attr_hash[HASHSIZE]);
const char *git_attr_name(const struct git_attr *attr)
@@ -665,7 +665,7 @@ static int macroexpand_one(int attr_nr, int rem);
static int fill_one(const char *what, struct match_attr *a, int rem)
{
- struct git_attr_check *check = check_all_attr;
+ struct git_attr_check_elem *check = check_all_attr;
int i;
for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
@@ -730,7 +730,7 @@ static int macroexpand_one(int nr, int rem)
* collected. Otherwise all attributes are collected.
*/
static void collect_some_attrs(const char *path, int num,
- struct git_attr_check *check)
+ struct git_attr_check_elem *check)
{
struct attr_stack *stk;
@@ -758,7 +758,7 @@ static void collect_some_attrs(const char *path, int num,
rem = 0;
for (i = 0; i < num; i++) {
if (!check[i].attr->maybe_real) {
- struct git_attr_check *c;
+ struct git_attr_check_elem *c;
c = check_all_attr + check[i].attr->attr_nr;
c->value = ATTR__UNSET;
rem++;
@@ -773,7 +773,7 @@ static void collect_some_attrs(const char *path, int num,
rem = fill(path, pathlen, basename_offset, stk, rem);
}
-int git_check_attr(const char *path, int num, struct git_attr_check *check)
+int git_check_attrs(const char *path, int num, struct git_attr_check_elem *check)
{
int i;
@@ -789,7 +789,7 @@ int git_check_attr(const char *path, int num, struct git_attr_check *check)
return 0;
}
-int git_all_attrs(const char *path, int *num, struct git_attr_check **check)
+int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check)
{
int i, count, j;
diff --git a/attr.h b/attr.h
index 00d7a66..dd3c4a3 100644
--- a/attr.h
+++ b/attr.h
@@ -20,11 +20,11 @@ extern const char git_attr__false[];
#define ATTR_UNSET(v) ((v) == NULL)
/*
- * Send one or more git_attr_check to git_check_attr(), and
+ * Send one or more git_attr_check to git_check_attrs(), and
* each 'value' member tells what its value is.
* Unset one is returned as NULL.
*/
-struct git_attr_check {
+struct git_attr_check_elem {
const struct git_attr *attr;
const char *value;
};
@@ -36,7 +36,7 @@ struct git_attr_check {
*/
extern const char *git_attr_name(const struct git_attr *);
-int git_check_attr(const char *path, int, struct git_attr_check *);
+int git_check_attrs(const char *path, int, struct git_attr_check_elem *);
/*
* Retrieve all attributes that apply to the specified path. *num
@@ -45,7 +45,7 @@ int git_check_attr(const char *path, int, struct git_attr_check *);
* objects describing the attributes and their values. *check must be
* free()ed by the caller.
*/
-int git_all_attrs(const char *path, int *num, struct git_attr_check **check);
+int git_all_attrs(const char *path, int *num, struct git_attr_check_elem **check);
enum git_attr_direction {
GIT_ATTR_CHECKIN,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 53a5a18..97e3837 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -24,8 +24,8 @@ static const struct option check_attr_options[] = {
OPT_END()
};
-static void output_attr(int cnt, struct git_attr_check *check,
- const char *file)
+static void output_attr(int cnt, struct git_attr_check_elem *check,
+ const char *file)
{
int j;
for (j = 0; j < cnt; j++) {
@@ -51,14 +51,15 @@ static void output_attr(int cnt, struct git_attr_check *check,
}
}
-static void check_attr(const char *prefix, int cnt,
- struct git_attr_check *check, const char *file)
+static void check_attr(const char *prefix,
+ int cnt, struct git_attr_check_elem *check,
+ const char *file)
{
char *full_path =
prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
if (check != NULL) {
- if (git_check_attr(full_path, cnt, check))
- die("git_check_attr died");
+ if (git_check_attrs(full_path, cnt, check))
+ die("git_check_attrs died");
output_attr(cnt, check, file);
} else {
if (git_all_attrs(full_path, &cnt, &check))
@@ -69,8 +70,8 @@ static void check_attr(const char *prefix, int cnt,
free(full_path);
}
-static void check_attr_stdin_paths(const char *prefix, int cnt,
- struct git_attr_check *check)
+static void check_attr_stdin_paths(const char *prefix,
+ int cnt, struct git_attr_check_elem *check)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf unquoted = STRBUF_INIT;
@@ -99,7 +100,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 *check;
+ struct git_attr_check_elem *check;
int cnt, i, doubledash, filei;
if (!is_bare_repository())
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1e7c2a9..3cb38ed 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -896,7 +896,7 @@ static void write_pack_file(void)
written, nr_result);
}
-static void setup_delta_attr_check(struct git_attr_check *check)
+static void setup_delta_attr_check(struct git_attr_check_elem *check)
{
static struct git_attr *attr_delta;
@@ -908,10 +908,10 @@ static void setup_delta_attr_check(struct git_attr_check *check)
static int no_try_delta(const char *path)
{
- struct git_attr_check check[1];
+ struct git_attr_check_elem check[1];
setup_delta_attr_check(check);
- if (git_check_attr(path, ARRAY_SIZE(check), check))
+ if (git_check_attrs(path, ARRAY_SIZE(check), check))
return 0;
if (ATTR_FALSE(check->value))
return 1;
diff --git a/convert.c b/convert.c
index 077f5e6..c95ae71 100644
--- a/convert.c
+++ b/convert.c
@@ -718,7 +718,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-static enum crlf_action git_path_check_crlf(struct git_attr_check *check)
+static enum crlf_action git_path_check_crlf(struct git_attr_check_elem *check)
{
const char *value = check->value;
@@ -735,7 +735,7 @@ static enum crlf_action git_path_check_crlf(struct git_attr_check *check)
return CRLF_UNDEFINED;
}
-static enum eol git_path_check_eol(struct git_attr_check *check)
+static enum eol git_path_check_eol(struct git_attr_check_elem *check)
{
const char *value = check->value;
@@ -748,7 +748,7 @@ static enum eol git_path_check_eol(struct git_attr_check *check)
return EOL_UNSET;
}
-static struct convert_driver *git_path_check_convert(struct git_attr_check *check)
+static struct convert_driver *git_path_check_convert(struct git_attr_check_elem *check)
{
const char *value = check->value;
struct convert_driver *drv;
@@ -761,7 +761,7 @@ static struct convert_driver *git_path_check_convert(struct git_attr_check *chec
return NULL;
}
-static int git_path_check_ident(struct git_attr_check *check)
+static int git_path_check_ident(struct git_attr_check_elem *check)
{
const char *value = check->value;
@@ -783,7 +783,7 @@ static const char *conv_attr_name[] = {
static void convert_attrs(struct conv_attrs *ca, const char *path)
{
int i;
- static struct git_attr_check ccheck[NUM_CONV_ATTRS];
+ static struct git_attr_check_elem ccheck[NUM_CONV_ATTRS];
if (!ccheck[0].attr) {
for (i = 0; i < NUM_CONV_ATTRS; i++)
@@ -792,7 +792,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
git_config(read_convert_config, NULL);
}
- if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
+ if (!git_check_attrs(path, NUM_CONV_ATTRS, ccheck)) {
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 ad8be42..eb2c37e 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -336,13 +336,13 @@ 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 check[2])
+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_attr(path, 2, check);
+ return git_check_attrs(path, 2, check);
}
static void normalize_file(mmfile_t *mm, const char *path)
@@ -362,7 +362,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 check[2];
+ static struct git_attr_check_elem check[2];
static const struct ll_merge_options default_opts;
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
@@ -398,12 +398,12 @@ int ll_merge(mmbuffer_t *result_buf,
int ll_merge_marker_size(const char *path)
{
- static struct git_attr_check check;
+ static struct git_attr_check_elem check;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
if (!check.attr)
check.attr = git_attr("conflict-marker-size");
- if (!git_check_attr(path, 1, &check) && check.value) {
+ if (!git_check_attrs(path, 1, &check) && check.value) {
marker_size = atoi(check.value);
if (marker_size <= 0)
marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
diff --git a/userdiff.c b/userdiff.c
index 2125d6d..4de3289 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -263,7 +263,7 @@ 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 check;
+ struct git_attr_check_elem check;
if (!attr)
attr = git_attr("diff");
@@ -271,7 +271,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path)
if (!path)
return NULL;
- if (git_check_attr(path, 1, &check))
+ if (git_check_attrs(path, 1, &check))
return NULL;
if (ATTR_TRUE(check.value))
diff --git a/ws.c b/ws.c
index ea4b2b1..7350905 100644
--- a/ws.c
+++ b/ws.c
@@ -71,7 +71,7 @@ unsigned parse_whitespace_rule(const char *string)
return rule;
}
-static void setup_whitespace_attr_check(struct git_attr_check *check)
+static void setup_whitespace_attr_check(struct git_attr_check_elem *check)
{
static struct git_attr *attr_whitespace;
@@ -82,10 +82,10 @@ static void setup_whitespace_attr_check(struct git_attr_check *check)
unsigned whitespace_rule(const char *pathname)
{
- struct git_attr_check attr_whitespace_rule;
+ struct git_attr_check_elem attr_whitespace_rule;
setup_whitespace_attr_check(&attr_whitespace_rule);
- if (!git_check_attr(pathname, 1, &attr_whitespace_rule)) {
+ if (!git_check_attrs(pathname, 1, &attr_whitespace_rule)) {
const char *value;
value = attr_whitespace_rule.value;
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 09/35] attr.c: plug small leak in parse_attr_line()
From: Stefan Beller @ 2016-11-10 20:34 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
If any error is noticed after the match_attr structure is allocated,
we shouldn't just return NULL from this function.
Add a fail_return label that frees the allocated structure and
returns NULL, and consistently jump there when we want to return
NULL after cleaning up.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/attr.c b/attr.c
index 5c35d42..1877f7a 100644
--- a/attr.c
+++ b/attr.c
@@ -223,7 +223,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
if (!macro_ok) {
fprintf(stderr, "%s not allowed: %s:%d\n",
name, src, lineno);
- return NULL;
+ goto fail_return;
}
is_macro = 1;
name += strlen(ATTRIBUTE_MACRO_PREFIX);
@@ -233,7 +233,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
fprintf(stderr,
"%.*s is not a valid attribute name: %s:%d\n",
namelen, name, src, lineno);
- return NULL;
+ goto fail_return;
}
}
else
@@ -246,7 +246,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
for (cp = states, num_attr = 0; *cp; num_attr++) {
cp = parse_attr(src, lineno, cp, NULL);
if (!cp)
- return NULL;
+ goto fail_return;
}
res = xcalloc(1,
@@ -267,7 +267,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
if (res->u.pat.flags & EXC_FLAG_NEGATIVE) {
warning(_("Negative patterns are ignored in git attributes\n"
"Use '\\!' for literal leading exclamation."));
- return NULL;
+ goto fail_return;
}
}
res->is_macro = is_macro;
@@ -283,6 +283,10 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
}
return res;
+
+fail_return:
+ free(res);
+ return NULL;
}
/*
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 05/35] attr.c: complete a sentence in a comment
From: Stefan Beller @ 2016-11-10 20:33 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index 05db667..a7f2c3f 100644
--- a/attr.c
+++ b/attr.c
@@ -300,7 +300,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
* directory (again, reading the file from top to bottom) down to the
* current directory, and then scan the list backwards to find the first match.
* This is exactly the same as what is_excluded() does in dir.c to deal with
- * .gitignore
+ * .gitignore file and info/excludes file as a fallback.
*/
static struct attr_stack {
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 06/35] attr.c: mark where #if DEBUG ends more clearly
From: Stefan Beller @ 2016-11-10 20:33 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/attr.c b/attr.c
index a7f2c3f..95416d3 100644
--- a/attr.c
+++ b/attr.c
@@ -469,7 +469,7 @@ static void debug_set(const char *what, const char *match, struct git_attr *attr
#define debug_push(a) do { ; } while (0)
#define debug_pop(a) do { ; } while (0)
#define debug_set(a,b,c,d) do { ; } while (0)
-#endif
+#endif /* DEBUG_ATTR */
static void drop_attr_stack(void)
{
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCH 02/35] attr.c: use strchrnul() to scan for one line
From: Stefan Beller @ 2016-11-10 20:33 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
In-Reply-To: <20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
attr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index eec5d7d..45aec1b 100644
--- a/attr.c
+++ b/attr.c
@@ -402,8 +402,8 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
for (sp = buf; *sp; ) {
char *ep;
int more;
- for (ep = sp; *ep && *ep != '\n'; ep++)
- ;
+
+ ep = strchrnul(sp, '\n');
more = (*ep == '\n');
*ep = '\0';
handle_attr_line(res, sp, path, ++lineno, macro_ok);
--
2.10.1.469.g00a8914
^ permalink raw reply related
* [PATCHv3 00/35] Revamp the attr subsystem!
From: Stefan Beller @ 2016-11-10 20:33 UTC (permalink / raw)
To: gitster, bmwill, pclouds; +Cc: git, Stefan Beller
previous discussion: https://public-inbox.org/git/20161028185502.8789-1-sbeller@google.com/
Changes since v2:
* dropped one patch (that exposes git_attr_counted, nobody uses
it throughout the series )
* added pathspec guarding and added a test with git-add to see if it works.
* squashed "SQUASH" commits.
* changed test for multithreaded tests as git-status makes use of pathspec
code that doesn't support this new magic yet.
interdiff to currently queued at the bottom of this email.
v2 was:
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.
Thanks,
Stefan
Junio C Hamano (23):
commit.c: use strchrnul() to scan for one line
attr.c: use strchrnul() to scan for one line
attr.c: update a stale comment on "struct match_attr"
attr.c: explain the lack of attr-name syntax check in parse_attr()
attr.c: complete a sentence in a comment
attr.c: mark where #if DEBUG ends more clearly
attr.c: simplify macroexpand_one()
attr.c: tighten constness around "git_attr" structure
attr.c: plug small leak in parse_attr_line()
attr: rename function and struct related to checking attributes
attr: (re)introduce git_check_attr() and struct git_attr_check
attr: convert git_all_attrs() to use "struct git_attr_check"
attr: convert git_check_attrs() callers to use the new API
attr: retire git_check_attrs() API
attr: add counted string version of git_check_attr()
attr: expose validity check for attribute names
attr.c: add push_stack() helper
attr.c: pass struct git_attr_check down the callchain
attr.c: rename a local variable check
attr.c: correct ugly hack for git_all_attrs()
attr.c: introduce empty_attr_check_elems()
attr.c: always pass check[] to collect_some_attrs()
attr.c: outline the future plans by heavily commenting
Nguyễn Thái Ngọc Duy (1):
attr: support quoting pathname patterns in C style
Stefan Beller (11):
attr: make git_check_attr_counted static
attr: convert to new threadsafe API
attr: keep attr stack for each check
Documentation: fix a typo
pathspec: move long magic parsing out of prefix_pathspec
pathspec: move prefix check out of the inner loop
pathspec: allow querying for attributes
pathspec: allow escaped query values
submodule update: add `--init-default-path` switch
clone: add --init-submodule=<pathspec> switch
completion: clone can initialize specific submodules
Documentation/config.txt | 5 +
Documentation/git-clone.txt | 23 +-
Documentation/git-submodule.txt | 17 +-
Documentation/gitattributes.txt | 10 +-
Documentation/glossary-content.txt | 20 +
Documentation/technical/api-gitattributes.txt | 140 ++++---
archive.c | 25 +-
attr.c | 517 ++++++++++++++++++--------
attr.h | 71 ++--
builtin/check-attr.c | 74 ++--
builtin/clone.c | 36 +-
builtin/pack-objects.c | 27 +-
commit.c | 3 +-
common-main.c | 2 +
contrib/completion/git-completion.bash | 1 +
convert.c | 46 ++-
dir.c | 41 +-
git-submodule.sh | 21 +-
hashmap.h | 2 +
ll-merge.c | 35 +-
pathspec.c | 228 ++++++++++--
pathspec.h | 19 +-
t/t0003-attributes.sh | 26 ++
t/t6134-pathspec-with-labels.sh | 201 ++++++++++
t/t7400-submodule-basic.sh | 134 +++++++
userdiff.c | 21 +-
ws.c | 19 +-
27 files changed, 1354 insertions(+), 410 deletions(-)
create mode 100755 t/t6134-pathspec-with-labels.sh
-- interdiff to current origin/sb/attr:
diff --git a/attr.c b/attr.c
index cf15e76..979bd3c 100644
--- a/attr.c
+++ b/attr.c
@@ -135,7 +135,7 @@ static void report_invalid_attr(const char *name, size_t len,
strbuf_release(&err);
}
-struct git_attr *git_attr_counted(const char *name, size_t len)
+static struct git_attr *git_attr_internal(const char *name, int len)
{
unsigned hval = hash_name(name, len);
unsigned pos = hval % HASHSIZE;
@@ -171,7 +171,7 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
struct git_attr *git_attr(const char *name)
{
- return git_attr_counted(name, strlen(name));
+ return git_attr_internal(name, strlen(name));
}
/* What does a matched pattern decide? */
@@ -259,7 +259,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_counted(cp, len);
+ e->attr = git_attr_internal(cp, len);
}
return ep + strspn(ep, blank);
}
@@ -320,7 +320,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_counted(name, namelen);
+ res->u.attr = git_attr_internal(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 e7534da..ab381ae 100644
--- a/attr.h
+++ b/attr.h
@@ -2,7 +2,7 @@
#define ATTR_H
/*
- * Must be called on platforms that do not support static initialisation
+ * Must be called on platforms that do not support static initialization
* of mutexes.
*/
extern void attr_start(void);
diff --git a/dir.c b/dir.c
index 3ec9117..c56d2ee 100644
--- a/dir.c
+++ b/dir.c
@@ -140,7 +140,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (n = 0; n < pathspec->nr; n++) {
size_t i = 0, len = 0, item_len;
@@ -349,7 +350,8 @@ static int do_match_pathspec(const struct pathspec *ps,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
if (!ps->nr) {
if (!ps->recursive ||
diff --git a/pathspec.c b/pathspec.c
index 8ea5dcb..d702803 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -232,6 +232,7 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
char *attr_body = xmemdupz(body, len - strlen("attr:"));
parse_pathspec_attr_match(item, attr_body);
free(attr_body);
+ *magic |= PATHSPEC_ATTR;
continue;
}
diff --git a/pathspec.h b/pathspec.h
index ebef4bf..c2d046d 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -8,13 +8,15 @@
#define PATHSPEC_GLOB (1<<3)
#define PATHSPEC_ICASE (1<<4)
#define PATHSPEC_EXCLUDE (1<<5)
+#define PATHSPEC_ATTR (1<<6)
#define PATHSPEC_ALL_MAGIC \
(PATHSPEC_FROMTOP | \
PATHSPEC_MAXDEPTH | \
PATHSPEC_LITERAL | \
PATHSPEC_GLOB | \
PATHSPEC_ICASE | \
- PATHSPEC_EXCLUDE)
+ PATHSPEC_EXCLUDE | \
+ PATHSPEC_ATTR)
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern satisfies GFNM_ONESTAR */
diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
index 3d03e0a..5aafc0c 100755
--- a/t/t6134-pathspec-with-labels.sh
+++ b/t/t6134-pathspec-with-labels.sh
@@ -160,7 +160,23 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
'
test_expect_success 'checking attributes in a multithreaded process' '
- git status ":(attr:labelB)"
+ cat <<-EOF >expect &&
+ fileA:A
+ EOF
+ echo A >fileA &&
+ echo A >fileB &&
+ git grep A -- ":(attr:labelA)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'fail if attr magic is used places not implemented' '
+ # The main purpose of this test is to check that we actually fail
+ # when you attempt to use attr magic in commands that do not implement
+ # attr magic. This test does not advocate git-add to stay that way,
+ # though, but git-add is convenient as it has its own internal pathspec
+ # parsing.
+ test_must_fail git add ":(attr:labelB)" 2>actual &&
+ test_i18ngrep "unsupported magic" actual
'
test_expect_success 'abort on giving invalid label on the command line' '
--
2.10.1.469.g00a8914
^ permalink raw reply related
* Re: [PATCH v2] t6026-merge-attr: don't fail if sleep exits early
From: Junio C Hamano @ 2016-11-10 19:20 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Jeff King, Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <mvm8tsrbusp.fsf_-_@hawking.suse.de>
Andreas Schwab <schwab@suse.de> writes:
> Commit 5babb5bdb3 ("t6026-merge-attr: clean up background process at end
> of test case") added a kill command to clean up after the test, but this
> can fail if the sleep command exits before the cleanup is executed.
> Ignore the error from the kill command.
>
> Explicitly check for the existence of the pid file to test that the merge
> driver was actually called.
>
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
OK. sleep.pid is a reasonable easy-to-access side effect we can
observe to make sure that the sleep-one-second merge driver was
indeed invoked, which was missing from the earlier round.
Thanks, will apply.
> t/t6026-merge-attr.sh | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
> index 7a6e33e673..03d13d00b5 100755
> --- a/t/t6026-merge-attr.sh
> +++ b/t/t6026-merge-attr.sh
> @@ -187,13 +187,14 @@ test_expect_success 'custom merge does not lock index' '
> sleep 1 &
> echo $! >sleep.pid
> EOF
> - test_when_finished "kill \$(cat sleep.pid)" &&
> + test_when_finished "kill \$(cat sleep.pid) || :" &&
>
> test_write_lines >.gitattributes \
> "* merge=ours" "text merge=sleep-one-second" &&
> test_config merge.ours.driver true &&
> test_config merge.sleep-one-second.driver ./sleep-one-second.sh &&
> - git merge master
> + git merge master &&
> + test -f sleep.pid
> '
>
> test_done
> --
> 2.10.2
^ permalink raw reply
* Re: [PATCH v2] rebase: add --forget to cleanup rebase, leave everything else untouched
From: Junio C Hamano @ 2016-11-10 19:07 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8CHWZyFgZu1=VKCQc+hbHUmZiAQ9ToL=NDF=YBEkLfbog@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Thu, Nov 10, 2016 at 6:09 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Thu, Nov 10, 2016 at 3:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>>>
>>>> ---
>>>> v2 changes just the subject line
>>>
>>> That's not sufficient, is it? What you did in the documentation
>>> would raise the same "Hmph, is this only about HEAD?" and unlike the
>>> commit subject, it will carve it in stone for end-users.
>>
>> Oops. I forgot about git-rebase.txt. How about this?
>>
>> --forget::
>> Abort the rebase operation but leave HEAD, the index and
>> working tree untouched.
>
> Or, since --abort describes it as "reset HEAD to the original branch",
> we could write "Abort the rebase operation. Unlike --abort, HEAD is
> not restored back to the original branch". Index and worktree are
> implied by "not restored". Not sure if it's too subtle.
I think you are moving in the right direction, even though "Abort
it; unlike --abort, does this instead" is a bit hard to follow.
^ permalink raw reply
* Re: [PATCH v7 05/17] ref-filter: move get_head_description() from branch.c
From: Karthik Nayak @ 2016-11-10 19:01 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xo4M5g15B7zKzC-zJvd6O0Wfpks142-+C8tAqTfyEJ+nA@mail.gmail.com>
On Wed, Nov 9, 2016 at 5:01 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 12:11 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>
>>
>> - if (starts_with(name, "refname"))
>> + if (starts_with(name, "refname")) {
>> refname = ref->refname;
>> - else if (starts_with(name, "symref"))
>> + if (ref->kind & FILTER_REFS_DETACHED_HEAD)
>> + refname = get_head_description();
>
> Since this (I think?) changes behavior of refname would it make sense
> to add a test for this?
>
At the moment there is no way to check this, since this option is not used by
git for-each-ref or git tag (both of which completely use ref-filter
ATM). This is
however tested as eventually git branch uses ref-filter.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
From: Karthik Nayak @ 2016-11-10 17:31 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xoavgQ=NK8GGGXf90ESuPs+mFwHzEWbsnjWjZ4V7TWpjw@mail.gmail.com>
On Wed, Nov 9, 2016 at 4:52 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 12:11 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Ok. How does this handle whitespace? The previous if implementation
> treated whitespace as trimming to ignore. Does this require an exact
> whitespace match? It appears by the code that strings must match
> exactly. Would it make more sense to always trim the value of
> whitespace first before comparison? Hmm.. I think we should avoid
> doing that actually.
>
This does not trim whitespace what so ever and requires an exact match.
I don't see the reason behind trimming whitespace though.
>> In addition to the above, for commit and tag objects, the header
>> field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 8392303..44481c3 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -22,6 +22,8 @@ struct align {
>> };
>>
>> struct if_then_else {
>> + const char *if_equals,
>> + *not_equals;
>
> Ok so we add both if_equals and not_equals values. Could we re-use the
> same string?
If we use an union then we'll be saving space, but that comes at the
disadvantage that
we'd need an indicator to show which subatom we're using (i.e
if_equals or not_equals)
for the checks made in the code below.
At the expense of keeping two pointers we avoid the extra indicator
while keeping the code
elegant. Either ways not too keen on this, up for discussion :)
>
>> unsigned int then_atom_seen : 1,
>> else_atom_seen : 1,
>> condition_satisfied : 1;
>> @@ -49,6 +51,10 @@ static struct used_atom {
>> enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>> unsigned int nlines;
>> } contents;
>> + struct {
>> + const char *if_equals,
>> + *not_equals;
>
>
> Same here, why do we need both strings here stored separately? Could
> we instead store which state to check and store the string once? I'm
> not sure that really buys us any storage.
>
same as above, we utilize the fact that its easier to check the strings like
if (if_equals)
...
else (not_equals)
...
else we'd have to have an indicator to know which to check.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 04/17] ref-filter: modify "%(objectname:short)" to take length
From: Karthik Nayak @ 2016-11-10 17:36 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xqG5ACT2ZB=M8gLmDaYgSgX202+afJmZJKat6NzbFzL=g@mail.gmail.com>
On Wed, Nov 9, 2016 at 4:57 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 12:11 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> From: Karthik Nayak <karthik.188@gmail.com>
>>
>> Add support for %(objectname:short=<length>) which would print the
>> abbreviated unique objectname of given length. When no length is
>> specified, the length is 'DEFAULT_ABBREV'. The minimum length is
>> 'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
>> object name is unique.
>>
>
> Ok this makes sense. It may be annoying that the length might go
> beyond the size that we wanted, but I think it's better than printing
> a non-unique short abbreviation.
>
> I have one suggested change, which is to drop O_LENGTH and have
> O_SHORT store the length always, setting it to DEFAULT_ABBREV when no
> length provided. This allows you to drop some code. I don't think it's
> actually worth a re-roll by itself since the current code is correct.
>
> Thanks,
> Jake
>
That does make sense, It would also not error out when we use
%(objectname:short=) and
not specify the length. Idk, if that's desirable or not. But it does
make the code a little more
confusing to read at the same time.
So since its a small change, I'd be okay going either ways with this.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 02/17] ref-filter: include reference to 'used_atom' within 'atom_value'
From: Karthik Nayak @ 2016-11-10 17:16 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xqVAEuc12eQJ7-Et0CtXc12t5zDUg3Qsh3214cTh_brbA@mail.gmail.com>
>>
>> /*
>> @@ -370,7 +368,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s
>> push_stack_element(&state->stack);
>> new = state->stack;
>> new->at_end = end_align_handler;
>> - new->at_end_data = &atomv->u.align;
>> + new->at_end_data = &atomv->atom->u.align;
>
> At first, this confused me. I was like "we dropped the union, why are
> we still referencing it. But I realized that the "used_atom" struct
> actually contains the same union and we were copying it.
>
> Ok, so this looks good.
>
It is confusing if one only looks at the patch without actually going
through ref-filter.c. I'm sure your comment will help
anyone going through these patches.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v7 01/17] ref-filter: implement %(if), %(then), and %(else) atoms
From: Karthik Nayak @ 2016-11-10 17:11 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xoAmYqi0OazZux+rooXW+D_N9L5s+2BzUyJJJXtmLZX-Q@mail.gmail.com>
Hey,
On Wed, Nov 9, 2016 at 4:43 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 12:11 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> From: Karthik Nayak <karthik.188@gmail.com>
>>
>> +Some atoms like %(align) and %(if) always require a matching %(end).
>> +We call them "opening atoms" and sometimes denote them as %($open).
>> +
>> +When a scripting language specific quoting is in effect (i.e. one of
>> +`--shell`, `--perl`, `--python`, `--tcl` is used), except for opening
>> +atoms, replacement from every %(atom) is quoted when and only when it
>> +appears at the top-level (that is, when it appears outside
>> +%($open)...%(end)).
>> +
>> +When a scripting language specific quoting is in effect, everything
>> +between a top-level opening atom and its matching %(end) is evaluated
>> +according to the semantics of the opening atom and its result is
>> +quoted.
>> +
>>
>
> Nice, I like the explanation above.
>
All thanks to Eric, Junio, Christian, Matthieu and everyone else who
helped me phrase
these.
>>
>> + }
>> + } else if (!if_then_else->condition_satisfied)
>
> Minor nit. I'm not sure what standard we use here at Git, but
> traditionally, I prefer to see { } blocks on all sections even if only
> one of them needs it. (That is, only drop the braces when every
> section is one line.) It also looks weird with a comment since it
> appears as multiple lines to the reader. I think the braces improve
> readability.
>
> I don't know whether that's Git's code base standard or not, however.
> It's not really worth a re-roll unless something else would need to
> change.
>
I believe this is the syntax followed in Git, xdiff/xmerge.c:173 and so on.
You're comments are right on though.
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH v1 2/2] travis-ci: disable GIT_TEST_HTTPD for macOS
From: Jeff King @ 2016-11-10 16:10 UTC (permalink / raw)
To: Lars Schneider; +Cc: Junio C Hamano, Torsten Bögershausen, git
In-Reply-To: <CBAF806C-7E1E-4490-A07C-F98DB7488F5F@gmail.com>
On Thu, Nov 10, 2016 at 12:07:14PM +0100, Lars Schneider wrote:
> > Using Apache in the tests has been the source of frequent portability
> > problems and configuration headaches. I do wonder if we'd be better off
> > using some small special-purpose web server (even a short perl script
> > written around HTTP::Server::Simple or something).
> >
> > On the other hand, testing against Apache approximates a more real-world
> > case, which has value. It might be nice if our tests supported multiple
> > web servers, but that would mean duplicating the config for each
> > manually.
>
> I agree that the real-world Apache test is more valuable and I really want
> to keep the Linux Apache test running. However, I don't think many people
> use macOS as Git web server and therefore I thought it is not worth the
> effort to investigate this problem further.
IMHO, the value in the http tests is not testing the server side, but
the client side. Without being able to set up a dummy HTTP server, we do
not have any way to exercise the client side of git-over-http at all.
And people on macOS _do_ use that. :)
-Peff
^ 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