* [PATCH v3 4/5] trailer: have function to describe trailer layout
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Create a function that, taking a string, describes the position of its
trailer block (if available) and the contents thereof, and make trailer
use it. This makes it easier for other Git components, in the future, to
interpret trailer blocks in the same way as trailer.
In a subsequent patch, another component will be made to use this.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 118 +++++++++++++++++++++++++++++++++++++++++++-------------------
trailer.h | 25 +++++++++++++
2 files changed, 107 insertions(+), 36 deletions(-)
diff --git a/trailer.c b/trailer.c
index afbff4b..bc6893b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,6 +46,8 @@ static LIST_HEAD(conf_head);
static char *separators = ":";
+static int configured;
+
#define TRAILER_ARG_STRING "$ARG"
static const char *git_generated_prefixes[] = {
@@ -546,6 +548,17 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
return 0;
}
+static void ensure_configured(void)
+{
+ if (configured)
+ return;
+
+ /* Default config must be setup first */
+ git_config(git_trailer_default_config, NULL);
+ git_config(git_trailer_config, NULL);
+ configured = 1;
+}
+
static const char *token_from_item(struct arg_item *item, char *tok)
{
if (item->conf.key)
@@ -873,59 +886,43 @@ static int process_input_file(FILE *outfile,
const char *str,
struct list_head *head)
{
- int patch_start, trailer_start, trailer_end;
+ struct trailer_info info;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
- struct trailer_item *last = NULL;
- struct strbuf *trailer, **trailer_lines, **ptr;
+ int i;
- patch_start = find_patch_start(str);
- trailer_end = find_trailer_end(str, patch_start);
- trailer_start = find_trailer_start(str, trailer_end);
+ trailer_info_get(&info, str);
/* Print lines before the trailers as is */
- fwrite(str, 1, trailer_start, outfile);
+ fwrite(str, 1, info.trailer_start - str, outfile);
- if (!ends_with_blank_line(str, trailer_start))
+ if (!info.blank_line_before_trailer)
fprintf(outfile, "\n");
- /* Parse trailer lines */
- trailer_lines = strbuf_split_buf(str + trailer_start,
- trailer_end - trailer_start,
- '\n',
- 0);
- for (ptr = trailer_lines; *ptr; ptr++) {
+ for (i = 0; i < info.trailer_nr; i++) {
int separator_pos;
- trailer = *ptr;
- if (trailer->buf[0] == comment_line_char)
- continue;
- if (last && isspace(trailer->buf[0])) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf);
- strbuf_strip_suffix(&sb, "\n");
- free(last->value);
- last->value = strbuf_detach(&sb, NULL);
+ char *trailer = info.trailers[i];
+ if (trailer[0] == comment_line_char)
continue;
- }
- separator_pos = find_separator(trailer->buf, separators);
+ separator_pos = find_separator(trailer, separators);
if (separator_pos >= 1) {
- parse_trailer(&tok, &val, NULL, trailer->buf,
+ parse_trailer(&tok, &val, NULL, trailer,
separator_pos);
- last = add_trailer_item(head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ add_trailer_item(head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL));
} else {
- strbuf_addbuf(&val, trailer);
+ strbuf_addstr(&val, trailer);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
strbuf_detach(&val, NULL));
- last = NULL;
}
}
- strbuf_list_free(trailer_lines);
- return trailer_end;
+ trailer_info_release(&info);
+
+ return info.trailer_end - str;
}
static void free_all(struct list_head *head)
@@ -976,9 +973,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
int trailer_end;
FILE *outfile = stdout;
- /* Default config must be setup first */
- git_config(git_trailer_default_config, NULL);
- git_config(git_trailer_config, NULL);
+ ensure_configured();
read_input_file(&sb, file);
@@ -1005,3 +1000,54 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
strbuf_release(&sb);
}
+
+void trailer_info_get(struct trailer_info *info, const char *str)
+{
+ int patch_start, trailer_end, trailer_start;
+ struct strbuf **trailer_lines, **ptr;
+ char **trailer_strings = NULL;
+ size_t nr = 0, alloc = 0;
+ char **last = NULL;
+
+ ensure_configured();
+
+ patch_start = find_patch_start(str);
+ trailer_end = find_trailer_end(str, patch_start);
+ trailer_start = find_trailer_start(str, trailer_end);
+
+ trailer_lines = strbuf_split_buf(str + trailer_start,
+ trailer_end - trailer_start,
+ '\n',
+ 0);
+ for (ptr = trailer_lines; *ptr; ptr++) {
+ if (last && isspace((*ptr)->buf[0])) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_attach(&sb, *last, strlen(*last), strlen(*last));
+ strbuf_addbuf(&sb, *ptr);
+ *last = strbuf_detach(&sb, NULL);
+ continue;
+ }
+ ALLOC_GROW(trailer_strings, nr + 1, alloc);
+ trailer_strings[nr] = strbuf_detach(*ptr, NULL);
+ last = find_separator(trailer_strings[nr], separators) >= 1
+ ? &trailer_strings[nr]
+ : NULL;
+ nr++;
+ }
+ strbuf_list_free(trailer_lines);
+
+ info->blank_line_before_trailer = ends_with_blank_line(str,
+ trailer_start);
+ info->trailer_start = str + trailer_start;
+ info->trailer_end = str + trailer_end;
+ info->trailers = trailer_strings;
+ info->trailer_nr = nr;
+}
+
+void trailer_info_release(struct trailer_info *info)
+{
+ int i;
+ for (i = 0; i < info->trailer_nr; i++)
+ free(info->trailers[i]);
+ free(info->trailers);
+}
diff --git a/trailer.h b/trailer.h
index 36b40b8..65cc5d7 100644
--- a/trailer.h
+++ b/trailer.h
@@ -1,7 +1,32 @@
#ifndef TRAILER_H
#define TRAILER_H
+struct trailer_info {
+ /*
+ * True if there is a blank line before the location pointed to by
+ * trailer_start.
+ */
+ int blank_line_before_trailer;
+
+ /*
+ * Pointers to the start and end of the trailer block found. If there
+ * is no trailer block found, these 2 pointers point to the end of the
+ * input string.
+ */
+ const char *trailer_start, *trailer_end;
+
+ /*
+ * Array of trailers found.
+ */
+ char **trailers;
+ size_t trailer_nr;
+};
+
void process_trailers(const char *file, int in_place, int trim_empty,
struct string_list *trailers);
+void trailer_info_get(struct trailer_info *info, const char *str);
+
+void trailer_info_release(struct trailer_info *info);
+
#endif /* TRAILER_H */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 5/5] sequencer: use trailer's trailer layout
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Make sequencer use trailer.c's trailer layout definition, as opposed to
parsing the footer by itself. This makes "commit -s", "cherry-pick -x",
and "format-patch --signoff" consistent with trailer, allowing
non-trailer lines and multiple-line trailers in trailer blocks under
certain conditions, and therefore suppressing the extra newline in those
cases.
Consistency with trailer extends to respecting trailer configs. Tests
have been included to show that.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
sequencer.c | 75 +++++++++---------------------------------------
t/t3511-cherry-pick-x.sh | 16 +++++++++--
t/t4014-format-patch.sh | 37 ++++++++++++++++++++----
t/t7501-commit.sh | 36 +++++++++++++++++++++++
4 files changed, 95 insertions(+), 69 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..d64c973 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -16,6 +16,7 @@
#include "refs.h"
#include "argv-array.h"
#include "quote.h"
+#include "trailer.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -56,30 +57,6 @@ static const char *get_todo_path(const struct replay_opts *opts)
return git_path_todo_file();
}
-static int is_rfc2822_line(const char *buf, int len)
-{
- int i;
-
- for (i = 0; i < len; i++) {
- int ch = buf[i];
- if (ch == ':')
- return 1;
- if (!isalnum(ch) && ch != '-')
- break;
- }
-
- return 0;
-}
-
-static int is_cherry_picked_from_line(const char *buf, int len)
-{
- /*
- * We only care that it looks roughly like (cherry picked from ...)
- */
- return len > strlen(cherry_picked_prefix) + 1 &&
- starts_with(buf, cherry_picked_prefix) && buf[len - 1] == ')';
-}
-
/*
* Returns 0 for non-conforming footer
* Returns 1 for conforming footer
@@ -89,49 +66,25 @@ static int is_cherry_picked_from_line(const char *buf, int len)
static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
int ignore_footer)
{
- char prev;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
- int found_sob = 0;
-
- /* footer must end with newline */
- if (!len || buf[len - 1] != '\n')
- return 0;
+ struct trailer_info info;
+ int i;
+ int found_sob = 0, found_sob_last = 0;
- prev = '\0';
- for (i = len - 1; i > 0; i--) {
- char ch = buf[i];
- if (prev == '\n' && ch == '\n') /* paragraph break */
- break;
- prev = ch;
- }
+ trailer_info_get(&info, sb->buf);
- /* require at least one blank line */
- if (prev != '\n' || buf[i] != '\n')
+ if (info.trailer_start == info.trailer_end)
return 0;
- /* advance to start of last paragraph */
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- int found_rfc2822;
-
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
+ for (i = 0; i < info.trailer_nr; i++)
+ if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) {
+ found_sob = 1;
+ if (i == info.trailer_nr - 1)
+ found_sob_last = 1;
+ }
- found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
- if (found_rfc2822 && sob &&
- !strncmp(buf + i, sob->buf, sob->len))
- found_sob = k;
+ trailer_info_release(&info);
- if (!(found_rfc2822 ||
- is_cherry_picked_from_line(buf + i, k - i - 1)))
- return 0;
- }
- if (found_sob == i)
+ if (found_sob_last)
return 3;
if (found_sob)
return 2;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9cce5ae..bf0a5c9 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -25,9 +25,8 @@ Signed-off-by: B.U. Thor <buthor@example.com>"
mesg_broken_footer="$mesg_no_footer
-The signed-off-by string should begin with the words Signed-off-by followed
-by a colon and space, and then the signers name and email address. e.g.
-Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+This is not recognized as a footer because Myfooter is not a recognized token.
+Myfooter: A.U. Thor <author@example.com>"
mesg_with_footer_sob="$mesg_with_footer
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
@@ -112,6 +111,17 @@ test_expect_success 'cherry-pick -s inserts blank line after non-conforming foot
test_cmp expect actual
'
+test_expect_success 'cherry-pick -s recognizes trailer config' '
+ pristine_detach initial &&
+ git -c "trailer.Myfooter.ifexists=add" cherry-pick -s mesg-broken-footer &&
+ cat <<-EOF >expect &&
+ $mesg_broken_footer
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
pristine_detach initial &&
sha1=$(git rev-parse mesg-no-footer^0) &&
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ba4902d..482112c 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1294,8 +1294,7 @@ EOF
4:Subject: [PATCH] subject
8:
10:Signed-off-by: example happens to be wrapped here.
-11:
-12:Signed-off-by: C O Mitter <committer@example.com>
+11:Signed-off-by: C O Mitter <committer@example.com>
EOF
test_cmp expected actual
'
@@ -1368,7 +1367,7 @@ EOF
test_cmp expected actual
'
-test_expect_success 'signoff: detect garbage in non-conforming footer' '
+test_expect_success 'signoff: tolerate garbage in conforming footer' '
append_signoff <<\EOF >actual &&
subject
@@ -1383,8 +1382,36 @@ EOF
8:
10:
13:Signed-off-by: C O Mitter <committer@example.com>
-14:
-15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: respect trailer config' '
+ append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual &&
+
+ test_config trailer.Myfooter.ifexists add &&
+ append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:Signed-off-by: C O Mitter <committer@example.com>
EOF
test_cmp expected actual
'
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d84897a..4003a27 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -460,6 +460,42 @@ $alt" &&
test_cmp expected actual
'
+test_expect_success 'signoff respects trailer config' '
+
+ echo 5 >positive &&
+ git add positive &&
+ git commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+ git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+ (
+ echo subject
+ echo
+ echo non-trailer line
+ echo Myfooter: x
+ echo
+ echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+ ) >expected &&
+ test_cmp expected actual &&
+
+ echo 6 >positive &&
+ git add positive &&
+ git -c "trailer.Myfooter.ifexists=add" commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+ git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+ (
+ echo subject
+ echo
+ echo non-trailer line
+ echo Myfooter: x
+ echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+ ) >expected &&
+ test_cmp expected actual
+'
+
test_expect_success 'multiple -m' '
>negative &&
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 3/5] trailer: avoid unnecessary splitting on lines
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
trailer.c currently splits lines while processing a buffer (and also
rejoins lines when needing to invoke ignore_non_trailer).
Avoid such line splitting, except when generating the strings
corresponding to trailers (for ease of use by clients - a subsequent
patch will allow other components to obtain the layout of a trailer
block in a buffer, including the trailers themselves). The main purpose
of this is to make it easy to return pointers into the original buffer
(for a subsequent patch), but this also significantly reduces the number
of memory allocations required.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 194 ++++++++++++++++++++++++++++++++------------------------------
1 file changed, 100 insertions(+), 94 deletions(-)
diff --git a/trailer.c b/trailer.c
index 9d7765e..afbff4b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -102,12 +102,12 @@ static int same_trailer(struct trailer_item *a, struct arg_item *b)
return same_token(a, b) && same_value(a, b);
}
-static inline int contains_only_spaces(const char *str)
+static inline int is_blank_line(const char *str)
{
const char *s = str;
- while (*s && isspace(*s))
+ while (*s && *s != '\n' && isspace(*s))
s++;
- return !*s;
+ return !*s || *s == '\n';
}
static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b)
@@ -700,51 +700,71 @@ static void process_command_line_args(struct list_head *arg_head,
free(cl_separators);
}
-static struct strbuf **read_input_file(const char *file)
+static void read_input_file(struct strbuf *sb, const char *file)
{
- struct strbuf **lines;
- struct strbuf sb = STRBUF_INIT;
-
if (file) {
- if (strbuf_read_file(&sb, file, 0) < 0)
+ if (strbuf_read_file(sb, file, 0) < 0)
die_errno(_("could not read input file '%s'"), file);
} else {
- if (strbuf_read(&sb, fileno(stdin), 0) < 0)
+ if (strbuf_read(sb, fileno(stdin), 0) < 0)
die_errno(_("could not read from stdin"));
}
+}
- lines = strbuf_split(&sb, '\n');
+static const char *next_line(const char *str)
+{
+ const char *nl = strchrnul(str, '\n');
+ return nl + !!*nl;
+}
- strbuf_release(&sb);
+/*
+ * Return the position of the start of the last line. If len is 0, return -1.
+ */
+static int last_line(const char *buf, size_t len)
+{
+ int i;
+ if (len == 0)
+ return -1;
+ if (len == 1)
+ return 0;
+ /*
+ * Skip the last character (in addition to the null terminator),
+ * because if the last character is a newline, it is considered as part
+ * of the last line anyway.
+ */
+ i = len - 2;
- return lines;
+ for (; i >= 0; i--) {
+ if (buf[i] == '\n')
+ return i + 1;
+ }
+ return 0;
}
/*
- * Return the (0 based) index of the start of the patch or the line
- * count if there is no patch in the message.
+ * Return the position of the start of the patch or the length of str if there
+ * is no patch in the message.
*/
-static int find_patch_start(struct strbuf **lines, int count)
+static int find_patch_start(const char *str)
{
- int i;
+ const char *s;
- /* Get the start of the patch part if any */
- for (i = 0; i < count; i++) {
- if (starts_with(lines[i]->buf, "---"))
- return i;
+ for (s = str; *s; s = next_line(s)) {
+ if (starts_with(s, "---"))
+ return s - str;
}
- return count;
+ return s - str;
}
/*
- * Return the (0 based) index of the first trailer line or count if
- * there are no trailers. Trailers are searched only in the lines from
- * index (count - 1) down to index 0.
+ * Return the position of the first trailer line or len if there are no
+ * trailers.
*/
-static int find_trailer_start(struct strbuf **lines, int count)
+static int find_trailer_start(const char *buf, size_t len)
{
- int start, end_of_title, only_spaces = 1;
+ const char *s;
+ int end_of_title, l, only_spaces = 1;
int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
/*
* Number of possible continuation lines encountered. This will be
@@ -756,13 +776,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
int possible_continuation_lines = 0;
/* The first paragraph is the title and cannot be trailers */
- for (start = 0; start < count; start++) {
- if (lines[start]->buf[0] == comment_line_char)
+ for (s = buf; s < buf + len; s = next_line(s)) {
+ if (s[0] == comment_line_char)
continue;
- if (contains_only_spaces(lines[start]->buf))
+ if (is_blank_line(s))
break;
}
- end_of_title = start;
+ end_of_title = s - buf;
/*
* Get the start of the trailers by looking starting from the end for a
@@ -770,30 +790,33 @@ static int find_trailer_start(struct strbuf **lines, int count)
* trailers, or (ii) contains at least one Git-generated trailer and
* consists of at least 25% trailers.
*/
- for (start = count - 1; start >= end_of_title; start--) {
+ for (l = last_line(buf, len);
+ l >= end_of_title;
+ l = last_line(buf, l)) {
+ const char *bol = buf + l;
const char **p;
int separator_pos;
- if (lines[start]->buf[0] == comment_line_char) {
+ if (bol[0] == comment_line_char) {
non_trailer_lines += possible_continuation_lines;
possible_continuation_lines = 0;
continue;
}
- if (contains_only_spaces(lines[start]->buf)) {
+ if (is_blank_line(bol)) {
if (only_spaces)
continue;
non_trailer_lines += possible_continuation_lines;
if (recognized_prefix &&
trailer_lines * 3 >= non_trailer_lines)
- return start + 1;
- if (trailer_lines && !non_trailer_lines)
- return start + 1;
- return count;
+ return next_line(bol) - buf;
+ else if (trailer_lines && !non_trailer_lines)
+ return next_line(bol) - buf;
+ return len;
}
only_spaces = 0;
for (p = git_generated_prefixes; *p; p++) {
- if (starts_with(lines[start]->buf, *p)) {
+ if (starts_with(bol, *p)) {
trailer_lines++;
possible_continuation_lines = 0;
recognized_prefix = 1;
@@ -801,8 +824,8 @@ static int find_trailer_start(struct strbuf **lines, int count)
}
}
- separator_pos = find_separator(lines[start]->buf, separators);
- if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
+ separator_pos = find_separator(bol, separators);
+ if (separator_pos >= 1 && !isspace(bol[0])) {
struct list_head *pos;
trailer_lines++;
@@ -812,13 +835,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
list_for_each(pos, &conf_head) {
struct arg_item *item;
item = list_entry(pos, struct arg_item, list);
- if (token_matches_item(lines[start]->buf, item,
+ if (token_matches_item(bol, item,
separator_pos)) {
recognized_prefix = 1;
break;
}
}
- } else if (isspace(lines[start]->buf[0]))
+ } else if (isspace(bol[0]))
possible_continuation_lines++;
else {
non_trailer_lines++;
@@ -829,88 +852,70 @@ static int find_trailer_start(struct strbuf **lines, int count)
;
}
- return count;
-}
-
-/* Get the index of the end of the trailers */
-static int find_trailer_end(struct strbuf **lines, int patch_start)
-{
- struct strbuf sb = STRBUF_INIT;
- int i, ignore_bytes;
-
- for (i = 0; i < patch_start; i++)
- strbuf_addbuf(&sb, lines[i]);
- ignore_bytes = ignore_non_trailer(sb.buf, sb.len);
- strbuf_release(&sb);
- for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--)
- ignore_bytes -= lines[i]->len;
-
- return i + 1;
+ return len;
}
-static int has_blank_line_before(struct strbuf **lines, int start)
+/* Return the position of the end of the trailers. */
+static int find_trailer_end(const char *buf, size_t len)
{
- for (;start >= 0; start--) {
- if (lines[start]->buf[0] == comment_line_char)
- continue;
- return contains_only_spaces(lines[start]->buf);
- }
- return 0;
+ return len - ignore_non_trailer(buf, len);
}
-static void print_lines(FILE *outfile, struct strbuf **lines, int start, int end)
+static int ends_with_blank_line(const char *buf, size_t len)
{
- int i;
- for (i = start; lines[i] && i < end; i++)
- fprintf(outfile, "%s", lines[i]->buf);
+ int ll = last_line(buf, len);
+ if (ll < 0)
+ return 0;
+ return is_blank_line(buf + ll);
}
static int process_input_file(FILE *outfile,
- struct strbuf **lines,
+ const char *str,
struct list_head *head)
{
- int count = 0;
- int patch_start, trailer_start, trailer_end, i;
+ int patch_start, trailer_start, trailer_end;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
struct trailer_item *last = NULL;
+ struct strbuf *trailer, **trailer_lines, **ptr;
- /* Get the line count */
- while (lines[count])
- count++;
-
- patch_start = find_patch_start(lines, count);
- trailer_end = find_trailer_end(lines, patch_start);
- trailer_start = find_trailer_start(lines, trailer_end);
+ patch_start = find_patch_start(str);
+ trailer_end = find_trailer_end(str, patch_start);
+ trailer_start = find_trailer_start(str, trailer_end);
/* Print lines before the trailers as is */
- print_lines(outfile, lines, 0, trailer_start);
+ fwrite(str, 1, trailer_start, outfile);
- if (!has_blank_line_before(lines, trailer_start - 1))
+ if (!ends_with_blank_line(str, trailer_start))
fprintf(outfile, "\n");
/* Parse trailer lines */
- for (i = trailer_start; i < trailer_end; i++) {
+ trailer_lines = strbuf_split_buf(str + trailer_start,
+ trailer_end - trailer_start,
+ '\n',
+ 0);
+ for (ptr = trailer_lines; *ptr; ptr++) {
int separator_pos;
- if (lines[i]->buf[0] == comment_line_char)
+ trailer = *ptr;
+ if (trailer->buf[0] == comment_line_char)
continue;
- if (last && isspace(lines[i]->buf[0])) {
+ if (last && isspace(trailer->buf[0])) {
struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);
+ strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf);
strbuf_strip_suffix(&sb, "\n");
free(last->value);
last->value = strbuf_detach(&sb, NULL);
continue;
}
- separator_pos = find_separator(lines[i]->buf, separators);
+ separator_pos = find_separator(trailer->buf, separators);
if (separator_pos >= 1) {
- parse_trailer(&tok, &val, NULL, lines[i]->buf,
+ parse_trailer(&tok, &val, NULL, trailer->buf,
separator_pos);
last = add_trailer_item(head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL));
} else {
- strbuf_addbuf(&val, lines[i]);
+ strbuf_addbuf(&val, trailer);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
@@ -918,6 +923,7 @@ static int process_input_file(FILE *outfile,
last = NULL;
}
}
+ strbuf_list_free(trailer_lines);
return trailer_end;
}
@@ -966,7 +972,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
{
LIST_HEAD(head);
LIST_HEAD(arg_head);
- struct strbuf **lines;
+ struct strbuf sb = STRBUF_INIT;
int trailer_end;
FILE *outfile = stdout;
@@ -974,13 +980,13 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
git_config(git_trailer_default_config, NULL);
git_config(git_trailer_config, NULL);
- lines = read_input_file(file);
+ read_input_file(&sb, file);
if (in_place)
outfile = create_in_place_tempfile(file);
/* Print the lines before the trailers */
- trailer_end = process_input_file(outfile, lines, &head);
+ trailer_end = process_input_file(outfile, sb.buf, &head);
process_command_line_args(&arg_head, trailers);
@@ -991,11 +997,11 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
free_all(&head);
/* Print the lines after the trailers as is */
- print_lines(outfile, lines, trailer_end, INT_MAX);
+ fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);
if (in_place)
if (rename_tempfile(&trailers_tempfile, file))
die_errno(_("could not rename temporary file to %s"), file);
- strbuf_list_free(lines);
+ strbuf_release(&sb);
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 2/5] commit: make ignore_non_trailer take buf/len
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Make ignore_non_trailer take a buf/len pair instead of struct strbuf.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/commit.c | 2 +-
commit.c | 22 +++++++++++-----------
commit.h | 2 +-
trailer.c | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 8976c3d..887ccc7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -790,7 +790,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_stripspace(&sb, 0);
if (signoff)
- append_signoff(&sb, ignore_non_trailer(&sb), 0);
+ append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
die_errno(_("could not write commit template"));
diff --git a/commit.c b/commit.c
index 856fd4a..2cf8515 100644
--- a/commit.c
+++ b/commit.c
@@ -1649,7 +1649,7 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
}
/*
- * Inspect sb and determine the true "end" of the log message, in
+ * Inspect the given string and determine the true "end" of the log message, in
* order to find where to put a new Signed-off-by: line. Ignored are
* trailing comment lines and blank lines, and also the traditional
* "Conflicts:" block that is not commented out, so that we can use
@@ -1659,37 +1659,37 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
* Returns the number of bytes from the tail to ignore, to be fed as
* the second parameter to append_signoff().
*/
-int ignore_non_trailer(struct strbuf *sb)
+int ignore_non_trailer(const char *buf, size_t len)
{
int boc = 0;
int bol = 0;
int in_old_conflicts_block = 0;
- while (bol < sb->len) {
- char *next_line;
+ while (bol < len) {
+ const char *next_line = memchr(buf + bol, '\n', len - bol);
- if (!(next_line = memchr(sb->buf + bol, '\n', sb->len - bol)))
- next_line = sb->buf + sb->len;
+ if (!next_line)
+ next_line = buf + len;
else
next_line++;
- if (sb->buf[bol] == comment_line_char || sb->buf[bol] == '\n') {
+ if (buf[bol] == comment_line_char || buf[bol] == '\n') {
/* is this the first of the run of comments? */
if (!boc)
boc = bol;
/* otherwise, it is just continuing */
- } else if (starts_with(sb->buf + bol, "Conflicts:\n")) {
+ } else if (starts_with(buf + bol, "Conflicts:\n")) {
in_old_conflicts_block = 1;
if (!boc)
boc = bol;
- } else if (in_old_conflicts_block && sb->buf[bol] == '\t') {
+ } else if (in_old_conflicts_block && buf[bol] == '\t') {
; /* a pathname in the conflicts block */
} else if (boc) {
/* the previous was not trailing comment */
boc = 0;
in_old_conflicts_block = 0;
}
- bol = next_line - sb->buf;
+ bol = next_line - buf;
}
- return boc ? sb->len - boc : 0;
+ return boc ? len - boc : 0;
}
diff --git a/commit.h b/commit.h
index afd14f3..9c12abb 100644
--- a/commit.h
+++ b/commit.h
@@ -355,7 +355,7 @@ extern const char *find_commit_header(const char *msg, const char *key,
size_t *out_len);
/* Find the end of the log message, the right place for a new trailer. */
-extern int ignore_non_trailer(struct strbuf *sb);
+extern int ignore_non_trailer(const char *buf, size_t len);
typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
void *cb_data);
diff --git a/trailer.c b/trailer.c
index dc525e3..9d7765e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -840,7 +840,7 @@ static int find_trailer_end(struct strbuf **lines, int patch_start)
for (i = 0; i < patch_start; i++)
strbuf_addbuf(&sb, lines[i]);
- ignore_bytes = ignore_non_trailer(&sb);
+ ignore_bytes = ignore_non_trailer(sb.buf, sb.len);
strbuf_release(&sb);
for (i = patch_start - 1; i >= 0 && ignore_bytes > 0; i--)
ignore_bytes -= lines[i]->len;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 1/5] trailer: be stricter in parsing separators
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1478107666.git.jonathantanmy@google.com>
Currently, a line is interpreted to be a trailer line if it contains a
separator. Make parsing stricter by requiring the text on the left of
the separator, if not the empty string, to be of the "<token><optional
whitespace>" form.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/trailer.c b/trailer.c
index f0ecde2..dc525e3 100644
--- a/trailer.c
+++ b/trailer.c
@@ -563,15 +563,30 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
}
/*
- * Return the location of the first separator in line, or -1 if there is no
- * separator.
+ * If the given line is of the form
+ * "<token><optional whitespace><separator>..." or "<separator>...", return the
+ * location of the separator. Otherwise, return -1.
+ *
+ * The separator-starts-line case (in which this function returns 0) is
+ * distinguished from the non-well-formed-line case (in which this function
+ * returns -1) because some callers of this function need such a distinction.
*/
static int find_separator(const char *line, const char *separators)
{
- int loc = strcspn(line, separators);
- if (!line[loc])
- return -1;
- return loc;
+ int whitespace_found = 0;
+ const char *c;
+ for (c = line; *c; c++) {
+ if (strchr(separators, *c))
+ return c - line;
+ if (!whitespace_found && (isalnum(*c) || *c == '-'))
+ continue;
+ if (c != line && (*c == ' ' || *c == '\t')) {
+ whitespace_found = 1;
+ continue;
+ }
+ break;
+ }
+ return -1;
}
/*
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 0/5] Make other git commands use trailer layout
From: Jonathan Tan @ 2016-11-02 17:29 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster
In-Reply-To: <cover.1477698917.git.jonathantanmy@google.com>
This is the same as v2 except that in 1/5, the comment about
find_separators has been moved from the commit message to the code
itself. Also, a trailing whitespace and unused variable fix.
Jonathan Tan (5):
trailer: be stricter in parsing separators
commit: make ignore_non_trailer take buf/len
trailer: avoid unnecessary splitting on lines
trailer: have function to describe trailer layout
sequencer: use trailer's trailer layout
builtin/commit.c | 2 +-
commit.c | 22 ++--
commit.h | 2 +-
sequencer.c | 75 +++---------
t/t3511-cherry-pick-x.sh | 16 ++-
t/t4014-format-patch.sh | 37 +++++-
t/t7501-commit.sh | 36 ++++++
trailer.c | 299 +++++++++++++++++++++++++++++------------------
trailer.h | 25 ++++
9 files changed, 316 insertions(+), 198 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Torsten Bögershausen @ 2016-11-02 17:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwpgoqjct.fsf@gitster.mtv.corp.google.com>
> * ls/filter-process (2016-10-17) 14 commits
> (merged to 'next' on 2016-10-19 at ffd0de042c)
Some (late, as I recently got a new battery for the Mac OS 10.6 test system)
comments:
t0021 failes here:
Can't locate object method "flush" via package "IO::Handle" at /Users/tb/projects/git/git.next/t/t0021/rot13-filter.pl line 90.
fatal: The remote end hung up unexpectedly
perl itself is 5.10 and we use the one shipped with Mac OS.
Why that ?
t0021 uses the hard-coded path:
t0021/rot13-filter.pl (around line 345) and the nice macro
PERL_PATH from the Makefile is fully ignored.
Commenting out the different "flush" makes the test hang, and I haven't digged further.
^ permalink raw reply
* Re: How can I tell, from a script, if “git cherry-pick” fails?
From: Pranit Bauva @ 2016-11-02 15:36 UTC (permalink / raw)
To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAGSZTjKg1=tMYgFiwys=ePVT+3p6KTa1mQ0fP9pPns-Nvd+6fA@mail.gmail.com>
Hey Kevin,
On Wed, Nov 2, 2016 at 8:44 PM, Kevin Layer <layer@known.net> wrote:
> If the cherry-pick fails due to a merge conflict, it just returns an
> exit status of 0. I have a script that does a series of cherry-picks
> and I need to know if they succeed.
Well, I haven't checked what it returns in git 2.10, but you can
always redirect the stdout and stderr to the output and grep for the
text which it shows in the actual output. Here[1] is an example of how
to do it.
[1]: https://github.com/git/git/blob/master/t/t3507-cherry-pick-conflict.sh#L42-L55
Regards,
Pranit Bauva
^ permalink raw reply
* Re: How can I tell, from a script, if “git cherry-pick” fails?
From: Kevin Layer @ 2016-11-02 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <CAGSZTjKg1=tMYgFiwys=ePVT+3p6KTa1mQ0fP9pPns-Nvd+6fA@mail.gmail.com>
Nevermind. It's working as it should. The script that was doing the
cherry-pick was doing it in an if and I neglected to exit with a
non-zero status. Sorry for the noise.
On Wed, Nov 2, 2016 at 8:14 AM, Kevin Layer <layer@known.net> wrote:
> If the cherry-pick fails due to a merge conflict, it just returns an
> exit status of 0. I have a script that does a series of cherry-picks
> and I need to know if they succeed.
>
> I'm sure this has been covered before.
>
> Using git version 1.8.3.1.
>
> Thank you.
>
> Kevin
^ permalink raw reply
* How can I tell, from a script, if “git cherry-pick” fails?
From: Kevin Layer @ 2016-11-02 15:14 UTC (permalink / raw)
To: git
If the cherry-pick fails due to a merge conflict, it just returns an
exit status of 0. I have a script that does a series of cherry-picks
and I need to know if they succeed.
I'm sure this has been covered before.
Using git version 1.8.3.1.
Thank you.
Kevin
^ permalink raw reply
* Re: RFE: Discard hunks during `git add -p`
From: Konstantin Khomoutov @ 2016-11-02 14:07 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: git
In-Reply-To: <alpine.LSU.2.20.1611021435280.21207@nerf40.vanv.qr>
On Wed, 2 Nov 2016 14:46:04 +0100 (CET)
Jan Engelhardt <jengelh@inai.de> wrote:
> Current version: 2.10.2
> Example workflow:
>
> * I would do a global substitution across a source tree, e.g. `perl
> -i -pe 's{OLD_FOO\(x\)}{NEW_BAR(x, 0)}' *.c`
> * Using `git add -p`, I would verify each of the substitutions that
> they make sense in their respective locations, and, based on that,
> answer "y" or "n" to the interactive prompting to stage good hunks.
> * When done with add-p, I would commit the so-staged hunks,
> and then use `git reset --hard` to discard all changes that were
> not acknowledged during add-p.
>
> Being able to discard hunks (reset working copy to index contents)
> during add-p would alleviate the (quite broad) hard reset.
Couldn't you just do
git checkout -- .
after staging your approved changes?
To selectively zap uncommitted changes from your working tree, you could
do
git checkout --patch -- .
I'm not sure overloading `git add` with a "reverse" action is a good
idea. I'm actually prefer pragmatism over conceptual purity but I'm
not sure the prospective gain here is clear.
^ permalink raw reply
* RFE: Discard hunks during `git add -p`
From: Jan Engelhardt @ 2016-11-02 13:46 UTC (permalink / raw)
To: git
Current version: 2.10.2
Example workflow:
* I would do a global substitution across a source tree, e.g. `perl -i
-pe 's{OLD_FOO\(x\)}{NEW_BAR(x, 0)}' *.c`
* Using `git add -p`, I would verify each of the substitutions that they
make sense in their respective locations, and, based on that,
answer "y" or "n" to the interactive prompting to stage good hunks.
* When done with add-p, I would commit the so-staged hunks,
and then use `git reset --hard` to discard all changes that were
not acknowledged during add-p.
Being able to discard hunks (reset working copy to index contents)
during add-p would alleviate the (quite broad) hard reset.
Similar approach:
* global substitution
* Using `git add -p`, some hunks may warrant some more editing, doable
with the "e" command. The index would be updated with the extra
change, but the working copy be left as-is.
* When rerunning `git add -p` in such a state, a difference is shown
again for such edited spots, which I would like to discard (bring
the working copy into sync with index).
^ permalink raw reply
* [PATCH 5/5] exclude: do not respect symlinks for in-tree .gitignore
From: Jeff King @ 2016-11-02 13:09 UTC (permalink / raw)
To: git
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
Like .gitattributes, we would like to make sure that
.gitignore files are handled consistently whether read from
the index or from the filesystem. We can do so by using
O_NOFOLLOW when opening the files.
Signed-off-by: Jeff King <peff@peff.net>
---
dir.c | 9 +++++++--
t/t0008-ignores.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c
index 4fa1ca109..cf3fde005 100644
--- a/dir.c
+++ b/dir.c
@@ -693,6 +693,7 @@ static void invalidate_directory(struct untracked_cache *uc,
/* Flags for add_excludes() */
#define EXCLUDE_CHECK_INDEX (1<<0)
+#define EXCLUDE_NOFOLLOW (1<<1)
/*
* Given a file with name "fname", read it (either from disk, or from
@@ -713,7 +714,11 @@ static int add_excludes(const char *fname, const char *base, int baselen,
size_t size = 0;
char *buf, *entry;
- fd = open(fname, O_RDONLY);
+ if (flags & EXCLUDE_NOFOLLOW)
+ fd = open_nofollow(fname, O_RDONLY);
+ else
+ fd = open(fname, O_RDONLY);
+
if (fd < 0 || fstat(fd, &st) < 0) {
if (errno != ENOENT)
warn_on_inaccessible(fname);
@@ -1130,7 +1135,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
strbuf_addstr(&sb, dir->exclude_per_dir);
el->src = strbuf_detach(&sb, NULL);
add_excludes(el->src, el->src, stk->baselen, el,
- EXCLUDE_CHECK_INDEX,
+ EXCLUDE_CHECK_INDEX | EXCLUDE_NOFOLLOW,
untracked ? &sha1_stat : NULL);
}
/*
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index d27f438bf..7348b8e6a 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -841,4 +841,33 @@ test_expect_success 'info/exclude trumps core.excludesfile' '
test_cmp expect actual
'
+test_expect_success SYMLINKS 'set up ignore file for symlink tests' '
+ echo "*" >ignore
+'
+
+test_expect_success SYMLINKS 'symlinks respected in core.excludesFile' '
+ test_when_finished "rm symlink" &&
+ ln -s ignore symlink &&
+ test_config core.excludesFile "$(pwd)/symlink" &&
+ echo file >expect &&
+ git check-ignore file >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
+ test_when_finished "rm .git/info/exclude" &&
+ ln -sf ../../ignore .git/info/exclude &&
+ echo file >expect &&
+ git check-ignore file >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success SYMLINKS 'symlinks not respected in-tree' '
+ test_when_finished "rm .gitignore" &&
+ ln -sf ignore .gitignore &&
+ >expect &&
+ test_must_fail git check-ignore file >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Jeff King @ 2016-11-02 13:08 UTC (permalink / raw)
To: git
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
The attributes system may sometimes read in-tree files from
the filesystem, and sometimes from the index. In the latter
case, we do not resolve symbolic links (and are not likely
to ever start doing so). Let's open filesystem links with
O_NOFOLLOW so that the two cases behave consistently.
As a bonus, this means that git will not follow such
symlinks to read and parse out-of-tree paths. It's unlikely
that this could have any security implications (a malicious
repository can already feed arbitrary content to the
attribute parser, and any disclosure of the out-of-tree
contents happens only to stderr). But it's one less oddball
thing to worry about.
Note that O_NOFOLLOW only prevents following links for the
path itself, not intermediate directories in the path. At
first glance, it seems like
ln -s /some/path in-repo
might still look at "in-repo/.gitattributes", following the
symlink to "/some/path/.gitattributes". However, if
"in-repo" is a symbolic link, then we know that it has no
git paths below it, and will never look at its
.gitattributes file.
We will continue to support out-of-tree symbolic links
(e.g., in $GIT_DIR/info/attributes); this just affects
in-tree links. When a symbolic link is encountered, the
contents are ignored and a warning is printed. POSIX
specifies ELOOP in this case, so the user would generally
see something like:
warning: unable to access '.gitattributes': Too many levels of symbolic links
Signed-off-by: Jeff King <peff@peff.net>
---
attr.c | 17 +++++++++++++----
t/t0003-attributes.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/attr.c b/attr.c
index 79bd89226..abf375c8a 100644
--- a/attr.c
+++ b/attr.c
@@ -153,6 +153,7 @@ static const char blank[] = " \t\r\n";
/* Flags usable in read_attr() and parse_attr_line() family of functions. */
#define READ_ATTR_MACRO_OK (1<<0)
+#define READ_ATTR_NOFOLLOW (1<<1)
/*
* Parse a whitespace-delimited attribute state (i.e., "attr",
@@ -371,16 +372,24 @@ static struct index_state *use_index;
static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
{
- FILE *fp = fopen(path, "r");
+ int fd;
+ FILE *fp;
struct attr_stack *res;
char buf[2048];
int lineno = 0;
- if (!fp) {
+ if (flags & READ_ATTR_NOFOLLOW)
+ fd = open_nofollow(path, O_RDONLY);
+ else
+ fd = open(path, O_RDONLY);
+
+ if (fd < 0) {
if (errno != ENOENT && errno != ENOTDIR)
warn_on_inaccessible(path);
return NULL;
}
+ fp = xfdopen(fd, "r");
+
res = xcalloc(1, sizeof(*res));
while (fgets(buf, sizeof(buf), fp)) {
char *bufp = buf;
@@ -528,7 +537,7 @@ static void bootstrap_attr_stack(void)
}
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
- elem = read_attr(GITATTRIBUTES_FILE, flags);
+ elem = read_attr(GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);
elem->origin = xstrdup("");
elem->originlen = 0;
elem->prev = attr_stack;
@@ -620,7 +629,7 @@ static void prepare_attr_stack(const char *path, int dirlen)
strbuf_add(&pathbuf, path, cp - path);
strbuf_addch(&pathbuf, '/');
strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
- elem = read_attr(pathbuf.buf, 0);
+ elem = read_attr(pathbuf.buf, READ_ATTR_NOFOLLOW);
strbuf_setlen(&pathbuf, cp - path);
elem->origin = strbuf_detach(&pathbuf, &elem->originlen);
elem->prev = attr_stack;
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index f0fbb4255..bd01945e5 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -297,4 +297,35 @@ test_expect_success 'bare repository: test info/attributes' '
)
'
+check_symlink () {
+ echo "$1: symlink: $2" >expect &&
+ git check-attr symlink "$1" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success SYMLINKS 'set up attr file for symlink tests' '
+ echo "* symlink" >attr
+'
+
+test_expect_success SYMLINKS 'symlinks respected in core.attributesFile' '
+ test_when_finished "rm symlink" &&
+ ln -s attr symlink &&
+ test_config core.attributesFile "$(pwd)/symlink" &&
+ check_symlink file set
+'
+
+test_expect_success SYMLINKS 'symlinks respected in info/attributes' '
+ test_when_finished "rm .git/info/attributes" &&
+ ln -s ../../attr .git/info/attributes &&
+ check_symlink file set
+'
+
+test_expect_success SYMLINKS 'symlinks not respected in-tree' '
+ test_when_finished "rm .gitattributes" &&
+ ln -sf attr .gitattributes &&
+ mkdir subdir &&
+ ln -sf ../attr subdir/.gitattributes &&
+ check_symlink subdir/file unspecified
+'
+
test_done
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 3/5] exclude: convert "check_index" into a flags field
From: Jeff King @ 2016-11-02 13:07 UTC (permalink / raw)
To: git
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
We pass the "check_index" flag through the variants of
add_excludes(). Let's turn this into a full flags bit-field,
so that we can add more flags to it without affecting the
function signature.
Note that only one caller actually needs to use the new flag
name, as the rest all were passing "0" already.
Signed-off-by: Jeff King <peff@peff.net>
---
dir.c | 13 +++++++++----
dir.h | 2 +-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/dir.c b/dir.c
index bfa8c8a9a..4fa1ca109 100644
--- a/dir.c
+++ b/dir.c
@@ -691,6 +691,9 @@ static void invalidate_directory(struct untracked_cache *uc,
dir->dirs[i]->recurse = 0;
}
+/* Flags for add_excludes() */
+#define EXCLUDE_CHECK_INDEX (1<<0)
+
/*
* Given a file with name "fname", read it (either from disk, or from
* the index if "check_index" is non-zero), parse it and store the
@@ -701,9 +704,10 @@ static void invalidate_directory(struct untracked_cache *uc,
* ss_valid is non-zero, "ss" must contain good value as input.
*/
static int add_excludes(const char *fname, const char *base, int baselen,
- struct exclude_list *el, int check_index,
+ struct exclude_list *el, unsigned flags,
struct sha1_stat *sha1_stat)
{
+ int check_index = !!(flags & EXCLUDE_CHECK_INDEX);
struct stat st;
int fd, i, lineno = 1;
size_t size = 0;
@@ -787,9 +791,9 @@ static int add_excludes(const char *fname, const char *base, int baselen,
int add_excludes_from_file_to_list(const char *fname, const char *base,
int baselen, struct exclude_list *el,
- int check_index)
+ unsigned flags)
{
- return add_excludes(fname, base, baselen, el, check_index, NULL);
+ return add_excludes(fname, base, baselen, el, flags, NULL);
}
struct exclude_list *add_exclude_list(struct dir_struct *dir,
@@ -1125,7 +1129,8 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
strbuf_addbuf(&sb, &dir->basebuf);
strbuf_addstr(&sb, dir->exclude_per_dir);
el->src = strbuf_detach(&sb, NULL);
- add_excludes(el->src, el->src, stk->baselen, el, 1,
+ add_excludes(el->src, el->src, stk->baselen, el,
+ EXCLUDE_CHECK_INDEX,
untracked ? &sha1_stat : NULL);
}
/*
diff --git a/dir.h b/dir.h
index 97c83bb38..ba7eb924c 100644
--- a/dir.h
+++ b/dir.h
@@ -239,7 +239,7 @@ extern int is_excluded(struct dir_struct *dir, const char *name, int *dtype);
extern struct exclude_list *add_exclude_list(struct dir_struct *dir,
int group_type, const char *src);
extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,
- struct exclude_list *el, int check_index);
+ struct exclude_list *el, unsigned flags);
extern void add_excludes_from_file(struct dir_struct *, const char *fname);
extern void parse_exclude_pattern(const char **string, int *patternlen, unsigned *flags, int *nowildcardlen);
extern void add_exclude(const char *string, const char *base,
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 2/5] attr: convert "macro_ok" into a flags field
From: Jeff King @ 2016-11-02 13:06 UTC (permalink / raw)
To: git
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
The attribute code can have a rather deep callstack, through
which we have to pass the "macro_ok" flag. In anticipation
of adding other flags, let's convert this to a generic
bit-field.
Signed-off-by: Jeff King <peff@peff.net>
---
attr.c | 43 ++++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/attr.c b/attr.c
index 1fcf042b8..79bd89226 100644
--- a/attr.c
+++ b/attr.c
@@ -151,6 +151,9 @@ struct match_attr {
static const char blank[] = " \t\r\n";
+/* Flags usable in read_attr() and parse_attr_line() family of functions. */
+#define READ_ATTR_MACRO_OK (1<<0)
+
/*
* Parse a whitespace-delimited attribute state (i.e., "attr",
* "-attr", "!attr", or "attr=value") from the string starting at src.
@@ -200,7 +203,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
}
static struct match_attr *parse_attr_line(const char *line, const char *src,
- int lineno, int macro_ok)
+ int lineno, unsigned flags)
{
int namelen;
int num_attr, i;
@@ -215,7 +218,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
namelen = strcspn(name, blank);
if (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&
starts_with(name, ATTRIBUTE_MACRO_PREFIX)) {
- if (!macro_ok) {
+ if (!(flags & READ_ATTR_MACRO_OK)) {
fprintf(stderr, "%s not allowed: %s:%d\n",
name, src, lineno);
return NULL;
@@ -339,11 +342,11 @@ static void handle_attr_line(struct attr_stack *res,
const char *line,
const char *src,
int lineno,
- int macro_ok)
+ unsigned flags)
{
struct match_attr *a;
- a = parse_attr_line(line, src, lineno, macro_ok);
+ a = parse_attr_line(line, src, lineno, flags);
if (!a)
return;
ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
@@ -358,14 +361,15 @@ static struct attr_stack *read_attr_from_array(const char **list)
res = xcalloc(1, sizeof(*res));
while ((line = *(list++)) != NULL)
- handle_attr_line(res, line, "[builtin]", ++lineno, 1);
+ handle_attr_line(res, line, "[builtin]", ++lineno,
+ READ_ATTR_MACRO_OK);
return res;
}
static enum git_attr_direction direction;
static struct index_state *use_index;
-static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
+static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
{
FILE *fp = fopen(path, "r");
struct attr_stack *res;
@@ -382,13 +386,13 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
char *bufp = buf;
if (!lineno)
skip_utf8_bom(&bufp, strlen(bufp));
- handle_attr_line(res, bufp, path, ++lineno, macro_ok);
+ handle_attr_line(res, bufp, path, ++lineno, flags);
}
fclose(fp);
return res;
}
-static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
+static struct attr_stack *read_attr_from_index(const char *path, unsigned flags)
{
struct attr_stack *res;
char *buf, *sp;
@@ -406,34 +410,34 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
;
more = (*ep == '\n');
*ep = '\0';
- handle_attr_line(res, sp, path, ++lineno, macro_ok);
+ handle_attr_line(res, sp, path, ++lineno, flags);
sp = ep + more;
}
free(buf);
return res;
}
-static struct attr_stack *read_attr(const char *path, int macro_ok)
+static struct attr_stack *read_attr(const char *path, unsigned flags)
{
struct attr_stack *res;
if (direction == GIT_ATTR_CHECKOUT) {
- res = read_attr_from_index(path, macro_ok);
+ res = read_attr_from_index(path, flags);
if (!res)
- res = read_attr_from_file(path, macro_ok);
+ res = read_attr_from_file(path, flags);
}
else if (direction == GIT_ATTR_CHECKIN) {
- res = read_attr_from_file(path, macro_ok);
+ res = read_attr_from_file(path, flags);
if (!res)
/*
* There is no checked out .gitattributes file there, but
* we might have it in the index. We allow operation in a
* sparsely checked out work tree, so read from it.
*/
- res = read_attr_from_index(path, macro_ok);
+ res = read_attr_from_index(path, flags);
}
else
- res = read_attr_from_index(path, macro_ok);
+ res = read_attr_from_index(path, flags);
if (!res)
res = xcalloc(1, sizeof(*res));
return res;
@@ -493,6 +497,7 @@ static GIT_PATH_FUNC(git_path_info_attributes, INFOATTRIBUTES_FILE)
static void bootstrap_attr_stack(void)
{
struct attr_stack *elem;
+ unsigned flags = READ_ATTR_MACRO_OK;
if (attr_stack)
return;
@@ -503,7 +508,7 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;
if (git_attr_system()) {
- elem = read_attr_from_file(git_etc_gitattributes(), 1);
+ elem = read_attr_from_file(git_etc_gitattributes(), flags);
if (elem) {
elem->origin = NULL;
elem->prev = attr_stack;
@@ -514,7 +519,7 @@ static void bootstrap_attr_stack(void)
if (!git_attributes_file)
git_attributes_file = xdg_config_home("attributes");
if (git_attributes_file) {
- elem = read_attr_from_file(git_attributes_file, 1);
+ elem = read_attr_from_file(git_attributes_file, flags);
if (elem) {
elem->origin = NULL;
elem->prev = attr_stack;
@@ -523,7 +528,7 @@ static void bootstrap_attr_stack(void)
}
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
- elem = read_attr(GITATTRIBUTES_FILE, 1);
+ elem = read_attr(GITATTRIBUTES_FILE, flags);
elem->origin = xstrdup("");
elem->originlen = 0;
elem->prev = attr_stack;
@@ -532,7 +537,7 @@ static void bootstrap_attr_stack(void)
}
if (startup_info->have_repository)
- elem = read_attr_from_file(git_path_info_attributes(), 1);
+ elem = read_attr_from_file(git_path_info_attributes(), flags);
else
elem = NULL;
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 1/5] add open_nofollow() helper
From: Jeff King @ 2016-11-02 13:06 UTC (permalink / raw)
To: git
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
Some callers of open() would like to optionally use
O_NOFOLLOW, but it is not available on all platforms. We
could abstract this by publicly defining O_NOFOLLOW to 0 on
those platforms, but that leaves us no room for any
workarounds (e.g., by checking the file type via lstat()).
Instead, let's abstract it into its own function. We don't
implement any workarounds here, but it it would be easy to
add them later.
Signed-off-by: Jeff King <peff@peff.net>
---
I didn't add the workaround because I think the current callers are OK
with "best effort", and doing it non-racily is quite tricky (though we
might also be OK with a racy version; we are not trying to beat
/tmp races, but just making sure a checkout that we did is sane).
git-compat-util.h | 3 +++
wrapper.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092..a2cc33ebc 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1080,6 +1080,9 @@ int access_or_die(const char *path, int mode, unsigned flag);
/* Warn on an inaccessible file that ought to be accessible */
void warn_on_inaccessible(const char *path);
+/* Open with O_NOFOLLOW, if available on this platform */
+int open_nofollow(const char *path, int flags);
+
#ifdef GMTIME_UNRELIABLE_ERRORS
struct tm *git_gmtime(const time_t *);
struct tm *git_gmtime_r(const time_t *, struct tm *);
diff --git a/wrapper.c b/wrapper.c
index e7f197996..6bc7f24f5 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -679,3 +679,11 @@ void sleep_millisec(int millisec)
{
poll(NULL, 0, millisec);
}
+
+#ifndef O_NOFOLLOW
+#define O_NOFOLLOW 0
+#endif
+int open_nofollow(const char *path, int flags)
+{
+ return open(path, flags | O_NOFOLLOW);
+}
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 0/5] disallow symlinks for .gitignore and .gitattributes
From: Jeff King @ 2016-11-02 13:04 UTC (permalink / raw)
To: git
I noticed in a nearby discussion that we will follow in-filesystem
symlinks for in-tree .gitignore and .gitattributes files, but not when
those files are read out of the index (e.g., when switching branches).
This series teaches git to open those files with O_NOFOLLOW (when it is
available) to get more consistent behavior. Note that this only applies
to the in-tree versions; you can still symlink $GIT_DIR/info/attributes,
etc.
I stopped short of warning about symlinked entries in git-fsck, but
perhaps we would want to do that as well (doing it completely is tricky
because of all of the case-folding issues around matching pathnames).
[1/5]: add open_nofollow() helper
[2/5]: attr: convert "macro_ok" into a flags field
[3/5]: exclude: convert "check_index" into a flags field
[4/5]: attr: do not respect symlinks for in-tree .gitattributes
[5/5]: exclude: do not respect symlinks for in-tree .gitignore
attr.c | 58 ++++++++++++++++++++++++++++++++-------------------
dir.c | 20 +++++++++++++-----
dir.h | 2 +-
git-compat-util.h | 3 +++
t/t0003-attributes.sh | 31 +++++++++++++++++++++++++++
t/t0008-ignores.sh | 29 ++++++++++++++++++++++++++
wrapper.c | 8 +++++++
7 files changed, 123 insertions(+), 28 deletions(-)
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Junio C Hamano @ 2016-11-02 12:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1611021054030.3264@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 31 Oct 2016, Junio C Hamano wrote:
>
>> * jc/git-open-cloexec (2016-10-31) 3 commits
>> - sha1_file: stop opening files with O_NOATIME
>> - git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
>> - git_open(): untangle possible NOATIME and CLOEXEC interactions
>>
>> The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
>> opens has been simplified.
>
> This branch (in particular, the "use fcntl(2)" one) breaks the build on
> Windows. Until this breakage is resolved, I won't be able to see (nor
> report) any test breakages in `pu`.
Thanks for a heads-up. Anything in 'pu' won't be of importance
until 2.11 final, so please don't worry too much about it. Instead
please do make sure the tip of 'master' is OK.
Having said that, an incremental update or replacement to help
preparing it for post 2.11 final is appreciated ;-)
FYI, I may be offline for a few days.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Junio C Hamano @ 2016-11-02 12:55 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <20161102071646.GA5094@tb-raspi>
Torsten Bögershausen <tboegi@web.de> writes:
>> * tb/convert-stream-check (2016-10-27) 2 commits
>> - convert.c: stream and fast search for binary
>> - read-cache: factor out get_sha1_from_index() helper
>>
>
> It looks to be a good "proof of concept".
>
> The current series is far away from being ready, so please kick it
> out of pu and feel free to discard.
Thanks for a heads-up. Anything in 'pu' won't be of importance
until 2.11 final, so please don't worry too much about it.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Johannes Schindelin @ 2016-11-02 9:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwpgoqjct.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 31 Oct 2016, Junio C Hamano wrote:
> * jc/git-open-cloexec (2016-10-31) 3 commits
> - sha1_file: stop opening files with O_NOATIME
> - git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
> - git_open(): untangle possible NOATIME and CLOEXEC interactions
>
> The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
> opens has been simplified.
This branch (in particular, the "use fcntl(2)" one) breaks the build on
Windows. Until this breakage is resolved, I won't be able to see (nor
report) any test breakages in `pu`.
Ciao,
Dscho
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Torsten Bögershausen @ 2016-11-02 7:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwpgoqjct.fsf@gitster.mtv.corp.google.com>
>
> * tb/convert-stream-check (2016-10-27) 2 commits
> - convert.c: stream and fast search for binary
> - read-cache: factor out get_sha1_from_index() helper
>
> End-of-line conversion sometimes needs to see if the current blob
> in the index has NULs and CRs to base its decision. We used to
> always get a full statistics over the blob, but in many cases we
> can return early when we have seen "enough" (e.g. if we see a
> single NUL, the blob will be handled as binary). The codepaths
> have been optimized by using streaming interface.
>
> The tip seems to do too much in a single commit and may be better split.
> cf. <20161012134724.28287-1-tboegi@web.de>
> cf. <xmqqd1il5w4e.fsf@gitster.mtv.corp.google.com>
Reviews have been done, thanks everybody.
It looks to be a good "proof of concept".
The current series is far away from being ready, so please kick it
out of pu and feel free to discard.
^ permalink raw reply
* Re: [PATCH 3/3] abbrev: auto size the default abbreviation
From: Jeff King @ 2016-11-02 2:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <xmqqmvhimzrh.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 01, 2016 at 06:33:38PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Fri, Sep 30, 2016 at 05:19:37PM -0700, Junio C Hamano wrote:
> >
> >> Introduce a mechanism, where we estimate the number of objects in
> >> the repository upon the first request to abbreviate an object name
> >> with the default setting and come up with a sane default for the
> >> repository. Based on the expectation that we would see collision in
> >> a repository with 2^(2N) objects when using object names shortened
> >> to first N bits, use sufficient number of hexdigits to cover the
> >> number of objects in the repository. Each hexdigit (4-bits) we add
> >> to the shortened name allows us to have four times (2-bits) as many
> >> objects in the repository.
>
> I was idly browsing the draft release notes and then documentation
> and noticed that, even though the new default is to auto-scale,
> there is no mention of that in the documentation and there is no way
> to explicitly ask for auto-scaling.
>
> I wonder if we want to have something like this. I actually am
> inclined to drop the change to config.c and remove the new mention
> of "auto" in the documentation.
I doubt anybody cares that much either way, but in theory
core.abbrev=auto is a way to override core.abbrev=10 in /etc/gitconfig
or something. Though I'm having trouble envisioning a case where anybody
would set it in /etc/gitconfig, or why somebody would then want to
override that back to auto.
So I think it is fine either way (but I do agree that the core.abbrev
needs _some_ update to mention the auto-scaling behavior).
> diff --git a/config.c b/config.c
> index 83fdecb1bc..c363cca4a9 100644
> --- a/config.c
> +++ b/config.c
> @@ -834,10 +834,16 @@ static int git_default_core_config(const char *var, const char *value)
> }
>
> if (!strcmp(var, "core.abbrev")) {
> - int abbrev = git_config_int(var, value);
> - if (abbrev < minimum_abbrev || abbrev > 40)
> - return -1;
> - default_abbrev = abbrev;
> + if (!value)
> + return config_error_nonbool(var);
> + if (!strcasecmp(value, "auto"))
> + default_abbrev = -1;
> + else {
> + int abbrev = git_config_int(var, value);
> + if (abbrev < minimum_abbrev || abbrev > 40)
> + return -1;
> + default_abbrev = abbrev;
> + }
This isn't a new problem you added, but that "return -1" would probably
leave people confused:
$ git -c core.abbrev=2 log
fatal: unable to parse 'core.abbrev' from command-line config
Probably something like:
return error("abbrev length out of range: %d", abbrev);
would be more descriptive. I doubt it's a big deal in practice, though
(why would you set core.abbrev to something silly in the first place?).
-Peff
^ permalink raw reply
* Re: [PATCH v1 05/19] update-index: warn in case of split-index incoherency
From: Junio C Hamano @ 2016-11-02 1:37 UTC (permalink / raw)
To: Christian Couder
Cc: Duy Nguyen, Git Mailing List,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CAP8UFD18gcMJY7zjXw+ry6Kq2Foug9r0e=OVgZ_hcFkEVfnChA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
>>> Wrap this string and the one below with _() so they can be translated.
>>
>> True.
>>
>> I further wonder if a natural reaction from users after seeing this
>> message is "I do want to--what else would I use that option to run
>> you for? Just do as you are told, instead of telling me what to
>> do!". Is this warning really a good idea, or shouldn't these places
>> be setting the configuration?
>
> In 435ec090ec (config: add core.untrackedCache, 2016-01-27) we decided
> to just use warning() after discussing if we should instead set the
> configuration.
Yeah that seems to be the version that was committed. I then wonder
if the users would naturally have a similar raction to that warning
as well.
^ permalink raw reply
* Re: [PATCH 3/3] abbrev: auto size the default abbreviation
From: Junio C Hamano @ 2016-11-02 1:33 UTC (permalink / raw)
To: Jeff King; +Cc: git, Linus Torvalds
In-Reply-To: <20161003222701.za5njew33rqc5b6g@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Sep 30, 2016 at 05:19:37PM -0700, Junio C Hamano wrote:
>
>> Introduce a mechanism, where we estimate the number of objects in
>> the repository upon the first request to abbreviate an object name
>> with the default setting and come up with a sane default for the
>> repository. Based on the expectation that we would see collision in
>> a repository with 2^(2N) objects when using object names shortened
>> to first N bits, use sufficient number of hexdigits to cover the
>> number of objects in the repository. Each hexdigit (4-bits) we add
>> to the shortened name allows us to have four times (2-bits) as many
>> objects in the repository.
I was idly browsing the draft release notes and then documentation
and noticed that, even though the new default is to auto-scale,
there is no mention of that in the documentation and there is no way
to explicitly ask for auto-scaling.
I wonder if we want to have something like this. I actually am
inclined to drop the change to config.c and remove the new mention
of "auto" in the documentation.
Documentation/config.txt | 9 +++++----
config.c | 14 ++++++++++----
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66aae7..b02f8a4025 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -783,10 +783,11 @@ core.sparseCheckout::
linkgit:git-read-tree[1] for more information.
core.abbrev::
- Set the length object names are abbreviated to. If unspecified,
- many commands abbreviate to 7 hexdigits, which may not be enough
- for abbreviated object names to stay unique for sufficiently long
- time.
+ Set the length object names are abbreviated to. If
+ unspecified or set to "auto", an appropriate value is
+ computed based on the approximate number of packed objects
+ in your repository, which hopefully is enough for
+ abbreviated object names to stay unique for some time.
add.ignoreErrors::
add.ignore-errors (deprecated)::
diff --git a/config.c b/config.c
index 83fdecb1bc..c363cca4a9 100644
--- a/config.c
+++ b/config.c
@@ -834,10 +834,16 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.abbrev")) {
- int abbrev = git_config_int(var, value);
- if (abbrev < minimum_abbrev || abbrev > 40)
- return -1;
- default_abbrev = abbrev;
+ if (!value)
+ return config_error_nonbool(var);
+ if (!strcasecmp(value, "auto"))
+ default_abbrev = -1;
+ else {
+ int abbrev = git_config_int(var, value);
+ if (abbrev < minimum_abbrev || abbrev > 40)
+ return -1;
+ default_abbrev = abbrev;
+ }
return 0;
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox