* 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
* 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: 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
* 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: 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
* [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
* [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 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 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 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 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
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Jeff King @ 2016-11-02 17:40 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
On Wed, Nov 02, 2016 at 05:04:15PM +0000, Torsten Bögershausen wrote:
> > * 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.
Wow, haven't seen that bug in a while[1]. The problem is that STDIN is a
filehandle object, but older versions of perl do not automatically load
IO::Handle to get all of the methods. This was fixed in perl 5.13.
We can work around it with:
use IO::Handle;
at the top of the script. That should work everywhere, as IO::Handle has
been part of the core system for ages. But another option would be to
just turn on autoflush, with:
$| = 1;
at the top of the script (though it looks like we flush $debug, too, so
we'd probably need to "select $debug; $| = 1" there, too). The "use"
command is preferable IMHO.
> 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.
Yeah, we should be using PERL_PATH. Doing so inside the filter config
value is probably a pain due to shell quoting issues. But we could
use write_script() to get a local copy.
I'll see if I can work up a patch.
-Peff
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=261953
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Johannes Sixt @ 2016-11-02 17:43 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
Am 02.11.2016 um 18:04 schrieb Torsten Bögershausen:
>> * 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.
>
https://public-inbox.org/git/e8deda5f-11a6-1463-4fc5-25454084ccb1@kdbg.org/
-- Hannes
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2016, #09; Mon, 31)
From: Lars Schneider @ 2016-11-02 17:44 UTC (permalink / raw)
To: Torsten Bögershausen, mlbright; +Cc: Junio C Hamano, git
In-Reply-To: <20161102170415.GA6420@tb-raspi>
On 2 Nov 2016, at 17:04, Torsten Bögershausen <tboegi@web.de> wrote:
>> * 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 ?
Thanks for reporting! It surprises me that flush is not available. I don't have a 10.6 system to tests. Where you able to reproduce the problem on a newer system with an older Perl, too?
@Martin: do you have a clue?
I can't debug the issue right now (only on mobile) but I will look into it on Friday!
> t0021 uses the hard-coded path:
> t0021/rot13-filter.pl (around line 345) and the nice macro
> PERL_PATH from the Makefile is fully t
Can you check the line number? Rot13-filter.pl has only 192 lines. I'll look into the PERL_PATH.
> Commenting out the different "flush" makes the test hang, and I haven't digged further.
That would be expected because the pl file writes output to a file which is checked by the calling sh script.
Thanks,
Lars
^ permalink raw reply
* Re: [PATCH v11 13/14] convert: add filter.<driver>.process option
From: Johannes Sixt @ 2016-11-02 18:03 UTC (permalink / raw)
To: larsxschneider; +Cc: git, gitster, jnareb, peff, ramsay, tboegi
In-Reply-To: <20161016232038.84951-14-larsxschneider@gmail.com>
Am 17.10.2016 um 01:20 schrieb larsxschneider@gmail.com:
> +# Compare two files and ensure that `clean` and `smudge` respectively are
> +# called at least once if specified in the `expect` file. The actual
> +# invocation count is not relevant because their number can vary.
> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/
> +test_cmp_count () {
> + expect=$1
> + actual=$2
> + for FILE in "$expect" "$actual"
> + do
> + sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
> + sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
> + sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
This is not sufficiently portable. Some versions of uniq write the
count left-adjusted, not right-adjusted. How about this on top:
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index a20b9f58e3..f60858c517 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -40,10 +40,9 @@ test_cmp_count () {
actual=$2
for FILE in "$expect" "$actual"
do
- sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
- sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
- sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
- mv "$FILE.tmp" "$FILE"
+ sort "$FILE" | uniq -c |
+ sed -e "s/^ *[0-9][0-9]* *IN: /x IN: /" >"$FILE.tmp" &&
+ mv "$FILE.tmp" "$FILE" || return
done &&
test_cmp "$expect" "$actual"
}
-- Hannes
^ permalink raw reply related
* [PATCH 0/4] t0021 perl portability fixups
From: Jeff King @ 2016-11-02 18:16 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102174021.awhmbmuffbzv5b7t@sigill.intra.peff.net>
This series should fix the portability problem you saw (in the final
patch), and fixes the failure to use PERL_PATH along the way.
[1/4]: t0021: use write_script to create rot13 shell script
[2/4]: t0021: put $TEST_ROOT in $PATH
[3/4]: t0021: use $PERL_PATH for rot13-filter.pl
[4/4]: t0021: fix filehandle usage on older perl
t/t0021-conversion.sh | 30 ++++++++++++++++--------------
t/t0021/rot13-filter.pl | 2 +-
2 files changed, 17 insertions(+), 15 deletions(-)
mode change 100755 => 100644 t/t0021/rot13-filter.pl
^ permalink raw reply
* [PATCH 1/4] t0021: use write_script to create rot13 shell script
From: Jeff King @ 2016-11-02 18:17 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
This avoids us fooling around with $SHELL_PATH and the
executable bit ourselves.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index a20b9f58e..dfde22549 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -6,13 +6,11 @@ test_description='blob conversion via gitattributes'
TEST_ROOT="$(pwd)"
-cat <<EOF >"$TEST_ROOT/rot13.sh"
-#!$SHELL_PATH
+write_script <<\EOF "$TEST_ROOT/rot13.sh"
tr \
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
EOF
-chmod +x "$TEST_ROOT/rot13.sh"
generate_random_characters () {
LEN=$1
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 2/4] t0021: put $TEST_ROOT in $PATH
From: Jeff King @ 2016-11-02 18:18 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
We create a rot13.sh script in the trash directory, but need
to call it by its full path when we have moved our cwd to
another directory. Let's just put $TEST_ROOT in our $PATH so
that the script is always found.
This is a minor convenience for rot13.sh, but will be a
major one when we switch rot13-filter.pl to a script in the
same directory, as it means we will not have to deal with
shell quoting inside the filter-process config.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index dfde22549..c1ad20c61 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -5,6 +5,7 @@ test_description='blob conversion via gitattributes'
. ./test-lib.sh
TEST_ROOT="$(pwd)"
+PATH=$TEST_ROOT:$PATH
write_script <<\EOF "$TEST_ROOT/rot13.sh"
tr \
@@ -64,7 +65,7 @@ test_cmp_exclude_clean () {
# is equal to the committed content.
test_cmp_committed_rot13 () {
test_cmp "$1" "$2" &&
- "$TEST_ROOT/rot13.sh" <"$1" >expected &&
+ rot13.sh <"$1" >expected &&
git cat-file blob :"$2" >actual &&
test_cmp expected actual
}
@@ -513,7 +514,7 @@ test_expect_success PERL 'required process filter should process multiple packet
for FILE in "$TEST_ROOT"/*.file
do
cp "$FILE" . &&
- "$TEST_ROOT/rot13.sh" <"$FILE" >"$FILE.rot13"
+ rot13.sh <"$FILE" >"$FILE.rot13"
done &&
echo "*.file filter=protocol" >.gitattributes &&
@@ -616,7 +617,7 @@ test_expect_success PERL 'process filter should restart after unexpected write f
# Smudge failed
! test_cmp smudge-write-fail.o smudge-write-fail.r &&
- "$TEST_ROOT/rot13.sh" <smudge-write-fail.o >expected &&
+ rot13.sh <smudge-write-fail.o >expected &&
git cat-file blob :smudge-write-fail.r >actual &&
test_cmp expected actual
)
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 3/4] t0021: use $PERL_PATH for rot13-filter.pl
From: Jeff King @ 2016-11-02 18:20 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
The rot13-filter.pl script hardcodes "#!/usr/bin/perl", and
does not respect $PERL_PATH at all. That is a problem if the
system does not have perl at that path, or if it has a perl
that is too old to run a complicated script like the
rot13-filter (but PERL_PATH points to a more modern one).
We can fix this by using write_script() to create a new copy
of the script with the correct #!-line. In theory we could
move the whole script inside t0021-conversion.sh rather than
having it as an auxiliary file, but it's long enough that
it just makes things harder to read.
As a bonus, we can stop using the full path to the script in
the filter-process config we add (because the trash
directory is in our PATH). Not only is this shorter, but it
sidesteps any shell-quoting issues. The original was broken
when $TEST_DIRECTORY contained a space, because it was
interpolated in the outer script.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t0021-conversion.sh | 19 +++++++++++--------
t/t0021/rot13-filter.pl | 1 -
2 files changed, 11 insertions(+), 9 deletions(-)
mode change 100755 => 100644 t/t0021/rot13-filter.pl
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index c1ad20c61..a8fa52148 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -13,6 +13,9 @@ tr \
'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
EOF
+write_script rot13-filter.pl "$PERL_PATH" \
+ <"$TEST_DIRECTORY"/t0021/rot13-filter.pl
+
generate_random_characters () {
LEN=$1
NAME=$2
@@ -341,7 +344,7 @@ test_expect_success 'diff does not reuse worktree files that need cleaning' '
'
test_expect_success PERL 'required process filter should filter data' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -434,7 +437,7 @@ test_expect_success PERL 'required process filter should filter data' '
test_expect_success PERL 'required process filter takes precedence' '
test_config_global filter.protocol.clean false &&
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -459,7 +462,7 @@ test_expect_success PERL 'required process filter takes precedence' '
'
test_expect_success PERL 'required process filter should be used only for "clean" operation only' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean" &&
rm -rf repo &&
mkdir repo &&
(
@@ -494,7 +497,7 @@ test_expect_success PERL 'required process filter should be used only for "clean
'
test_expect_success PERL 'required process filter should process multiple packets' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
@@ -554,7 +557,7 @@ test_expect_success PERL 'required process filter should process multiple packet
'
test_expect_success PERL 'required process filter with clean error should fail' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
test_config_global filter.protocol.required true &&
rm -rf repo &&
mkdir repo &&
@@ -573,7 +576,7 @@ test_expect_success PERL 'required process filter with clean error should fail'
'
test_expect_success PERL 'process filter should restart after unexpected write failure' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
@@ -624,7 +627,7 @@ test_expect_success PERL 'process filter should restart after unexpected write f
'
test_expect_success PERL 'process filter should not be restarted if it signals an error' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
@@ -663,7 +666,7 @@ test_expect_success PERL 'process filter should not be restarted if it signals a
'
test_expect_success PERL 'process filter abort stops processing of all further files' '
- test_config_global filter.protocol.process "$TEST_DIRECTORY/t0021/rot13-filter.pl clean smudge" &&
+ test_config_global filter.protocol.process "rot13-filter.pl clean smudge" &&
rm -rf repo &&
mkdir repo &&
(
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
old mode 100755
new mode 100644
index ae4c50f5c..e3ea58e1e
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -1,4 +1,3 @@
-#!/usr/bin/perl
#
# Example implementation for the Git filter protocol version 2
# See Documentation/gitattributes.txt, section "Filter Protocol"
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* [PATCH 4/4] t0021: fix filehandle usage on older perl
From: Jeff King @ 2016-11-02 18:23 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Sixt, Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102181625.e2uprqdlzl7z2xrz@sigill.intra.peff.net>
The rot13-filter.pl script calls methods on implicitly
defined filehandles (STDOUT, and the result of an open()
call). Prior to perl 5.13, these methods are not
automatically loaded, and perl will complain with:
Can't locate object method "flush" via package "IO::Handle"
Let's explicitly load IO::File (which inherits from
IO::Handle). That's more than we need for just "flush", but
matches what perl has done since:
http://perl5.git.perl.org/perl.git/commit/15e6cdd91beb4cefae4b65e855d68cf64766965d
Signed-off-by: Jeff King <peff@peff.net>
---
I see J6t solved this a week ago using "FileHandle". These days that is
basically a compatibility synonym for IO::File. I think both should be
available pretty much everywhere, so I went with IO::File for the
reasons above. But if that doesn't work for some reason, switching to
"use FileHandle" should be OK, too.
I don't have an old enough perl easily available to test it either way.
t/t0021/rot13-filter.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t0021/rot13-filter.pl b/t/t0021/rot13-filter.pl
index e3ea58e1e..4d5697ee5 100644
--- a/t/t0021/rot13-filter.pl
+++ b/t/t0021/rot13-filter.pl
@@ -21,6 +21,7 @@
use strict;
use warnings;
+use IO::File;
my $MAX_PACKET_CONTENT_SIZE = 65516;
my @capabilities = @ARGV;
--
2.11.0.rc0.258.gf434c15
^ permalink raw reply related
* Re: [PATCH 0/5] disallow symlinks for .gitignore and .gitattributes
From: Stefan Beller @ 2016-11-02 19:46 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20161102130432.d3zprdul4sqgcfwu@sigill.intra.peff.net>
On Wed, Nov 2, 2016 at 6:04 AM, Jeff King <peff@peff.net> wrote:
>
> attr.c | 58 ++++++++++++++++++++++++++++++++-------------------
$ git diff --stat origin/master..origin/sb/attr |grep attr.c
attr.c | 531 +-
From a cursory read of your series this may result in a merge
conflict, but would be
easily fixable (changed signature of functions that clash).
^ permalink raw reply
* Re: [PATCH 0/5] disallow symlinks for .gitignore and .gitattributes
From: Jeff King @ 2016-11-02 19:56 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kY195Bff5bUVm44Bx7+XzDDehYbRxgdii4s040pQXRRSQ@mail.gmail.com>
On Wed, Nov 02, 2016 at 12:46:28PM -0700, Stefan Beller wrote:
> On Wed, Nov 2, 2016 at 6:04 AM, Jeff King <peff@peff.net> wrote:
>
> >
> > attr.c | 58 ++++++++++++++++++++++++++++++++-------------------
>
> $ git diff --stat origin/master..origin/sb/attr |grep attr.c
> attr.c | 531 +-
>
> From a cursory read of your series this may result in a merge
> conflict, but would be
> easily fixable (changed signature of functions that clash).
Yeah, I knew you guys were doing some refactoring of the attribute code
elsewhere, but hadn't actually seen how bad the damage was. I just did
the merge with sb/attr, though, and the conflicts are quite trivial
(mostly s/1/flags/ in bootstrap_attr_stack()).
I'm happy to re-roll on a different base if sb/attr graduates first, but
I suspect Junio can just resolve the conflicts at merge time.
-Peff
^ permalink raw reply
* send-email garbled header with trailing doublequote in email
From: Andrea Arcangeli @ 2016-11-02 20:27 UTC (permalink / raw)
To: git
Hello,
send-email gets confused with one trailing " at the end of the
email. Details and how to reproduce below, it breaks also with
upstream git version 2.10.2.dirty.
Feel free to CC me if you need any further info.
Thanks,
Andrea
----- Forwarded message from Andrea Arcangeli <aarcange@redhat.com> -----
Date: Wed, 2 Nov 2016 21:07:02 +0100
From: Andrea Arcangeli <aarcange@redhat.com>
To: linux-mm@kvack.org
Subject: Re: [PATCH 00/33] userfaultfd tmpfs/hugetlbfs/non-cooperative
User-Agent: Mutt/1.7.1 (2016-10-04)
FYI: apparently I hit a git bug in this submit... reproducible with
the below command:
git send-email -1 --to '"what ever" <--your--@--email--.com>"'
after replacing --your--@email--.com with your own email.
*snip*
Dry-OK. Log says:
To: "what ever" " <--your--@--email--.com>
*snip*
X-Mailer: git-send-email 2.7.3
Result: OK
It's not ok if the --dry-run outputs the above with a fine header, but
the actual header in the email data is different. Of course I tested
--dry-run twice and it was fine like the above is fine as well.
*snip*
----- End forwarded message -----
^ permalink raw reply
* Re: [PATCH 4/4] t0021: fix filehandle usage on older perl
From: Torsten Bögershausen @ 2016-11-02 20:54 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Lars Schneider, Junio C Hamano, git
In-Reply-To: <20161102182301.hezruhtuhra6uoft@sigill.intra.peff.net>
> +use IO::File;
That did the trick (the J6T version work as well)
Thanks for the fast responses.
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.10.2
From: Johannes Schindelin @ 2016-11-02 20:58 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]
Dear Git users,
It is my pleasure to announce that Git for Windows 2.10.2 is available from:
https://git-for-windows.github.io/
Changes since Git for Windows v2.10.1(2) (October 13th 2016)
Git for windows v2.10.1(2) was a MinGit-only release (i.e. there was no
Git for windows installer for that version).
New Features
• Comes with Git v2.10.2.
• Comes with Git Credential Manager v1.8.1.
• Comes with cURL v7.51.0.
• Git for Windows can now be built easily with Visual C++ 2015.
• The installer now logs post-install errors more verbosely.
• A new option asks the installer to skip installation if Git's files
are in use.
• A new option asks the installer to quietly skip downgrading Git for
Windows, without indicating failure.
• There is now an explicit option for symbolic link support,
including a link to a more verbose explanation of the issue.
Bug Fixes
• when upgrading Git for Windows, SSH agent processes are now
auto-terminated.
• When trying to install/upgrade on a Windows version that is no
longer supported, we now refuse to do so.
Filename | SHA-256
-------- | -------
Git-2.10.2-64-bit.exe | 57238ebf86f8b51e32cab44bb91c8060e04774676b77b9fb92f78e7bc7e9a179
Git-2.10.2-32-bit.exe | 8b15054a4ea2dd5d2be0471a430d8ae7c772b94e1a1048221083a0040011011c
PortableGit-2.10.2-64-bit.7z.exe | 101314826892480043d5b11989726fc8ee448991eb7b0a1c61aca751161bc15b
PortableGit-2.10.2-32-bit.7z.exe | edc616817e96a6f15246bb0dd93c97e53d38d3b2a0b7375f26bd0bd082c41a73
MinGit-2.10.2-64-bit.zip | a10de5d5a8e71e207eff20d833ca56902a0668940e3def5f21d089e4f533ea40
MinGit-2.10.2-32-bit.zip | 2b191598bcb37565a2b80729ef8d00c03df02b75f6b9d012080c458999f89cc0
Git-2.10.2-64-bit.tar.bz2 | 7df449ac1813876b5a2e9215d94bca9458c2e0870c65e5b78bd7fc2a448a2a90
Git-2.10.2-32-bit.tar.bz2 | 3f8d0bebc53fabad863b8fe352a317fde61833efa4750f96656cf95017a621e8
Ciao,
Johannes
--
You received this message because you are subscribed to the Google Groups "git-for-windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to git-for-windows+unsubscribe@googlegroups.com.
To post to this group, send email to git-for-windows@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/git-for-windows/20161102204358.6856-1-johannes.schindelin%40gmx.de.
For more options, visit https://groups.google.com/d/optout.
^ 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