Re: [PATCH 4/4] sequencer: use trailer's trailer layout
Jonathan Tan writes:
> @@ -26,30 +27,6 @@ static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE)
> static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR)
> static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_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] == ')';
> -}
We lost two helper functions here, one to detect "Mail-header: like"
line, the other to detect "(cherry picked from") line. Let's see
how the updated caller can do without these. We know that both of
these can be caught if we grabbed the block of trailer block.
> @@ -59,49 +36,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;
So we feed the thing to trailer_info_get() which will find the
trailer block. If there is no trailer block, start and end will
point at the same place, which is trivial.
>
> - /* 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;
Then we scan the trailer block and see if we are looking at the same
s-o-b line as we are asked to look for, and if it is at the last
logical line in the trailer block.
> + trailer_info_release(&info);
>
> - if (!(found_rfc2822 ||
> - is_cherry_picked_from_line(buf + i, k - i - 1)))
> - return 0;
We used to reject a "last paragraph" that has "cruft" other than
"Mail-header: like" line or "(cherry-picked from" line. By reusing
the trailer code, we are getting consistently looser with its logic.
> - }
> - if (found_sob == i)
> + if (found_sob_last)
> return 3;
> if (found_sob)
> return 2;
I found it surprising that you said "commit -s", "cherry-pick -x"
and "format-patch -s" are covered by this patch and saw a change
only to sequencer.c.
It turns out append_signoff() is the central function that is shared
among builtin/commit.c::prepare_to_commit() that does "commit -s",
log-tree.c::show_log() that is called by "format-patch -s", and
sequencer.c::do_recursive_merge() that do_pick_commit() hence "git
cherry-pick -s" uses. And that function decides where to put a new
S-o-b by calling this has_conforming_footer() function.
In addition, do_pick_commit() also calls has_conforming_footer() to
decide where to add "(cherry picked from".
Whoever did the sequencer.c should be proud to structure the code to
make this change so easy to make (I know it is not me, and you
Jonathan know it is not you).
Nicely done by both of you.
> 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 "
>
> mesg_broken_footer="$mesg_no_footer
>
> -The signed-off-by string should begin with the words Signed-off-b
Re: [PATCH v2 1/6] submodules: add helper functions to determine presence of submodules
On Mon, Oct 31, 2016 at 3:38 PM, Brandon Williams wrote:
> +int is_submodule_checked_out(const char *path)
> +{
> + int ret = 0;
> + struct strbuf buf = STRBUF_INIT;
> +
> + strbuf_addf(&buf, "%s/.git", path);
> + ret = file_exists(buf.buf);
I think we can be more tight here; instead of checking
if the file or directory exists, we should be checking if
it is a valid git directory, i.e. s/file_exists/resolve_gitdir/
which returns a path to the actual git dir (in case of a .gitlink)
or NULL when nothing is found that looks like a git directory or
pointer to it.
> +
> + strbuf_release(&buf);
> + return ret;
> +}
> +
> int parse_submodule_update_strategy(const char *value,
> struct submodule_update_strategy *dst)
> {
> diff --git a/submodule.h b/submodule.h
> index d9e197a..bd039ca 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -37,6 +37,8 @@ void set_diffopt_flags_from_submodule_config(struct
> diff_options *diffopt,
> const char *path);
> int submodule_config(const char *var, const char *value, void *cb);
> void gitmodules_config(void);
> +extern int is_submodule_initialized(const char *path);
> +extern int is_submodule_checked_out(const char *path);
no need to put extern for function names. (no other functions in this
header are extern. so local consistency maybe? I'd also claim that
all other extern functions in headers ought to be declared without
being extern)
Also naming: I'd go with
is_submodule_populated ;)
as it will tell whether this function will tell you if there is a valid
submodule (and not just an empty dir as a place holder).
You don't have to run "git checkout" to arrive in that state,
but a plumbing command such as read_tree may have been used.
Re: [ANNOUNCE] Git v2.11.0-rc0
On Mon, Oct 31, 2016 at 02:49:42PM -0700, Junio C Hamano wrote: > [snip] Hello, I noticed a few minor typos in the changelog. > * The default abbreviation length, which has historically been 7, now >scales as the repository grows, using the approximate number of >objects in the reopsitory and a bit of math around the birthday ^^ repository >paradox. The logic suggests to use 12 hexdigits for the Linux >kernel, and 9 to 10 for Git itself. > > > Updates since v2.10 > --- > > [snip] > > * "git clone --resurse-submodules --reference $path $URL" is a way to ^^^ recurse >reduce network transfer cost by borrowing objects in an existing >$path repository when cloning the superproject from $URL; it >learned to also peek into $path for presense of corresponding presence > [snip] > > * Output from "git diff" can be made easier to read by selecting >which lines are common and which lines are added/deleted >intelligently when the lines before and after the changed section >are the same. A command line option is added to help with the >experiment to find a good heuristics. Maybe the name of the command line option should be added here. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 signature.asc Description: PGP signature
Re: [PATCH 3/4] trailer: have function to describe trailer layout
Jonathan Tan writes:
> static char *separators = ":";
>
> +static int configured = 0;
Avoid initializing a static to 0 or NULL; instead let .bss take care
of it.
> static const char *token_from_item(struct arg_item *item, char *tok)
> {
> if (item->conf.key)
> @@ -872,59 +885,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);
OK, it needs a reading of trailer_info_get() first to understand the
remainder of this function. The function would grab these fields
into info, among doing other things.
> /* 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");
... and one of the "other things" include setting the
->blank_line_before_trailer field.
> - /* 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)
> + char *trailer = info.trailers[i];
> + if (trailer[0] == comment_line_char)
> continue;
And info.trailers[] is no longer an array of strbuf; it is an array
of "char *", which I alluded to during my review of [2/4]. Looking
good.
> - 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);
> - continue;
> - }
> - separator_pos = find_separator(trailer->buf, separators);
> + separator_pos = find_separator(trailer, separators);
... presumably, the line-folding is already handled in
trailer_info_get(), so we do not need the "last" handling.
> if (separator_pos >= 1) {
... and it it is a "mail-header: looking" one, then add it one way.
> - parse_trailer(&tok, &val, NULL, trailer->buf,
> - separator_pos);
> - last = add_trailer_item(head,
> - strbuf_detach(&tok, NULL),
> - strbuf_detach(&val, NULL));
> + parse_trailer(&tok, &val, NULL, trailer,
> + separator_pos);
> + add_trailer_item(head,
> + strbuf_detach(&tok, NULL),
> + strbuf_detach(&val, NULL));
> } else {
> - strbuf_addbuf(&val, trailer);
> + strbuf_addstr(&val, trailer);
... otherwise add it another way.
> 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;
> }
Nicely done.
> @@ -1004,3 +999,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,
>
[PATCH v2 3/6] grep: add submodules as a grep source type
Add `GREP_SOURCE_SUBMODULE` as a grep_source type and cases for this new
type in the various switch statements in grep.c.
When initializing a grep_source with type `GREP_SOURCE_SUBMODULE` the
identifier can either be NULL (to indicate that the working tree will be
used) or a SHA1 (the REV of the submodule to be grep'd). If the
identifier is a SHA1 then we want to fall through to the
`GREP_SOURCE_SHA1` case to handle the copying of the SHA1.
Signed-off-by: Brandon Williams
---
grep.c | 16 +++-
grep.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/grep.c b/grep.c
index 1194d35..0dbdc1d 100644
--- a/grep.c
+++ b/grep.c
@@ -1735,12 +1735,23 @@ void grep_source_init(struct grep_source *gs, enum
grep_source_type type,
case GREP_SOURCE_FILE:
gs->identifier = xstrdup(identifier);
break;
+ case GREP_SOURCE_SUBMODULE:
+ if (!identifier) {
+ gs->identifier = NULL;
+ break;
+ }
+ /*
+* FALL THROUGH
+* If the identifier is non-NULL (in the submodule case) it
+* will be a SHA1 that needs to be copied.
+*/
case GREP_SOURCE_SHA1:
gs->identifier = xmalloc(20);
hashcpy(gs->identifier, identifier);
break;
case GREP_SOURCE_BUF:
gs->identifier = NULL;
+ break;
}
}
@@ -1760,6 +1771,7 @@ void grep_source_clear_data(struct grep_source *gs)
switch (gs->type) {
case GREP_SOURCE_FILE:
case GREP_SOURCE_SHA1:
+ case GREP_SOURCE_SUBMODULE:
free(gs->buf);
gs->buf = NULL;
gs->size = 0;
@@ -1831,8 +1843,10 @@ static int grep_source_load(struct grep_source *gs)
return grep_source_load_sha1(gs);
case GREP_SOURCE_BUF:
return gs->buf ? 0 : -1;
+ case GREP_SOURCE_SUBMODULE:
+ break;
}
- die("BUG: invalid grep_source type");
+ die("BUG: invalid grep_source type to load");
}
void grep_source_load_driver(struct grep_source *gs)
diff --git a/grep.h b/grep.h
index 5856a23..267534c 100644
--- a/grep.h
+++ b/grep.h
@@ -161,6 +161,7 @@ struct grep_source {
GREP_SOURCE_SHA1,
GREP_SOURCE_FILE,
GREP_SOURCE_BUF,
+ GREP_SOURCE_SUBMODULE,
} type;
void *identifier;
--
2.8.0.rc3.226.g39d4020
[PATCH v2 5/6] grep: enable recurse-submodules to work on objects
Teach grep to recursively search in submodules when provided with a
object. This allows grep to search a submodule based on the state
of the submodule that is present in a commit of the super project.
When grep is provided with a object, the name of the object is
prefixed to all output. In order to provide uniformity of output
between the parent and child processes the option `--parent-basename`
has been added so that the child can preface all of it's output with the
name of the parent's object instead of the name of the commit SHA1 of
the submodule. This changes output from the command
`git grep -e. -l --recurse-submodules HEAD` from:
HEAD:file
:sub/file
to:
HEAD:file
HEAD:sub/file
Signed-off-by: Brandon Williams
---
Documentation/git-grep.txt | 13 +-
builtin/grep.c | 83 +++---
t/t7814-grep-recurse-submodules.sh | 44 +++-
3 files changed, 131 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 17aa1ba..386a868 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,7 +26,7 @@ SYNOPSIS
[--threads ]
[-f ] [-e]
[--and|--or|--not|(|)|-e ...]
- [--recurse-submodules]
+ [--recurse-submodules] [--parent-basename]
[ [--[no-]exclude-standard] [--cached | --no-index | --untracked] |
...]
[--] [...]
@@ -91,7 +91,16 @@ OPTIONS
--recurse-submodules::
Recursively search in each submodule that has been initialized and
- checked out in the repository.
+ checked out in the repository. When used in combination with the
+option the prefix of all submodule output will be the name of
+ the parent project's object.
+
+--parent-basename::
+ For internal use only. In order to produce uniform output with the
+ --recurse-submodules option, this option can be used to provide the
+ basename of a parent's object to a submodule so the submodule
+ can prefix its output with the parent's name rather than the SHA1 of
+ the submodule.
-a::
--text::
diff --git a/builtin/grep.c b/builtin/grep.c
index cf4f51e..2f10930 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -19,6 +19,7 @@
#include "dir.h"
#include "pathspec.h"
#include "submodule.h"
+#include "submodule-config.h"
static char const * const grep_usage[] = {
N_("git grep [] [-e] [...] [[--] ...]"),
@@ -28,6 +29,7 @@ static char const * const grep_usage[] = {
static const char *super_prefix;
static int recurse_submodules;
static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+static const char *parent_basename;
static int grep_submodule_launch(struct grep_opt *opt,
const struct grep_source *gs);
@@ -535,19 +537,53 @@ static int grep_submodule_launch(struct grep_opt *opt,
{
struct child_process cp = CHILD_PROCESS_INIT;
int status, i;
+ const char *end_of_base;
+ const char *name;
struct work_item *w = opt->output_priv;
+ end_of_base = strchr(gs->name, ':');
+ if (end_of_base)
+ name = end_of_base + 1;
+ else
+ name = gs->name;
+
prepare_submodule_repo_env(&cp.env_array);
/* Add super prefix */
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
super_prefix ? super_prefix : "",
-gs->name);
+name);
argv_array_push(&cp.args, "grep");
+ /*
+* Add basename of parent project
+* When performing grep on a object the filename is prefixed
+* with the object's name: ':filename'. In order to
+* provide uniformity of output we want to pass the name of the
+* parent project's object name to the submodule so the submodule can
+* prefix its output with the parent's name and not its own SHA1.
+*/
+ if (end_of_base)
+ argv_array_pushf(&cp.args, "--parent-basename=%.*s",
+(int) (end_of_base - gs->name),
+gs->name);
+
/* Add options */
- for (i = 0; i < submodule_options.argc; i++)
+ for (i = 0; i < submodule_options.argc; i++) {
+ /*
+* If there is a identifier for the submodule, add the
+* rev after adding the submodule options but before the
+* pathspecs. To do this we listen for the '--' and insert the
+* sha1 before pushing the '--' onto the child process argv
+* array.
+*/
+ if (gs->identifier &&
+ !strcmp("--", submodule_options.argv[i])) {
+ argv_array_push(&cp.args, sha1_to_hex(gs->identifier));
+ }
+
argv_array_push(&cp.args, submodule_options.argv[i]);
+ }
[PATCH v2 1/6] submodules: add helper functions to determine presence of submodules
Add two helper functions to submodules.c.
`is_submodule_initialized()` checks if a submodule has been initialized
at a given path and `is_submodule_checked_out()` check if a submodule
has been checked out at a given path.
Signed-off-by: Brandon Williams
---
submodule.c | 39 +++
submodule.h | 2 ++
2 files changed, 41 insertions(+)
diff --git a/submodule.c b/submodule.c
index 6f7d883..ff4e7b2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,45 @@ void gitmodules_config(void)
}
}
+/*
+ * Determine if a submodule has been initialized at a given 'path'
+ */
+int is_submodule_initialized(const char *path)
+{
+ int ret = 0;
+ const struct submodule *module = NULL;
+
+ module = submodule_from_path(null_sha1, path);
+
+ if (module) {
+ struct strbuf buf = STRBUF_INIT;
+ char *submodule_url = NULL;
+
+ strbuf_addf(&buf, "submodule.%s.url", module->name);
+ ret = !git_config_get_string(buf.buf, &submodule_url);
+
+ free(submodule_url);
+ strbuf_release(&buf);
+ }
+
+ return ret;
+}
+
+/*
+ * Determine if a submodule has been checked out at a given 'path'
+ */
+int is_submodule_checked_out(const char *path)
+{
+ int ret = 0;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s/.git", path);
+ ret = file_exists(buf.buf);
+
+ strbuf_release(&buf);
+ return ret;
+}
+
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst)
{
diff --git a/submodule.h b/submodule.h
index d9e197a..bd039ca 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,8 @@ void set_diffopt_flags_from_submodule_config(struct
diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern int is_submodule_initialized(const char *path);
+extern int is_submodule_checked_out(const char *path);
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
const char *submodule_strategy_to_string(const struct
submodule_update_strategy *s);
--
2.8.0.rc3.226.g39d4020
[PATCH v2 2/6] submodules: load gitmodules file from commit sha1
Teach submodules to load a '.gitmodules' file from a commit sha1. This
enables the population of the submodule_cache to be based on the state
of the '.gitmodules' file from a particular commit.
Signed-off-by: Brandon Williams
---
cache.h| 2 ++
config.c | 8
submodule-config.c | 6 +++---
submodule-config.h | 3 +++
submodule.c| 12
submodule.h| 1 +
6 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 1be6526..559a461 100644
--- a/cache.h
+++ b/cache.h
@@ -1690,6 +1690,8 @@ extern int git_default_config(const char *, const char *,
void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
const char *name, const char *buf,
size_t len, void *data);
+extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
+const unsigned char *sha1, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern void git_config(config_fn_t fn, void *);
diff --git a/config.c b/config.c
index 83fdecb..4d78e72 100644
--- a/config.c
+++ b/config.c
@@ -1214,10 +1214,10 @@ int git_config_from_mem(config_fn_t fn, const enum
config_origin_type origin_typ
return do_config_from(&top, fn, data);
}
-static int git_config_from_blob_sha1(config_fn_t fn,
-const char *name,
-const unsigned char *sha1,
-void *data)
+int git_config_from_blob_sha1(config_fn_t fn,
+ const char *name,
+ const unsigned char *sha1,
+ void *data)
{
enum object_type type;
char *buf;
diff --git a/submodule-config.c b/submodule-config.c
index 098085b..8b9a2ef 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -379,9 +379,9 @@ static int parse_config(const char *var, const char *value,
void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
- unsigned char *gitmodules_sha1,
- struct strbuf *rev)
+int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev)
{
int ret = 0;
diff --git a/submodule-config.h b/submodule-config.h
index d05c542..78584ba 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -29,6 +29,9 @@ const struct submodule *submodule_from_name(const unsigned
char *commit_sha1,
const char *name);
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path);
+extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev);
void submodule_free(void);
#endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index ff4e7b2..19dfbd4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,18 @@ void gitmodules_config(void)
}
}
+void gitmodules_config_sha1(const unsigned char *commit_sha1)
+{
+ struct strbuf rev = STRBUF_INIT;
+ unsigned char sha1[20];
+
+ if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
+ git_config_from_blob_sha1(submodule_config, rev.buf,
+ sha1, NULL);
+ }
+ strbuf_release(&rev);
+}
+
/*
* Determine if a submodule has been initialized at a given 'path'
*/
diff --git a/submodule.h b/submodule.h
index bd039ca..9a24ac8 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct
diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path);
extern int is_submodule_checked_out(const char *path);
int parse_submodule_update_strategy(const char *value,
--
2.8.0.rc3.226.g39d4020
[PATCH v2 4/6] grep: optionally recurse into submodules
Allow grep to recognize submodules and recursively search for patterns in
each submodule. This is done by forking off a process to recursively
call grep on each submodule. The top level --super-prefix option is
used to pass a path to the submodule which can in turn be used to
prepend to output or in pathspec matching logic.
Recursion only occurs for submodules which have been initialized and
checked out by the parent project. If a submodule hasn't been
initialized and checked out it is simply skipped.
In order to support the existing multi-threading infrastructure in grep,
output from each child process is captured in a strbuf so that it can be
later printed to the console in an ordered fashion.
To limit the number of theads that are created, each child process has
half the number of threads as its parents (minimum of 1), otherwise we
potentailly have a fork-bomb.
Signed-off-by: Brandon Williams
---
Documentation/git-grep.txt | 5 +
builtin/grep.c | 300 ++---
git.c | 2 +-
t/t7814-grep-recurse-submodules.sh | 99
4 files changed, 385 insertions(+), 21 deletions(-)
create mode 100755 t/t7814-grep-recurse-submodules.sh
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 0ecea6e..17aa1ba 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,6 +26,7 @@ SYNOPSIS
[--threads ]
[-f ] [-e]
[--and|--or|--not|(|)|-e ...]
+ [--recurse-submodules]
[ [--[no-]exclude-standard] [--cached | --no-index | --untracked] |
...]
[--] [...]
@@ -88,6 +89,10 @@ OPTIONS
mechanism. Only useful when searching files in the current directory
with `--no-index`.
+--recurse-submodules::
+ Recursively search in each submodule that has been initialized and
+ checked out in the repository.
+
-a::
--text::
Process binary files as if they were text.
diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6a..cf4f51e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,12 +18,20 @@
#include "quote.h"
#include "dir.h"
#include "pathspec.h"
+#include "submodule.h"
static char const * const grep_usage[] = {
N_("git grep [] [-e] [...] [[--] ...]"),
NULL
};
+static const char *super_prefix;
+static int recurse_submodules;
+static struct argv_array submodule_options = ARGV_ARRAY_INIT;
+
+static int grep_submodule_launch(struct grep_opt *opt,
+const struct grep_source *gs);
+
#define GREP_NUM_THREADS_DEFAULT 8
static int num_threads;
@@ -174,7 +182,10 @@ static void *run(void *arg)
break;
opt->output_priv = w;
- hit |= grep_source(opt, &w->source);
+ if (w->source.type == GREP_SOURCE_SUBMODULE)
+ hit |= grep_submodule_launch(opt, &w->source);
+ else
+ hit |= grep_source(opt, &w->source);
grep_source_clear_data(&w->source);
work_done(w);
}
@@ -300,6 +311,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned
char *sha1,
if (opt->relative && opt->prefix_length) {
quote_path_relative(filename + tree_name_len, opt->prefix,
&pathbuf);
strbuf_insert(&pathbuf, 0, filename, tree_name_len);
+ } else if (super_prefix) {
+ strbuf_add(&pathbuf, filename, tree_name_len);
+ strbuf_addstr(&pathbuf, super_prefix);
+ strbuf_addstr(&pathbuf, filename + tree_name_len);
} else {
strbuf_addstr(&pathbuf, filename);
}
@@ -328,10 +343,13 @@ static int grep_file(struct grep_opt *opt, const char
*filename)
{
struct strbuf buf = STRBUF_INIT;
- if (opt->relative && opt->prefix_length)
+ if (opt->relative && opt->prefix_length) {
quote_path_relative(filename, opt->prefix, &buf);
- else
+ } else {
+ if (super_prefix)
+ strbuf_addstr(&buf, super_prefix);
strbuf_addstr(&buf, filename);
+ }
#ifndef NO_PTHREADS
if (num_threads) {
@@ -378,31 +396,258 @@ static void run_pager(struct grep_opt *opt, const char
*prefix)
exit(status);
}
-static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
int cached)
+static void compile_submodule_options(const struct grep_opt *opt,
+ const struct pathspec *pathspec,
+ int cached, int untracked,
+ int opt_exclude, int use_index,
+ int pattern_type_arg)
+{
+ struct grep_pat *pattern;
+ int i;
+
+ if (recurse_submodules)
+ argv_array_push(&submodule_options, "--recurse-submodules");
+
+ if
[PATCH v2 6/6] grep: search history of moved submodules
If a submodule was renamed at any point since it's inception then if you
were to try and grep on a commit prior to the submodule being moved, you
wouldn't be able to find a working directory for the submodule since the
path in the past is different from the current path.
This patch teaches grep to find the .git directory for a submodule in
the parents .git/modules/ directory in the event the path to the
submodule in the commit that is being searched differs from the state of
the currently checked out commit. If found, the child process that is
spawned to grep the submodule will chdir into its gitdir instead of a
working directory.
In order to override the explicit setting of submodule child process's
gitdir environment variable (which was introduced in '10f5c526')
`GIT_DIR_ENVIORMENT` needs to be pushed onto child process's env_array.
This allows the searching of history from a submodule's gitdir, rather
than from a working directory.
Signed-off-by: Brandon Williams
---
builtin/grep.c | 18 +
t/t7814-grep-recurse-submodules.sh | 41 ++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 2f10930..032d476 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -548,6 +548,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
name = gs->name;
prepare_submodule_repo_env(&cp.env_array);
+ argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
/* Add super prefix */
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
@@ -612,10 +613,19 @@ static int grep_submodule(struct grep_opt *opt, const
unsigned char *sha1,
{
if (!(is_submodule_initialized(path) &&
is_submodule_checked_out(path))) {
- warning("skiping submodule '%s%s' since it is not initialized
and checked out",
- super_prefix ? super_prefix : "",
- path);
- return 0;
+ /*
+* If searching history, check for the presense of the
+* submodule's gitdir before skipping the submodule.
+*/
+ if (sha1) {
+ path = git_path("modules/%s",
+ submodule_from_path(null_sha1,
path)->name);
+
+ if(!(is_directory(path) && is_git_directory(path)))
+ return 0;
+ } else {
+ return 0;
+ }
}
#ifndef NO_PTHREADS
diff --git a/t/t7814-grep-recurse-submodules.sh
b/t/t7814-grep-recurse-submodules.sh
index 3d1892d..ee173ad 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -127,6 +127,47 @@ test_expect_success 'grep tree and pathspecs' '
test_cmp expect actual
'
+test_expect_success 'grep history with moved submoules' '
+ git init parent &&
+ echo "foobar" >parent/file &&
+ git -C parent add file &&
+ git -C parent commit -m "add file" &&
+
+ git init sub &&
+ echo "foobar" >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+
+ git -C parent submodule add ../sub &&
+ git -C parent commit -m "add submodule" &&
+
+ cat >expect <<-\EOF &&
+ file:foobar
+ sub/file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules > actual &&
+ test_cmp expect actual &&
+
+ git -C parent mv sub sub-moved &&
+ git -C parent commit -m "moved submodule" &&
+
+ cat >expect <<-\EOF &&
+ file:foobar
+ sub-moved/file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules > actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ HEAD^:file:foobar
+ HEAD^:sub/file:foobar
+ EOF
+ git -C parent grep -e "foobar" --recurse-submodules HEAD^ > actual &&
+ test_cmp expect actual &&
+
+ rm -rf parent sub
+'
+
test_incompatible_with_recurse_submodules ()
{
test_expect_success "--recurse-submodules and $1 are incompatible" "
--
2.8.0.rc3.226.g39d4020
[PATCH v2 0/6] recursively grep across submodules
A few minor style issues have been taken care of from v1 of this series. I also added an additional patch to enable grep to function on history where the submodule has been moved. I also changed how tree grep performs pathspec checking against submodule entries in order to fix a test that was breaking with v1 of the series. Brandon Williams (6): submodules: add helper functions to determine presence of submodules submodules: load gitmodules file from commit sha1 grep: add submodules as a grep source type grep: optionally recurse into submodules grep: enable recurse-submodules to work on objects grep: search history of moved submodules Documentation/git-grep.txt | 14 ++ builtin/grep.c | 387 ++--- cache.h| 2 + config.c | 8 +- git.c | 2 +- grep.c | 16 +- grep.h | 1 + submodule-config.c | 6 +- submodule-config.h | 3 + submodule.c| 51 + submodule.h| 3 + t/t7814-grep-recurse-submodules.sh | 182 + 12 files changed, 643 insertions(+), 32 deletions(-) create mode 100755 t/t7814-grep-recurse-submodules.sh -- 2.8.0.rc3.226.g39d4020
[ANNOUNCE] Git v2.11.0-rc0
An early preview release Git v2.11.0-rc0 is now available for
testing at the usual places. It is comprised of 617 non-merge
commits since v2.10.0, contributed by 64 people, 14 of which are
new faces.
An updated, slightly slipped from the original, schedule is found at
http://tinyurl.com/gitCal and I am hoping that we can conclude this
cycle before US Thanksgiving.
The tarballs are found at:
https://www.kernel.org/pub/software/scm/git/testing/
The following public repositories all have a copy of the
'v2.11.0-rc0' tag and the 'master' branch that the tag points at:
url = https://kernel.googlesource.com/pub/scm/git/git
url = git://repo.or.cz/alt-git.git
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
New contributors whose contributions weren't in v2.10.0 are as follows.
Welcome to the Git development community!
Aaron M Watson, Brandon Williams, Brian Henderson, Emily Xie,
Gavin Lambert, Ian Kelling, Jeff Hostetler, Mantas Mikulėnas,
Petr Stodulka, Satoshi Yasushima, Stefan Christ, Vegard Nossum,
yaras, and Younes Khoudli.
Returning contributors who helped this release are as follows.
Thanks for your continued support.
Ævar Arnfjörð Bjarmason, Alexander Shopov, Alex Henrie,
Alex Riesen, Anders Kaseorg, Beat Bolli, brian m. carlson,
Chris Packham, Christian Couder, David Aguilar, David Turner,
Dennis Kaarsemaker, Dimitriy Ryazantcev, Elia Pinto, Eric Wong,
Jacob Keller, Jakub Narębski, Jean-Noël AVILA, Jeff King,
Jiang Xin, Johannes Schindelin, Johannes Sixt, Jonathan Nieder,
Jonathan Tan, Josh Triplett, Junio C Hamano, Karsten Blees,
Kevin Daudt, Kirill Smelkov, Lars Schneider, Linus Torvalds,
Matthieu Moy, Michael Haggerty, Michael J Gruber, Mike Ralphson,
Nguyễn Thái Ngọc Duy, Olaf Hering, Orgad Shaneh, Pat
Thoyts, Philip Oakley, Pranit Bauva, Ralf Thielow, Ray Chen,
René Scharfe, Ronnie Sahlberg, Stefan Beller, SZEDER Gábor,
Thomas Gummerer, Vasco Almeida, and Дилян Палаузов.
Git 2.11 Release Notes (draft)
==
Backward compatibility notes.
* An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"' by
mistake (when the user meant to give "$path"), which ends up
removing everything. This release starts warning about the
use of an empty string that is used for 'everything matches' and
asks users to use a more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
* The historical argument order "git merge HEAD ..."
has been deprecated for quite some time, and will be removed in the
next release (not this one).
* The default abbreviation length, which has historically been 7, now
scales as the repository grows, using the approximate number of
objects in the reopsitory and a bit of math around the birthday
paradox. The logic suggests to use 12 hexdigits for the Linux
kernel, and 9 to 10 for Git itself.
Updates since v2.10
---
UI, Workflows & Features
* Comes with new version of git-gui, now at its 0.21.0 tag.
* "git format-patch --cover-letter HEAD^" to format a single patch
with a separate cover letter now numbers the output as [PATCH 0/1]
and [PATCH 1/1] by default.
* An incoming "git push" that attempts to push too many bytes can now
be rejected by setting a new configuration variable at the receiving
end.
* "git nosuchcommand --help" said "No manual entry for gitnosuchcommand",
which was not intuitive, given that "git nosuchcommand" said "git:
'nosuchcommand' is not a git command".
* "git clone --resurse-submodules --reference $path $URL" is a way to
reduce network transfer cost by borrowing objects in an existing
$path repository when cloning the superproject from $URL; it
learned to also peek into $path for presense of corresponding
repositories of submodules and borrow objects from there when able.
* The "git diff --submodule={short,log}" mechanism has been enhanced
to allow "--submodule=diff" to show the patch between the submodule
commits bound to the superproject.
* Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
allowed to perform the "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
it had the feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
store and externalize for the consumption by the outside world,
lacked an equiv
What's cooking in git.git (Oct 2016, #09; Mon, 31)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
Git v2.10.2, the second maintenance release for 2.10.x track, has
been tagged. On the master front, an early preview v2.11.0-rc0 has
been tagged. An updated, slightly slipped from the original,
schedule is found at http://tinyurl.com/gitCal and I am hoping that
we can conclude this cycle before US Thanksgiving.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to "master"]
* ak/pre-receive-hook-template-modefix (2016-10-28) 1 commit
(merged to 'next' on 2016-10-28 at 86d95836a3)
+ pre-receive.sample: mark it executable
A trivial clean-up to a recently graduated topic.
* ak/sh-setup-dot-source-i18n-fix (2016-10-30) 1 commit
(merged to 'next' on 2016-10-31 at 538a72700e)
+ git-sh-setup: be explicit where to dot-source git-sh-i18n from.
Recent update to git-sh-setup (a library of shell functions that
are used by our in-tree scripted Porcelain commands) included
another shell library git-sh-i18n without specifying where it is,
relying on the $PATH. This has been fixed to be more explicit by
prefixing $(git --exec-path) output in front.
* aw/numbered-stash (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at 8d9325fa3a)
+ stash: allow stashes to be referenced by index only
The user always has to say "stash@{$N}" when naming a single
element in the default location of the stash, i.e. reflogs in
refs/stash. The "git stash" command learned to accept "git stash
apply 4" as a short-hand for "git stash apply stash@{4}".
* jk/common-main (2016-10-27) 1 commit
(merged to 'next' on 2016-10-28 at fcdd4f8a26)
+ git-compat-util: move content inside ifdef/endif guards
A trivial clean-up to a recently graduated topic.
* jk/rebase-config-insn-fmt-docfix (2016-10-30) 1 commit
(merged to 'next' on 2016-10-31 at d48e20ffa2)
+ doc: fix missing "::" in config list
Documentation fix.
* jt/trailer-with-cruft (2016-10-21) 8 commits
(merged to 'next' on 2016-10-27 at b5d1a21811)
+ trailer: support values folded to multiple lines
+ trailer: forbid leading whitespace in trailers
+ trailer: allow non-trailers in trailer block
+ trailer: clarify failure modes in parse_trailer
+ trailer: make args have their own struct
+ trailer: streamline trailer item create and add
+ trailer: use list.h for doubly-linked list
+ trailer: improve const correctness
(this branch is used by jt/use-trailer-api-in-commands.)
Update "interpret-trailers" machinery and teaches it that people in
real world write all sorts of crufts in the "trailer" that was
originally designed to have the neat-o "Mail-Header: like thing"
and nothing else.
* ls/filter-process (2016-10-17) 14 commits
(merged to 'next' on 2016-10-19 at ffd0de042c)
+ contrib/long-running-filter: add long running filter example
+ convert: add filter..process option
+ convert: prepare filter..process option
+ convert: make apply_filter() adhere to standard Git error handling
+ pkt-line: add functions to read/write flush terminated packet streams
+ pkt-line: add packet_write_gently()
+ pkt-line: add packet_flush_gently()
+ pkt-line: add packet_write_fmt_gently()
+ pkt-line: extract set_packet_header()
+ pkt-line: rename packet_write() to packet_write_fmt()
+ run-command: add clean_on_exit_handler
+ run-command: move check_pipe() from write_or_die to run_command
+ convert: modernize tests
+ convert: quote filter names in error messages
The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
* ls/git-open-cloexec (2016-10-25) 3 commits
(merged to 'next' on 2016-10-26 at f7259cbddb)
+ read-cache: make sure file handles are not inherited by child processes
+ sha1_file: open window into packfiles with O_CLOEXEC
+ sha1_file: rename git_open_noatime() to git_open()
(this branch is used by jc/git-open-cloexec.)
Git generally does not explicitly close file descriptors that were
open in the parent process when spawning a child process, but most
of the time the child does not want to access them. As Windows does
not allow removing or renaming a file that has a file descriptor
open, a slow-to-exit child can even break the parent process by
holding onto them. Use O_CLOEXEC flag to open files in various
codepaths.
* nd/test-helpers (2016-10-27) 1 commit
(
Re: [PATCH 2/4] trailer: avoid unnecessary splitting on lines
Christian Couder writes:
> On Sat, Oct 29, 2016 at 2:05 AM, Jonathan Tan
> wrote:
>> 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
>> ---
>> trailer.c | 215
>> +-
>> 1 file changed, 116 insertions(+), 99 deletions(-)
>
> IMHO it is telling that this needs 17 more lines.
Given that among which 13 are additional comments to explain what is
going on, I think this is a surprisingly good outcome, relative to
what the patch achieves (reducing a lot of split/rejoin operations
and need for allocation associated with them that are all made
unnecessary).
I agree with you that it is telling to see that this needs only 17
more lines (or 4, if you only count the true code additions) and
clearly shows that strbuf_split() helper function was a not good
match for the codepath touched by this patch. The helper function
is very tempting in that with a simple single call it will give us
tons of strbufs in an array, each of which are _MUCH_ more flexible
than simple strings if we ever need to manipulate them by trimming,
splicing and inserting etc. This patch shows us that if we do not
need the flexibility, doing strbuf_split() as the first thing on the
input and having to deal with an array of strbuf is an overall loss,
I would think.
Although I admit that I carefully read only up to [2/4] I am fairly
happy with this series. It finally fixes the second of the two
issues I pointed out in the review of the original series (the other
one was fixed in the series that this topic depends on), paying down
technical debt.
>> @@ -954,7 +971,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;
>
> We often use "sb" as the name of strbuf variables, but I think at
> least here (and maybe in other places above) we could use something a
> bit more telling, like "input_buf" perhaps.
That sounds sensible.
>
>> int trailer_end;
>> FILE *outfile = stdout;
>>
Re: [PATCH] push: do not use potentially ambiguous default refspec
Dennis Kaarsemaker writes: > On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote: > >> * It is appreciated if somebody with spare cycles can add a test or >>two for this in t/t5523-push-upstream.sh or somewhere nearby. > > 5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's > something squashable that fails before your patch and succeeds after. Thanks. > >>8 > Subject: [PATCH] push: test pushing ambiguously named branches > > Signed-off-by: Dennis Kaarsemaker > --- > t/t5528-push-default.sh | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh > index 73f4bb6..ac103ce 100755 > --- a/t/t5528-push-default.sh > +++ b/t/t5528-push-default.sh > @@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with > upstream, matching and simple' > test_push_failure upstream > ' > > +test_expect_success 'push ambiguously named branch with upstream, matching > and simple' ' > + git checkout -b ambiguous && > + test_config branch.ambiguous.remote parent1 && > + test_config branch.ambiguous.merge refs/heads/ambiguous && > + git tag ambiguous && > + test_push_success simple ambiguous && > + test_push_success matching ambiguous && > + test_push_success upstream ambiguous > +' > + > test_expect_success 'push from/to new branch with current creates remote > branch' ' > test_config branch.new-branch.remote repo1 && > git checkout new-branch &&
Re: [PATCH 2/4] trailer: avoid unnecessary splitting on lines
Jonathan Tan writes:
> diff --git a/trailer.c b/trailer.c
> index 6d8375b..d4d9e10 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';
> }
This used to be fed a single line and did not have to worry about
stopping at the end of a line, but now it does have to, and does so
correctly. OK.
And the updated function name certainly reflects the fact that this
is (now) about a line-oriented processing better, and this renaming
makes sense.
> @@ -566,13 +566,18 @@ 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.
> + * Return the location of the first separator in the given line, or -1 if
> there
> + * is no separator.
> + *
> + * The interests parameter must contain the acceptable separators and may
> + * contain '\n'. If interests contains '\n', the input line is considered to
> + * end at the first newline encountered. Otherwise, the input line is
> + * considered to end at its null terminator.
> */
The name of that terminating byte is NUL, not NULL.
> -static int find_separator(const char *line, const char *separators)
> +static int find_separator(const char *line, const char *interests)
> {
> - int loc = strcspn(line, separators);
> - if (!line[loc])
> + int loc = strcspn(line, interests);
> + if (!line[loc] || line[loc] == '\n')
> return -1;
> return loc;
> }
I am not sure interests is a better name than separators. If the
original used "interests", I do not think it is worth renaming it to
"separators", and I doubt that the renaming of the parameter in this
patch is an improvement.
> @@ -688,51 +693,71 @@ static void process_command_line_args(struct list_head
> *arg_head,
> -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"));
> }
> +}
The original used to return an array of strbufs but we now just read
into a single strbuf. The caller may need to do more, or perhaps
the early split into array of strbufs was mostly wasteful use of
more flexible data structure. We'll find out when we read what
happens to the data read here.
> +static const char *next_line(const char *str)
> +{
> + const char *nl = strchrnul(str, '\n');
> + return nl + !!*nl;
> +}
"next_line()" gives a pointer to either the beginning of the next
line, or points at the NUL that terminates the whole buffer. OK.
> +/*
> + * 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;
... and if the last character is not a newline, the line is an
incomplete last line. OK.
> - return lines;
> + for (; i >= 0; i--) {
> + if (buf[i] == '\n')
> + return i + 1;
> + }
> + return 0;
> }
OK.
> /*
> - * 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;
> }
The origi
Re: [PATCH] push: do not use potentially ambiguous default refspec
On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote: > * It is appreciated if somebody with spare cycles can add a test or >two for this in t/t5523-push-upstream.sh or somewhere nearby. 5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's something squashable that fails before your patch and succeeds after. >8 Subject: [PATCH] push: test pushing ambiguously named branches Signed-off-by: Dennis Kaarsemaker --- t/t5528-push-default.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index 73f4bb6..ac103ce 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with upstream, matching and simple' test_push_failure upstream ' +test_expect_success 'push ambiguously named branch with upstream, matching and simple' ' + git checkout -b ambiguous && + test_config branch.ambiguous.remote parent1 && + test_config branch.ambiguous.merge refs/heads/ambiguous && + git tag ambiguous && + test_push_success simple ambiguous && + test_push_success matching ambiguous && + test_push_success upstream ambiguous +' + test_expect_success 'push from/to new branch with current creates remote branch' ' test_config branch.new-branch.remote repo1 && git checkout new-branch && -- 2.10.1-449-gab0f84c
Re: [PATCH] rebase: add --forget to cleanup rebase, leave HEAD untouched
Duy Nguyen writes:
> On Wed, Oct 26, 2016 at 11:51 PM, Junio C Hamano wrote:
>> Nguyễn Thái Ngọc Duy writes:
>>
>>> There are occasions when you decide to abort an in-progress rebase and
>>> move on to do something else but you forget to do "git rebase --abort"
>>> first. Or the rebase has been in progress for so long you forgot about
>>> it. By the time you realize that (e.g. by starting another rebase)
>>> it's already too late to retrace your steps. The solution is normally
>>>
>>> rm -r .git/
>>>
>>> and continue with your life. But there could be two different
>>> directories for (and it obviously requires some
>>> knowledge of how rebase works), and the ".git" part could be much
>>> longer if you are not at top-dir, or in a linked worktree. And
>>> "rm -r" is very dangerous to do in .git, a mistake in there could
>>> destroy object database or other important data.
>>>
>>> Provide "git rebase --forget" for this exact use case.
>>
>> Two and a half comments.
>>
>> - The title says "leave HEAD untouched". Are my working tree files
>>and my index also safe from this operation, or is HEAD the only
>>thing that is protected?
>
> Everything is protected. I will rephrase the title a bit. The option
> is basically a safe form of "rm -r .git/rebase-{apply,merge}".
We are not in a hurry, as it is not likely that this will hit 2.11
even if we saw a rerolled version yesterday, but it would be nice to
cook it on 'next' so that it can be on 'master' early after the
upcoming release.
Thanks.
Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
On Mon, Oct 31, 2016 at 10:55:32AM -0700, Junio C Hamano wrote: > > So I guess it's possible that it produces a noticeable effect in some > > cases, but I'm still somewhat doubtful. And actually repacking your > > repository had a greater effect in every case I measured (in addition to > > providing other speedups). > > Let's keep doubting. I prefer one-step-at-a-time approach to > things anyway, and what I plan in the near term are: > > * use the "open() with O_NOATIME|O_CLOEXEC, gradually losing the >bits during fallback" approach in the ls/git-open-cloexec topic, >in order to help ls/filter-process topic be part of the upcoming >release; > > * simplify the logic to the "open(2) with O_CLOEXEC, set O_NOATIME >with fcntl(2)" in jc/git-open-cloexec~1 after 2.11 ships; > > * cook "drop the latter half of setting O_NOATIME" which is at the >tip of jc/git-open-cloexec in 'next', and while Linus is looking >the other way ^W^W^W^W^W^W^W after people had chance to complain >with numbers, merge it to a future release iff it still looked OK >to drop O_NOATIME thing. Great, that sounds like a good way to proceed (and if the final step never happens, no big deal). -Peff
Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
Jeff King writes: > If you set a probe on touch_atime() in the kernel (which is called for > every attempt to smudge the atime, regardless of mount options, but is > skipped when the descriptor was opened with O_NOATIME), you can see the > impact. Here's a command I picked because it reads a lot of objects (run > on my git.git clone): > > $ perf stat -e probe:touch-atime git log -Sfoo >/dev/null > > And the probe:touch_atime counts before (stock git) and after (a patch > to drop O_NOATIME): > > before: 22,235 >after: 22,362 > > So that's only half a percent difference. And it's on a reasonably messy > clone that is partway to triggering an auto-repack: > ... > So I guess it's possible that it produces a noticeable effect in some > cases, but I'm still somewhat doubtful. And actually repacking your > repository had a greater effect in every case I measured (in addition to > providing other speedups). Let's keep doubting. I prefer one-step-at-a-time approach to things anyway, and what I plan in the near term are: * use the "open() with O_NOATIME|O_CLOEXEC, gradually losing the bits during fallback" approach in the ls/git-open-cloexec topic, in order to help ls/filter-process topic be part of the upcoming release; * simplify the logic to the "open(2) with O_CLOEXEC, set O_NOATIME with fcntl(2)" in jc/git-open-cloexec~1 after 2.11 ships; * cook "drop the latter half of setting O_NOATIME" which is at the tip of jc/git-open-cloexec in 'next', and while Linus is looking the other way ^W^W^W^W^W^W^W after people had chance to complain with numbers, merge it to a future release iff it still looked OK to drop O_NOATIME thing.
Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
Linus Torvalds writes:
> On Sat, Oct 29, 2016 at 1:25 AM, Johannes Schindelin
> wrote:
>>
>> Correct. We cannot change an open file handle's state ("FD_CLOEXEC") on
>> Windows, but we can ask open() to open said file handle with the correct
>> flag ("O_CLOEXEC", which is mapped to O_NOINHERIT on Windows.
>
> Ok. So then I have no issues with it, and let's use O_CLOEXEC if it
> exists and fcntl(FD_CLOEXEC) if O_CLOEXEC doesn't exist.
I am not sure if the fallback to FD_CLOEXEC is worth doing, because
it does not exist on Windows, where leaking an open file descriptor
to a child process matters more than other platforms [*1*].
Of course, the world is not all Windows, and there may be other
platforms this fallback may help. But there is a chance that some
other thread may fork between the time we open(2) and the time we
call fcntl(2), leaving FD_CLOEXEC ineffective and unreliable, so
that's another thing that makes me doubt if FD_CLOEXEC fallback is
worth doing.
Having said that, here is a rewrite of [*2*] to use the fallback
would look like. After this topic settles, we may want to also
update the tempfile.c::create_tempfile() implementation to use the
helper function we use here.
[Footnotes]
*1* The call in ce_compare_data() is to see if the contents in the
working tree file match what is in the index. Perhaps the
program is "git checkout another-branch" that knows this path is
absent in the branch being switched to and trying to make sure
we lose no local modification. When the path needs to be passed
through the clean/smudge filter before hashing to see if the
working tree contents hashes to the object in the index, we need
to fork and then after the filter finishes its processing, we
close the file descriptor and then unlink(2) the path if it is
clean. We are about to add a variant of clean/smudge mechanism
where a lazily spawned process can clean contents of multiple
paths and that is where cloexec starts to matter. This function
starts to inspect one path, opens a fd to it, finds that it
needs filtering, spawns a long-lingering process, while leaking
the original fd to it. After feeding the contents via pipe and
then receiving the filtered contents (the protocol is not full
duplex and there won't be a stuck pipe), the parent would decide
that the path is clean and attempts to unlink(2). Windows does
not let you unlink a path with open filedescriptor held, causing
"checkout" to fail. As it is only one fd that leaks to the
child, no matter how many subsequent paths are filtered by the
same long-lingering child process, it will be a far less
important issue on other platforms that lets you unlink(2) such
a file.
*2* http://public-inbox.org/git/xmqqh97w38gj@gitster.mtv.corp.google.com
---
cache.h | 1 +
git-compat-util.h | 4
read-cache.c | 9 +
sha1_file.c | 47 ---
4 files changed, 34 insertions(+), 27 deletions(-)
diff --git a/cache.h b/cache.h
index a902ca1f8e..6f1c21a352 100644
--- a/cache.h
+++ b/cache.h
@@ -1122,6 +1122,7 @@ extern int write_sha1_file(const void *buf, unsigned long
len, const char *type,
extern int hash_sha1_file_literally(const void *buf, unsigned long len, const
char *type, unsigned char *sha1, unsigned flags);
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned
char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int git_open_cloexec(const char *name, int flags);
extern int git_open(const char *name);
extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
extern int unpack_sha1_header(git_zstream *stream, unsigned char *map,
unsigned long mapsize, void *buffer, unsigned long bufsiz);
diff --git a/git-compat-util.h b/git-compat-util.h
index 43718dabae..64407c701c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -683,6 +683,10 @@ char *gitstrdup(const char *s);
#define O_CLOEXEC 0
#endif
+#ifndef FD_CLOEXEC
+#define FD_CLOEXEC 0
+#endif
+
#ifdef FREAD_READS_DIRECTORIES
#ifdef fopen
#undef fopen
diff --git a/read-cache.c b/read-cache.c
index db5d910642..c27d3e240b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,14 +156,7 @@ void fill_stat_cache_info(struct cache_entry *ce, struct
stat *st)
static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
{
int match = -1;
- static int cloexec = O_CLOEXEC;
- int fd = open(ce->name, O_RDONLY | cloexec);
-
- if ((cloexec & O_CLOEXEC) && fd < 0 && errno == EINVAL) {
- /* Try again w/o O_CLOEXEC: the kernel might not support it */
- cloexec &= ~O_CLOEXEC;
- fd = open(ce->name, O_RDONLY | cloexec);
- }
+ int fd = git_open_cloexec(ce->name, O_RDONLY);
if (fd >= 0) {
unsigned char sha1[20];
diff --git a/sha1_file.c
Re: Reporting Bug in Git Version Control System
W dniu 25.10.2016 o 10:51, Pranit Bauva pisze: > Hey Yash, > > Junio has explained the problem very well. Since you seem to be a > beginner (guessing purely by your email) I will tell you how to fix > it. > > Remember when you would have first installed git, you would have done > something like `git config --global user.name ` and > `git config --global user.email `, it gets > permanently stored in the git configuration file (~/.gitconfig). Now > all the commits in git are made with this name and email. If you want > to change this, again run the above commands with your new name and > email. After that commits will be done with a different name and > email. Hope this helps! :) First, per user Git configuration doesn't necessarily go into ~/.gitconfig; it could be in ~/.config/git/config Second, you can configure user.email and user.name with `git config` or `git config --local`. The information would go into .git/config for specific repository. This might be a better solution if you want different settings for different repositories. -- Jakub Narębski
Re: [git rebase -i] show time and author besides commit hash and message?
On Mon, Oct 31, 2016 at 11:27:33PM +0800, ryenus wrote: > > It is possible to change the format globally via config option > > rebase.instructionFormat: > > > > $ git config rebase.instructionFormat "%an (%ad): %s" > > > > The format is the same as for 'git log'. This one outputs author > > name, date, and the first line of commit message. > > Thanks for the prompt response! > I tried immediately, it works just as you have pointed out. > > Just it seems coloring is not supported? probably because > we're inside an editor? Yes. If git outputs its own ANSI codes there, they generally look bad in an editor (interpreted as raw bytes, not as coloring). Any existing coloring you see is likely due to syntax highlighting done by your editor. You can extend that to match your desired format, but exactly how would depend on which editor you're using. -Peff
Re: [git rebase -i] show time and author besides commit hash and message?
> It is possible to change the format globally via config option > rebase.instructionFormat: > > $ git config rebase.instructionFormat "%an (%ad): %s" > > The format is the same as for 'git log'. This one outputs author > name, date, and the first line of commit message. Thanks for the prompt response! I tried immediately, it works just as you have pointed out. Just it seems coloring is not supported? probably because we're inside an editor? > This option is supported since Git 2.6. I missed it, what about including a note about this option as part of the comment in git rebase -i? since that's the place where people would most likely think about it? > Or are you interested in a command-line option that can change > the format on per-invocation basis? I think there isn't one. > It can be interesting to add it, but I don't think it has much > utility... Not much, for git log I'd rather setup an alias "git la", than having to remember various fields, colors, padding options to have a nice output.
Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
On Fri, Oct 28, 2016 at 09:13:41AM -0700, Linus Torvalds wrote:
> On Fri, Oct 28, 2016 at 4:11 AM, Johannes Schindelin
> wrote:
> >
> > You guys. I mean: You guys! You sure make my life hard. A brief look at
> > mingw.h could have answered your implicit question:
>
> So here's what you guys should do:
>
> - leave O_NOATIME damn well alone. It works. It has worked for 10+
> years. Stop arguing against it, people who do.
For some definition of worked, perhaps.
If you set a probe on touch_atime() in the kernel (which is called for
every attempt to smudge the atime, regardless of mount options, but is
skipped when the descriptor was opened with O_NOATIME), you can see the
impact. Here's a command I picked because it reads a lot of objects (run
on my git.git clone):
$ perf stat -e probe:touch-atime git log -Sfoo >/dev/null
And the probe:touch_atime counts before (stock git) and after (a patch
to drop O_NOATIME):
before: 22,235
after: 22,362
So that's only half a percent difference. And it's on a reasonably messy
clone that is partway to triggering an auto-repack:
$ git count-objects -v
count: 6167
size: 61128
in-pack: 275773
packs: 18
size-pack: 86857
prune-packable: 25
garbage: 0
size-garbage: 0
Running "git gc" drops the probe count to 21,733.
It makes a bigger difference for some commands (it's more like 10% for
git-status). And smaller for others ("git log -p" triggers it over
100,000 times).
One thing missing in that count is how many of those calls would have
resulted in an actual disk write. Looking at strace, most of the
filesystem activity is opening .gitattributes files, and we end up
opening the same ones repeatedly (e.g., t/.gitattributes in git.git).
Multiple hits for a given inode in the same second get coalesced into at
most a single disk write.
So I guess it's possible that it produces a noticeable effect in some
cases, but I'm still somewhat doubtful. And actually repacking your
repository had a greater effect in every case I measured (in addition to
providing other speedups).
Like I said, I'm OK keeping O_NOATIME. It's just not that much code. But
if you really care about the issue of dirtying inodes via atime, you
should look into vastly increasing our use of O_NOATIME. Or possibly
looking at caching more in the attribute code.
-Peff
[no subject]
I am Mrs. Gu Kailai and i intend to make a DONATION. Contact my personal E-mail Via: mrsgukai...@post.cz for more details:
Re: Is the entire working copy “at one branch”?
Thanks Alexei! On Sat, Oct 29, 2016 at 12:47 PM, Alexei Lozovsky wrote: > Hi Stefan, > > Generally with git, your entire working copy will have the same > revision (set to current branch, aka HEAD). The idea behind this > is that your working copy of a repository should always be in > consistent state. > > You can check out specific files or directories from another > revision (mimicking "svn update -r1234 filename"): > > $ git checkout branch-or-sha-hash -- filename > > However, SVN tracks the 'revision' thing on per-file basis, while > in git this is a property of the working copy. So if you do like > above then git will be telling you that the 'filename' has been > changed (as it is certainly different from its pristine version > in HEAD): > > $ git status > On branch master > Changes to be committed: > (use "git reset HEAD ..." to unstage) > > modified: filename > > So it's generally not recommended to do such a thing. > > Another thing that you _can do_ in git to mimick SVN is the > 'standard layout'. There is a feature called "git worktree" which > allows you to have SVN-like directory structure with multiple > directories linked to different working copies: > > $ mkdir my-project > $ cd my-project > $ git clone my-project-repository master > $ mkdir branches > $ cd master > $ git worktree add -b branch-1 ../branches/branch-1 > $ git worktree add -b branch-2 ../branches/branch-2 > > After that you will have directory structure like this: > > $ tree my-project > my-project > ├── branches > │ ├── branch-1 > │ │ ├── 1 > │ │ ├── 2 > │ │ └── 3 > │ └── branch-2 > │ ├── 1 > │ ├── 2 > │ └── banana > └── master > ├── 1 > └── 2 > You can work with these working copies separately, like you > would be working with SVN. Commits in 'master' will go to the > 'master' branch, commits made in 'branches/branch-1' will go > to the 'branch-1' branch.
