Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 11:17 PM, Jeff King wrote: > On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote: > >> On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: >> > This was changed in 10a6cc8 (fetch --prune: Run prune before >> > fetching, 2014-01-02), but it seems that nobody in that >> > discussion realized we were advertising the "after" >> > explicitly. >> > >> > Signed-off-by: Jeff King >> > --- >> > I include myself in that "nobody" of course. :) >> > >> > Documentation/fetch-options.txt | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Documentation/fetch-options.txt >> > b/Documentation/fetch-options.txt >> > index 036edfb..b05a834 100644 >> > --- a/Documentation/fetch-options.txt >> > +++ b/Documentation/fetch-options.txt >> > @@ -52,7 +52,7 @@ ifndef::git-pull[] >> > >> > -p:: >> > --prune:: >> > - After fetching, remove any remote-tracking references that no >> > + Before fetching, remove any remote-tracking references that no >> > longer exist on the remote. Tags are not subject to pruning >> > if they are fetched only because of the default tag >> > auto-following or due to a --tags option. However, if tags >> >> What's the difference in behavior due to pruning before instead of >> after? Curious. It seems like pruning after would make more sense? > > See 10a6cc8. :) > > Basically, you have to prune first to make way for new incoming refs > when there is a D/F conflict. > > The downside is that there is a moment where objects may be unreferenced > (e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_ > create "bar"). And due to the way refs are stored, we do not keep even a > reflog for the deleted ref in the interim. > > -Peff Ah that makes sense, thanks. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 11:14:36PM -0700, Jacob Keller wrote: > On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: > > This was changed in 10a6cc8 (fetch --prune: Run prune before > > fetching, 2014-01-02), but it seems that nobody in that > > discussion realized we were advertising the "after" > > explicitly. > > > > Signed-off-by: Jeff King > > --- > > I include myself in that "nobody" of course. :) > > > > Documentation/fetch-options.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/fetch-options.txt > > b/Documentation/fetch-options.txt > > index 036edfb..b05a834 100644 > > --- a/Documentation/fetch-options.txt > > +++ b/Documentation/fetch-options.txt > > @@ -52,7 +52,7 @@ ifndef::git-pull[] > > > > -p:: > > --prune:: > > - After fetching, remove any remote-tracking references that no > > + Before fetching, remove any remote-tracking references that no > > longer exist on the remote. Tags are not subject to pruning > > if they are fetched only because of the default tag > > auto-following or due to a --tags option. However, if tags > > What's the difference in behavior due to pruning before instead of > after? Curious. It seems like pruning after would make more sense? See 10a6cc8. :) Basically, you have to prune first to make way for new incoming refs when there is a D/F conflict. The downside is that there is a moment where objects may be unreferenced (e.g., if upstream moved "foo" to "bar", we delete "foo" and _then_ create "bar"). And due to the way refs are stored, we do not keep even a reflog for the deleted ref in the interim. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] strbuf: describe the return value of strbuf_read_file
Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- strbuf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/strbuf.h b/strbuf.h index 7987405..83c5c98 100644 --- a/strbuf.h +++ b/strbuf.h @@ -377,6 +377,8 @@ extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint); /** * Read the contents of a file, specified by its path. The third argument * can be used to give a hint about the file size, to avoid reallocs. + * Return the number of bytes read or a negative value if some error + * occurred while opening or reading the file. */ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); -- 2.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fetch: document that pruning happens before fetching
On Mon, Jun 13, 2016 at 4:58 PM, Jeff King wrote: > This was changed in 10a6cc8 (fetch --prune: Run prune before > fetching, 2014-01-02), but it seems that nobody in that > discussion realized we were advertising the "after" > explicitly. > > Signed-off-by: Jeff King > --- > I include myself in that "nobody" of course. :) > > Documentation/fetch-options.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 036edfb..b05a834 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -52,7 +52,7 @@ ifndef::git-pull[] > > -p:: > --prune:: > - After fetching, remove any remote-tracking references that no > + Before fetching, remove any remote-tracking references that no > longer exist on the remote. Tags are not subject to pruning > if they are fetched only because of the default tag > auto-following or due to a --tags option. However, if tags What's the difference in behavior due to pruning before instead of after? Curious. It seems like pruning after would make more sense? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] strbuf: describe the return value of strbuf_read_file()
Hey Junio, On Tue, Jun 14, 2016 at 3:40 AM, Junio C Hamano wrote: >> It is easy to be misguided on the return value of the function >> strbuf_read_file(). It does follow the pattern of other standard functions >> for reading files but its better to explicitly specify it. > > Good thing to do; I wonder if we want to explicitly say -1 or > leave it at "negative values are errors", though (my knee-jerk > reaction being "do not over-specify more than absolute minimum > to write callers correctly"). Sure I will re-roll with specifying about negative values as errors. Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] blame,shortlog: don't make local option variables static
On Tue, Jun 14, 2016 at 12:32:15AM -0400, Eric Sunshine wrote:
> > + struct string_list range_list = STRING_LIST_INIT_NODUP;
>
> Related to this series, there's an additional "fix" which ought to be
> made, probably as a separate patch. In particular, in cmd_blame():
>
> if (lno && !range_list.nr)
> string_list_append(&range_list, xstrdup("1"));
>
> which supplies a default range ("line 1 through end of file") if -L
> was not specified. I used xstrdup() on the literal "1" in 58dbfa2
> (blame: accept multiple -L ranges, 2013-08-06) to be consistent with
> parse_opt_string_list() which was unconditionally xstrdup'ing the
> argument (but no longer does as of patch 1/3 of this series).
Yeah, I'd agree that this is a minor bug both before and after the
series due to the leak. Want to roll a patch on top?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/38] resolve_gitlink_ref(): implement using resolve_ref_recursively()
On Fri, Jun 3, 2016 at 5:03 PM, Michael Haggerty wrote:
> resolve_ref_recursively() can handle references in arbitrary files
> reference stores, so use it to resolve "gitlink" (i.e., submodule)
> references. Aside from removing redundant code, this allows submodule
> lookups to benefit from the much more robust code that we use for
> reading non-submodule references. And, since the code is now agnostic
> about reference backends, it will work for any future references
> backend (so move its definition to refs.c).
>
> Signed-off-by: Michael Haggerty
> ---
> diff --git a/refs.c b/refs.c
> @@ -1299,6 +1299,30 @@ const char *resolve_ref_unsafe(const char *refname,
> int resolve_flags,
> +int resolve_gitlink_ref(const char *path, const char *refname, unsigned char
> *sha1)
> +{
> + int len = strlen(path);
> + struct strbuf submodule = STRBUF_INIT;
> + struct ref_store *refs;
> + int flags;
> +
> + while (len && path[len-1] == '/')
> + len--;
> + if (!len)
> + return -1;
> +
> + strbuf_add(&submodule, path, len);
It took me a moment to figure out that you're using the strbuf only
for its side-effect of giving you a NUL-terminated string needed by
get_ref_store(), and not because you need any fancy functionality of
strbuf. I wonder if xstrndup() would have made this clearer.
Not worth a re-roll.
> + refs = get_ref_store(submodule.buf);
> + strbuf_release(&submodule);
> + if (!refs)
> + return -1;
> +
> + if (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||
> + is_null_sha1(sha1))
> + return -1;
> + return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] blame,shortlog: don't make local option variables static
On Mon, Jun 13, 2016 at 1:39 AM, Jeff King wrote:
> There's no need for these option variables to be static,
> except that they are referenced by the options array itself,
> which is static. But having all of this static is simply
> unnecessary and confusing (and inconsistent with most other
> commands, which either use a static global option list or a
> true function-local one).
>
> Note that in some cases we may need to actually initialize
> the variables (since we cannot rely on BSS to do so). This
> is a net improvement to readability, though, as we can use
> the more verbose initializers for our string_lists.
>
> Signed-off-by: Jeff King
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> @@ -2522,12 +2522,12 @@ int cmd_blame(int argc, const char **argv, const char
> *prefix)
> - static struct string_list range_list;
> - static int output_option = 0, opt = 0;
> - static int show_stats = 0;
> - static const char *revs_file = NULL;
> - static const char *contents_from = NULL;
> - static const struct option options[] = {
> + struct string_list range_list = STRING_LIST_INIT_NODUP;
Related to this series, there's an additional "fix" which ought to be
made, probably as a separate patch. In particular, in cmd_blame():
if (lno && !range_list.nr)
string_list_append(&range_list, xstrdup("1"));
which supplies a default range ("line 1 through end of file") if -L
was not specified. I used xstrdup() on the literal "1" in 58dbfa2
(blame: accept multiple -L ranges, 2013-08-06) to be consistent with
parse_opt_string_list() which was unconditionally xstrdup'ing the
argument (but no longer does as of patch 1/3 of this series).
> + int output_option = 0, opt = 0;
> + int show_stats = 0;
> + const char *revs_file = NULL;
> + const char *contents_from = NULL;
> + const struct option options[] = {
> OPT_BOOL(0, "incremental", &incremental, N_("Show blame
> entries as we find them, incrementally")),
> OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for
> boundary commits (Default: off)")),
> OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits
> as boundaries (Default: off)")),
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Don't free remote->name after fetch
Right. The string_list ends up getting (potentially) populated with a
mix of dup'd
and borrowed values. I figured it was safer to leak here (especially
as we're on
the way out anyway), than free memory that shouldn't be freed.
Actually, what motivates this (and I apologize that I didn't say this
earlier) is that
we added (in our repo) a bit of stats collection code that executes after the
string_list_clear(), and calls remote_get() which goes all sideways when some of
its memory has been freed.
As an alternative, I could xstrdup each instance where remote->name is appended,
which would make the string_list a homogenous dup'd list, which we
could then free.
If you prefer that I'll do a re-roll in that style (it just seemed to
me at the time like
it would be doing some useless allocations). I don't much mind either way.
--
- Keith
On Mon, Jun 13, 2016 at 6:25 PM, Junio C Hamano wrote:
>
> kmcgui...@twopensource.com writes:
>
> > From: Keith McGuigan
> >
> > The string_list gets populated with the names from the remotes[] array,
> > which are not dup'd and the list does not own.
> >
> > Signed-of-by: Keith McGuigan
> > ---
>
> For names that come from remote_get()->name, e.g.
>
> static int get_one_remote_for_fetch(struct remote *remote, void *priv)
> {
> struct string_list *list = priv;
> if (!remote->skip_default_update)
> string_list_append(list, remote->name);
> return 0;
> }
>
> you are correct that this is borrowed memory we are not allowed to
> free. But is borrowing from remote->name the only way this list is
> populated? For example, what happens in add_remote_or_group(),
> which does this?
>
> struct remote_group_data {
> const char *name;
> struct string_list *list;
> };
>
> static int get_remote_group(const char *key, const char *value, void
> *priv)
> {
> struct remote_group_data *g = priv;
>
> if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
> /* split list by white space */
> while (*value) {
> size_t wordlen = strcspn(value, " \t\n");
>
> if (wordlen >= 1)
> string_list_append(g->list,
>xstrndup(value,
> wordlen));
>
> This newly allocated piece of memory is held by g->list, which was...
>
> value += wordlen + (value[wordlen] != '\0');
> }
> }
>
> return 0;
> }
>
> static int add_remote_or_group(const char *name, struct string_list *list)
> {
> int prev_nr = list->nr;
> struct remote_group_data g;
> g.name = name; g.list = list;
>
> ... passed as a callback parameter from here.
>
> git_config(get_remote_group, &g);
> if (list->nr == prev_nr) {
> struct remote *remote = remote_get(name);
> if (!remote_is_configured(remote))
> return 0;
> string_list_append(list, remote->name);
>
> This makes remote->name borrowed, which we cannot free() as you
> point out.
>
> }
> return 1;
> }
>
> So, while I agree that many should not be freed, this change makes
> the code leak some at the same time.
>
>
>
> > builtin/fetch.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index 630ae6a1bb78..181da5a2e7a3 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char
> > *prefix)
> > argv_array_clear(&options);
> > }
> >
> > - /* All names were strdup()ed or strndup()ed */
> > - list.strdup_strings = 1;
> > string_list_clear(&list, 0);
> >
> > close_all_packs();
On Mon, Jun 13, 2016 at 8:11 PM, Keith McGuigan
wrote:
> Right. The string_list ends up getting (potentially) populated with a mix
> of dup'd
> and borrowed values. I figured it was safer to leak here (especially as
> we're on
> the way out anyway), than free memory that shouldn't be freed.
>
> Actually, what motivates this, and I apologize that I didn't say this
> earlier, is that
> we added (in our repo) a bit of stats collection code that executes after
> the
> string_list_clear(), and calls remote_get() which goes all sideways when
> some of
> its memory has been freed.
>
> As an alternative, I could xstrdup each instance where remote->name is
> appended,
> which would make the string_list a homogenous dup'd list, which we could
> then free.
> If you prefer that I'll do a re-roll in that style (it just seemed to me at
> the time like
> it would be doing some useless allocations). I don't much mind either way.
>
> --
> - Keith
>
> On Mon, Jun 13, 2016 at 6:25 PM, Junio C Ham
[PATCH] fetch: document that pruning happens before fetching
This was changed in 10a6cc8 (fetch --prune: Run prune before fetching, 2014-01-02), but it seems that nobody in that discussion realized we were advertising the "after" explicitly. Signed-off-by: Jeff King --- I include myself in that "nobody" of course. :) Documentation/fetch-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 036edfb..b05a834 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -52,7 +52,7 @@ ifndef::git-pull[] -p:: --prune:: - After fetching, remove any remote-tracking references that no + Before fetching, remove any remote-tracking references that no longer exist on the remote. Tags are not subject to pruning if they are fetched only because of the default tag auto-following or due to a --tags option. However, if tags -- 2.9.0.150.g8bd4cf6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
Christian Couder writes:
> +/*
> + * Try to apply a patch.
> + *
> + * Returns:
> + * -1 if an error happened
> + * 0 if the patch applied
> + * 1 if the patch did not apply
> + */
> static int apply_patch(struct apply_state *state,
> int fd,
> const char *filename,
> @@ -4413,6 +4421,7 @@ static int apply_patch(struct apply_state *state,
> struct strbuf buf = STRBUF_INIT; /* owns the patch text */
> struct patch *list = NULL, **listp = &list;
> int skipped_patch = 0;
> + int res = 0;
>
> state->patch_input_file = filename;
> read_patch_file(&buf, fd);
> @@ -4445,8 +4454,10 @@ static int apply_patch(struct apply_state *state,
> offset += nr;
> }
>
> - if (!list && !skipped_patch)
> - die(_("unrecognized input"));
> + if (!list && !skipped_patch) {
> + res = error(_("unrecognized input"));
> + goto end;
> + }
Before this patch, the program said "fatal: $message" and exited
with status = 128. All these changes in this step modifies the
external behaviour and make it say "error: $message" and exit with
status = 1 (at least the caller in apply_all_patches() does so).
Will that be an issue for the calling scripts?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h
Christian Couder writes:
> To libify `git apply` functionality we must make 'struct apply_state'
> usable outside "builtin/apply.c".
>
> Let's do that by creating a new "apply.h" and moving
> 'struct apply_state' there.
>
> Signed-off-by: Christian Couder
> ---
> apply.h | 100
>
> builtin/apply.c | 98 +-
> 2 files changed, 101 insertions(+), 97 deletions(-)
> create mode 100644 apply.h
>
> diff --git a/apply.h b/apply.h
> new file mode 100644
> index 000..9a98eae
> --- /dev/null
> +++ b/apply.h
> @@ -0,0 +1,100 @@
> +#ifndef APPLY_H
> +#define APPLY_H
> +
> +enum ws_error_action {
> + nowarn_ws_error,
> + warn_on_ws_error,
> + die_on_ws_error,
> + correct_ws_error
> +};
> +
> +enum ws_ignore {
> + ignore_ws_none,
> + ignore_ws_change
> +};
> +
> +/*
> + * We need to keep track of how symlinks in the preimage are
> + * manipulated by the patches. A patch to add a/b/c where a/b
> + * is a symlink should not be allowed to affect the directory
> + * the symlink points at, but if the same patch removes a/b,
> + * it is perfectly fine, as the patch removes a/b to make room
> + * to create a directory a/b so that a/b/c can be created.
> + *
> + * See also "struct string_list symlink_changes" in "struct
> + * apply_state".
> + */
> +#define SYMLINK_GOES_AWAY 01
> +#define SYMLINK_IN_RESULT 02
Everything below is agreeable, but all the names that are made
public above by this change do not sound specific enough to "apply".
I wonder if they should get "apply" somewhere in their names to
avoid confusion coming from the namespace contamination...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] send-email: create email parser subroutine
Samuel GROOT wrote: > On 06/09/2016 02:21 AM, Eric Wong wrote: > >Samuel GROOT wrote: > >>Email::Simple library uses qr/\x0a\x0d|\x0d\x0a|\x0a|\x0d/ [1]. > >>Should we handle \n\r at end of line as well? > > > >"\n\r" can never happen with local $/ = "\n" > > If the email file contains "\n\r", setting $/ = "\n" will leave "\r" at > the beginning of each line. > > We could trim them with: > > s/^\r//; > s/\r?\n$//; > > But is it worth adding `s/^\r//;` to handle that extremely rare case? I doubt it. Having a "\r" in the wrong place is likely a bug in whatever program that generated the email. It should be exposed so whoever generated that email has a chance to fix it on their end rather than being quietly hidden. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Don't free remote->name after fetch
kmcgui...@twopensource.com writes:
> From: Keith McGuigan
>
> The string_list gets populated with the names from the remotes[] array,
> which are not dup'd and the list does not own.
>
> Signed-of-by: Keith McGuigan
> ---
For names that come from remote_get()->name, e.g.
static int get_one_remote_for_fetch(struct remote *remote, void *priv)
{
struct string_list *list = priv;
if (!remote->skip_default_update)
string_list_append(list, remote->name);
return 0;
}
you are correct that this is borrowed memory we are not allowed to
free. But is borrowing from remote->name the only way this list is
populated? For example, what happens in add_remote_or_group(),
which does this?
struct remote_group_data {
const char *name;
struct string_list *list;
};
static int get_remote_group(const char *key, const char *value, void *priv)
{
struct remote_group_data *g = priv;
if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) {
/* split list by white space */
while (*value) {
size_t wordlen = strcspn(value, " \t\n");
if (wordlen >= 1)
string_list_append(g->list,
xstrndup(value,
wordlen));
This newly allocated piece of memory is held by g->list, which was...
value += wordlen + (value[wordlen] != '\0');
}
}
return 0;
}
static int add_remote_or_group(const char *name, struct string_list *list)
{
int prev_nr = list->nr;
struct remote_group_data g;
g.name = name; g.list = list;
... passed as a callback parameter from here.
git_config(get_remote_group, &g);
if (list->nr == prev_nr) {
struct remote *remote = remote_get(name);
if (!remote_is_configured(remote))
return 0;
string_list_append(list, remote->name);
This makes remote->name borrowed, which we cannot free() as you
point out.
}
return 1;
}
So, while I agree that many should not be freed, this change makes
the code leak some at the same time.
> builtin/fetch.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 630ae6a1bb78..181da5a2e7a3 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char
> *prefix)
> argv_array_clear(&options);
> }
>
> - /* All names were strdup()ed or strndup()ed */
> - list.strdup_strings = 1;
> string_list_clear(&list, 0);
>
> close_all_packs();
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/6] t9001: check email address is in Cc: field
On 06/09/2016 07:55 AM, Matthieu Moy wrote: Tom Russello writes: Check if the given utf-8 email address is in the Cc: field. I wouldn't harm to explain what was the problem with existing code here. If I understand correctly, that would be: Existing code just checked that the address appeared in a line starting with a space, but not to which field the address belonged. But probably the real motivation for this was that you want to introduce code that changes the layout and breaks this "address appears on a line starting with space"? Actually it was both, we wanted to make the tests less dependent to how data was displayed. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison
On 06/09/2016 08:01 AM, Matthieu Moy wrote: Samuel GROOT writes: On 06/08/2016 06:09 PM, Junio C Hamano wrote: Samuel GROOT writes: Actually we had issues when trying to refactor send-email's email parsing loop [1]. Email addresses in output file `commandeline1` in tests weren't sorted the same way as the reference file it was compared to. E.g.: !nob...@example.com! !aut...@example.com! !o...@example.com! !t...@example.com! And the reason why these addresses that are collected from the same input (i.e. command line, existing e-mail fields, footers, etc.) are shown in different order in your implementation is...? It's not shown in different order in our implementation, it's just a leftover of my refactor attempt [1]. I think the refactoring makes sense, but having this patch as PATCH 1/6 in a series about --in-reply-to confuses reviewers: they would expect this patch to be useful to the others in the series. If you have "reply to a message in a file" ready without the refactoring, and a mostly ready refactoring, then I think it makes sense to have two patch series, the first being only "reply to a message in a file". If the refactoring itself is not ready, you may send a separate series "tests clean up" and explain on the cover-letter that it's, well, only a test clean up. I think I will split the patch series into 3 smaller series: - "quote-email" feature - tests clean up - send-email code cleanup (including send-email's output) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] send-email: shorten send-email's output
On 06/09/2016 08:17 AM, Matthieu Moy wrote: Samuel GROOT writes: @@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' ' test_expect_success $PREREQ 'setup expect' " cat >expected-suppress-body <<\EOF 0001-Second.patch -(mbox) Adding cc: A from line 'From: A ' -(mbox) Adding cc: One from line 'Cc: One , t...@example.com' -(mbox) Adding cc: t...@example.com from line 'Cc: One , t...@example.com' -(cc-cmd) Adding cc: cc-...@example.com from: './cccmd' +Adding cc: A from From: header +Adding cc: One from Cc: header +Adding cc: t...@example.com from Cc: header +Adding cc: cc-...@example.com from: './cccmd' This hunk differs from the others a bit. I totally agree that removing the (mbox) prefix makes sense, but you're removing (cc-cmd) here, which did carry some information. I'd write it as Adding cc: cc-...@example.com from --cc-cmd: ./cccmd Indeed it's clearer, I will change that. It might make sense to split this into two patches: one for (mbox) + headers and one for (cc-cmd) and (to-cmd). Spotting special-cases like the above inside a long patch is hard for reviewers. I will split in the future patch series dedicated to clean send-email. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] send-email: create email parser subroutine
On 06/09/2016 02:21 AM, Eric Wong wrote: Samuel GROOT wrote: Email::Simple library uses qr/\x0a\x0d|\x0d\x0a|\x0a|\x0d/ [1]. Should we handle \n\r at end of line as well? "\n\r" can never happen with local $/ = "\n" If the email file contains "\n\r", setting $/ = "\n" will leave "\r" at the beginning of each line. We could trim them with: s/^\r//; s/\r?\n$//; But is it worth adding `s/^\r//;` to handle that extremely rare case? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/6] send-email: create email parser subroutine
On 06/09/2016 08:51 AM, Eric Sunshine wrote: On Wed, Jun 8, 2016 at 7:54 PM, Samuel GROOT wrote: On 06/08/2016 10:17 PM, Junio C Hamano wrote: Eric Sunshine writes: An embedded CR probably shouldn't happen, but I'm not convinced that folding it out is a good idea. I would think that you'd want to preserve the header's value verbatim. If anything, I'd expect to see the regex tightened to: s/\r?\n$//; Yes, that would be more sensible than silently removing \r in the middle which _is_ a sign of something funny going on. s/\r?\n$// looks fine. Email::Simple library uses qr/\x0a\x0d|\x0d\x0a|\x0a|\x0d/ [1]. [1] * http://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm You could certainly use s/\x0d?\x0a$// rather than s/\r?\n$// to be really robust, but I doubt it matters much today. The reason for using hex codes is that \r and \n will resolve to CR and LF in the local character encoding, and those may not be 0x0d and 0x0a, respectively. I believe the canonical example given in the Camel book was EBCIDIC in which \r is 0x0d, but \n is 0x25, not 0x0a as it is in ASCII. My question was more about the `\n\r` case handled by Email::Simple, does it make sense to handle it? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] strbuf: describe the return value of strbuf_read_file()
Pranit Bauva writes: > Mentored-by: Lars Schneider > Mentored-by: Christian Couder > Signed-off-by: Pranit Bauva > --- > It is easy to be misguided on the return value of the function > strbuf_read_file(). It does follow the pattern of other standard functions > for reading files but its better to explicitly specify it. Good thing to do; I wonder if we want to explicitly say -1 or leave it at "negative values are errors", though (my knee-jerk reaction being "do not over-specify more than absolute minimum to write callers correctly"). > > strbuf.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/strbuf.h b/strbuf.h > index 7987405..4b487f7 100644 > --- a/strbuf.h > +++ b/strbuf.h > @@ -377,6 +377,8 @@ extern ssize_t strbuf_read_once(struct strbuf *, int fd, > size_t hint); > /** > * Read the contents of a file, specified by its path. The third argument > * can be used to give a hint about the file size, to avoid reallocs. > + * Return the number of bytes read or -1 if some error occurred while > + * opening or reading the file. > */ > extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t > hint); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document the 'svn propset' command.
On 6/13/16 7:42 AM, Pranit Bauva wrote: Hey Alfred, On Mon, Jun 13, 2016 at 7:54 PM, Pranit Bauva wrote: Hey Alfred, On Mon, Jun 13, 2016 at 6:22 PM, Alfred Perlstein wrote: Thank you Pranit. I thought that "signed off by" is used once someone approved my patch as opposed to when it's in "proposal" stage. This was my first email with a patch for this issue, who should/could I have used for "signoff"? Signoff is used to indicate that you are OKAY with releasing your patch according to git's license. For more details see the Documentation/SubmittingPatches[1]. To summarize you will have to add this in the end : Signed-off-by: Alfred Perlstein Though I will still recommend you to go through [1] properly. Oops I forgot to put the link. [1]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L239-L307 Pranit, Ah thank you!!! This clarifies. I will resend the patch tonight. -Alfred -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re attr API further revamp
Stefan Beller writes: > On Mon, Jun 13, 2016 at 1:55 PM, Junio C Hamano wrote: >> I hate to be doing this, but we need yet another revamp to the attr >> API that affects all the callers. > > So you don't mean origin/jc/attr-more by this? Not really; the tip of attr-more needs to be discarded after this realization, but otherwise there is no API revamping in what is queued on that topic. > (Given that we have jc/attr and jc/attr-more, the third thing could go > with jc/even-more-attr. Though I do not propose that) The latter was merely because I couldn't rewind and rebuild the former during the pre-release freeze. The fixup! in the latter will be squashed into the former, etc... >> static struct git_attr_check *ccheck; >> const char *values[NUM_ATTRS]; >> >> git_attr_check_initl(&ccheck, "crlf", "ident", ..., NULL); >> git_attr_check(path, ccheck, values); > > and later on each thread will do a This will have to do nothing; it is "static", and will be held to the end of the lifetime of the program. You _can_ allocate the check structure per thread and free them when thread exits, but the usage pattern that appear in convert.c is oblivious to the threading. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re attr API further revamp
On Mon, Jun 13, 2016 at 1:55 PM, Junio C Hamano wrote:
> I hate to be doing this, but we need yet another revamp to the attr
> API that affects all the callers.
So you don't mean origin/jc/attr-more by this?
(Given that we have jc/attr and jc/attr-more, the third thing could go
with jc/even-more-attr. Though I do not propose that)
>
> In the original design, a codepath that wants to check attributes
> repeatedly for many paths (e.g. "convert" that wants to see what
> crlf, ident, filter, eol and text attributes are set to for paths
> that it is asked to munge the blob contents for) used to allocate
>
> static struct git_attr_check {
> struct git_attr *attr;
> const char *value;
> } ccheck[NUM_ATTRS];
>
> and populated the array when the first time the codepath was
> entered, e.g.
>
> if (!ccheck[0].attr) {
> for (i = 0; i < NUM_ATTRS; i++)
> ccheck[i].attr = git_attr(...);
> }
>
> and then make a call to the API to ask for these attributes in
> ccheck[] for a path, i.e.
>
> git_attr_check(path, NUM_ATTRS, ccheck);
>
> The API function will fill in the ccheck[].value fields and the
> caller will read from there how each attribute is set for the path.
>
> This was updated with recent jc/attr topic so that attr_check
> structure can be enriched to keep _more_ state, such as the
> pre-parsed representation of all the contents of the relevant
> .gitattributes files, which currently is held in a program-side
> singleton instance called attr_stack, partly in preparation for
> Stefan's sb/pathspec-label topic that is expected to place a lot
> heavier load to the attribute subsystem.
I do not quite expect that. Some numbers:
(I am looking at https://android.googlesource.com/platform/manifest)
* There are currently different 56 groups in that manifest
* with an average of 1.5 groups for each entry that has at least one group,
* or 0.88 groups per entry (which is one resulting file/submodule after the
conversion)
For reference:
* git.git makes use of 1 attr (whitespace), in 2 value configurations
* that is applied to each file.
So comparing one entry to the 1.5 or 0.88 above doesn't seem to
be that bad. That is what needs to be done on a per-file basis?
What is different is the number of different attrs in use by about
2 orders of magnitude, which needs to be collected upfront and stored
in a smart way?
>
> A caller of the API after that update would do more like
>
> static struct git_attr_check *ccheck;
>
> if (!ccheck)
> ccheck = git_attr_check_initl(...);
> git_attr_check(path, ccheck);
> for (i = 0; i < NUM_ATTRS; i++)
> if (ccheck->check[i].value == ATTR_UNSET)
> ... do the Unset thing ...
This is origin/jc/attr-more ?
>
> This however is not thread-safe for obvious two reasons.
>
> * Two threads can simultanously enter this section of the code and
>end up initializing ccheck twice;
>
> * Even though ccheck[].attr are constant for the purpose of this
>codepath (i.e. all threads passing through are interested in
>checking the same set of attributes), ccheck[].value fields
>cannot be shared across simultaneous threads like the above
>snippet does.
>
> So it appears that the final API visible from the callers needs to
> be updated further, perhaps something like:
>
>
> static struct git_attr_check *ccheck;
> const char *values[NUM_ATTRS];
>
> git_attr_check_initl(&ccheck, "crlf", "ident", ..., NULL);
> git_attr_check(path, ccheck, values);
and later on each thread will do a
git_attr_thread_byebye(&ccheck) ?
to free its thread local stuff or rather is the calling site responsible for
freeing up all of it after all threads are done?
(This is probably too much of a detail question now?)
>
> where git_attr_check_initl() would do the "if ccheck is NULL then
> initialize it to an instance" atomically in threading environment,
> and git_attr_check() returns its finding to values[] array the
> calling thread exclusively owns, while sharing the input ccheck
> and the list of attributes the call inquires among threads.
>
> Also unlike the earlier plan, attr_stack aka "where in the directory
> hierarchy are we asking attributes for?" will not be stored directly
> in git_attr_check structure. It is likely that a thread-local
> structure will be introduced hidden behind this API (i.e. the
> callers do not have to be aware of it), and attr_stack will be
> registered in this thread-local structure keyed by &ccheck, so that
> pair can have attr_stack instance of its own.
>
> This is because a single attr_stack per ccheck will not work as an
> optimization mechanism when multi-threaded. Two threads may be
> running the same convert() function, and they may be interested in
> the same set of attributes (e.g. "crlf", "ident", etc.) but they
> will be workin
Re: [PATCH] Refactor recv_sideband()
On Mon, 13 Jun 2016, Lukas Fleischer wrote:
> Improve the readability of recv_sideband() significantly by replacing
> fragile buffer manipulations with more sophisticated format strings.
> Also, reorganize the overall control flow, remove some superfluous
> variables and replace a custom implementation of strpbrk() with a call
> to the standard C library function.
>
> Signed-off-by: Lukas Fleischer
The previous code was a total abomination, even if I happen to know who
wrote it.
Acked-by: Nicolas Pitre
> I had a really hard time reading and understanding this function when I
> came up with my last patch. What I ended up with is almost a complete
> rewrite of recv_sideband() and I find the end result to be much more
> readable than what we have now. Given that this is quite invasive, it
> would be good to have some more eyes and opinions...
>
> If you want me to split this patch into smaller changes, please let me
> know. However, finding a good way to split this into logical changes
> might not be easy given that the new code does not have much in common
> with what we had before.
Indeed. Anyway, numbers speak for themselves:
> 1 file changed, 30 insertions(+), 64 deletions(-)
>
> diff --git a/sideband.c b/sideband.c
> index fde8adc..0a078c3 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -13,103 +13,69 @@
> * the remote died unexpectedly. A flush() concludes the stream.
> */
>
> -#define PREFIX "remote:"
> +#define PREFIX "remote: "
>
> #define ANSI_SUFFIX "\033[K"
> #define DUMB_SUFFIX ""
>
> -#define FIX_SIZE 10 /* large enough for any of the above */
> -
> int recv_sideband(const char *me, int in_stream, int out)
> {
> - unsigned pf = strlen(PREFIX);
> - unsigned sf;
> - char buf[LARGE_PACKET_MAX + 2*FIX_SIZE];
> - char *suffix, *term;
> - int skip_pf = 0;
> + const char *term;
> + const char *prefix = PREFIX, *suffix;
> + char buf[LARGE_PACKET_MAX + 1];
> + const char *b, *brk;
>
> - memcpy(buf, PREFIX, pf);
> term = getenv("TERM");
> if (isatty(2) && term && strcmp(term, "dumb"))
> suffix = ANSI_SUFFIX;
> else
> suffix = DUMB_SUFFIX;
> - sf = strlen(suffix);
>
> while (1) {
> int band, len;
> - len = packet_read(in_stream, NULL, NULL, buf + pf,
> LARGE_PACKET_MAX, 0);
> + len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
> 0);
> if (len == 0)
> break;
> if (len < 1) {
> fprintf(stderr, "%s: protocol error: no band
> designator\n", me);
> return SIDEBAND_PROTOCOL_ERROR;
> }
> - band = buf[pf] & 0xff;
> + band = buf[0] & 0xff;
> + buf[len] = '\0';
> len--;
> switch (band) {
> case 3:
> - buf[pf] = ' ';
> - buf[pf+1+len] = '\0';
> - fprintf(stderr, "%s\n", buf);
> + fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
> return SIDEBAND_REMOTE_ERROR;
> case 2:
> - buf[pf] = ' ';
> - do {
> - char *b = buf;
> - int brk = 0;
> -
> - /*
> - * If the last buffer didn't end with a line
> - * break then we should not print a prefix
> - * this time around.
> - */
> - if (skip_pf) {
> - b += pf+1;
> - } else {
> - len += pf+1;
> - brk += pf+1;
> - }
> + b = buf + 1;
>
> - /* Look for a line break. */
> - for (;;) {
> - brk++;
> - if (brk > len) {
> - brk = 0;
> - break;
> - }
> - if (b[brk-1] == '\n' ||
> - b[brk-1] == '\r')
> - break;
> - }
> + /*
> + * Append a suffix to each nonempty line to clear the
> + * end of the screen line.
> + */
> + while ((brk = strpbrk(b, "\n\r"))) {
> + int linelen = brk - b;
>
> - /*
> - * Let's insert a suffix to clear the end
> - * of the screen li
Re attr API further revamp
I hate to be doing this, but we need yet another revamp to the attr
API that affects all the callers.
In the original design, a codepath that wants to check attributes
repeatedly for many paths (e.g. "convert" that wants to see what
crlf, ident, filter, eol and text attributes are set to for paths
that it is asked to munge the blob contents for) used to allocate
static struct git_attr_check {
struct git_attr *attr;
const char *value;
} ccheck[NUM_ATTRS];
and populated the array when the first time the codepath was
entered, e.g.
if (!ccheck[0].attr) {
for (i = 0; i < NUM_ATTRS; i++)
ccheck[i].attr = git_attr(...);
}
and then make a call to the API to ask for these attributes in
ccheck[] for a path, i.e.
git_attr_check(path, NUM_ATTRS, ccheck);
The API function will fill in the ccheck[].value fields and the
caller will read from there how each attribute is set for the path.
This was updated with recent jc/attr topic so that attr_check
structure can be enriched to keep _more_ state, such as the
pre-parsed representation of all the contents of the relevant
.gitattributes files, which currently is held in a program-side
singleton instance called attr_stack, partly in preparation for
Stefan's sb/pathspec-label topic that is expected to place a lot
heavier load to the attribute subsystem.
A caller of the API after that update would do more like
static struct git_attr_check *ccheck;
if (!ccheck)
ccheck = git_attr_check_initl(...);
git_attr_check(path, ccheck);
for (i = 0; i < NUM_ATTRS; i++)
if (ccheck->check[i].value == ATTR_UNSET)
... do the Unset thing ...
This however is not thread-safe for obvious two reasons.
* Two threads can simultanously enter this section of the code and
end up initializing ccheck twice;
* Even though ccheck[].attr are constant for the purpose of this
codepath (i.e. all threads passing through are interested in
checking the same set of attributes), ccheck[].value fields
cannot be shared across simultaneous threads like the above
snippet does.
So it appears that the final API visible from the callers needs to
be updated further, perhaps something like:
static struct git_attr_check *ccheck;
const char *values[NUM_ATTRS];
git_attr_check_initl(&ccheck, "crlf", "ident", ..., NULL);
git_attr_check(path, ccheck, values);
where git_attr_check_initl() would do the "if ccheck is NULL then
initialize it to an instance" atomically in threading environment,
and git_attr_check() returns its finding to values[] array the
calling thread exclusively owns, while sharing the input ccheck
and the list of attributes the call inquires among threads.
Also unlike the earlier plan, attr_stack aka "where in the directory
hierarchy are we asking attributes for?" will not be stored directly
in git_attr_check structure. It is likely that a thread-local
structure will be introduced hidden behind this API (i.e. the
callers do not have to be aware of it), and attr_stack will be
registered in this thread-local structure keyed by &ccheck, so that
pair can have attr_stack instance of its own.
This is because a single attr_stack per ccheck will not work as an
optimization mechanism when multi-threaded. Two threads may be
running the same convert() function, and they may be interested in
the same set of attributes (e.g. "crlf", "ident", etc.) but they
will be working on different parts of the tree, so having attr_stack
per would make more sense.
All of which will be quite a lot of work, though, so do not expect
that it will appear by the end of this week ;-)
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] strbuf: describe the return value of strbuf_read_file()
Mentored-by: Lars Schneider Mentored-by: Christian Couder Signed-off-by: Pranit Bauva --- It is easy to be misguided on the return value of the function strbuf_read_file(). It does follow the pattern of other standard functions for reading files but its better to explicitly specify it. strbuf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/strbuf.h b/strbuf.h index 7987405..4b487f7 100644 --- a/strbuf.h +++ b/strbuf.h @@ -377,6 +377,8 @@ extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint); /** * Read the contents of a file, specified by its path. The third argument * can be used to give a hint about the file size, to avoid reallocs. + * Return the number of bytes read or -1 if some error occurred while + * opening or reading the file. */ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); -- 2.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Refactor recv_sideband()
Improve the readability of recv_sideband() significantly by replacing
fragile buffer manipulations with more sophisticated format strings.
Also, reorganize the overall control flow, remove some superfluous
variables and replace a custom implementation of strpbrk() with a call
to the standard C library function.
Signed-off-by: Lukas Fleischer
---
I had a really hard time reading and understanding this function when I
came up with my last patch. What I ended up with is almost a complete
rewrite of recv_sideband() and I find the end result to be much more
readable than what we have now. Given that this is quite invasive, it
would be good to have some more eyes and opinions...
If you want me to split this patch into smaller changes, please let me
know. However, finding a good way to split this into logical changes
might not be easy given that the new code does not have much in common
with what we had before.
sideband.c | 94 --
1 file changed, 30 insertions(+), 64 deletions(-)
diff --git a/sideband.c b/sideband.c
index fde8adc..0a078c3 100644
--- a/sideband.c
+++ b/sideband.c
@@ -13,103 +13,69 @@
* the remote died unexpectedly. A flush() concludes the stream.
*/
-#define PREFIX "remote:"
+#define PREFIX "remote: "
#define ANSI_SUFFIX "\033[K"
#define DUMB_SUFFIX ""
-#define FIX_SIZE 10 /* large enough for any of the above */
-
int recv_sideband(const char *me, int in_stream, int out)
{
- unsigned pf = strlen(PREFIX);
- unsigned sf;
- char buf[LARGE_PACKET_MAX + 2*FIX_SIZE];
- char *suffix, *term;
- int skip_pf = 0;
+ const char *term;
+ const char *prefix = PREFIX, *suffix;
+ char buf[LARGE_PACKET_MAX + 1];
+ const char *b, *brk;
- memcpy(buf, PREFIX, pf);
term = getenv("TERM");
if (isatty(2) && term && strcmp(term, "dumb"))
suffix = ANSI_SUFFIX;
else
suffix = DUMB_SUFFIX;
- sf = strlen(suffix);
while (1) {
int band, len;
- len = packet_read(in_stream, NULL, NULL, buf + pf,
LARGE_PACKET_MAX, 0);
+ len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
0);
if (len == 0)
break;
if (len < 1) {
fprintf(stderr, "%s: protocol error: no band
designator\n", me);
return SIDEBAND_PROTOCOL_ERROR;
}
- band = buf[pf] & 0xff;
+ band = buf[0] & 0xff;
+ buf[len] = '\0';
len--;
switch (band) {
case 3:
- buf[pf] = ' ';
- buf[pf+1+len] = '\0';
- fprintf(stderr, "%s\n", buf);
+ fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
return SIDEBAND_REMOTE_ERROR;
case 2:
- buf[pf] = ' ';
- do {
- char *b = buf;
- int brk = 0;
-
- /*
-* If the last buffer didn't end with a line
-* break then we should not print a prefix
-* this time around.
-*/
- if (skip_pf) {
- b += pf+1;
- } else {
- len += pf+1;
- brk += pf+1;
- }
+ b = buf + 1;
- /* Look for a line break. */
- for (;;) {
- brk++;
- if (brk > len) {
- brk = 0;
- break;
- }
- if (b[brk-1] == '\n' ||
- b[brk-1] == '\r')
- break;
- }
+ /*
+* Append a suffix to each nonempty line to clear the
+* end of the screen line.
+*/
+ while ((brk = strpbrk(b, "\n\r"))) {
+ int linelen = brk - b;
- /*
-* Let's insert a suffix to clear the end
-* of the screen line if a line break was
-* found. Also, if we don't skip the
-* prefix, then a non-empty string must be
-* presen
[ANNOUNCE] Git v2.9.0
The latest feature release Git v2.9.0 is now available at the usual places. It is comprised of 497 non-merge commits since v2.8.0, contributed by 75 people, 28 of which are new faces. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.9.0' 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.8.0 are as follows. Welcome to the Git development community! Alexander Rinass, Antonin, Armin Kunaschik, Benjamin Dopplinger, Ben Woosley, Erwan Mathoniere, Gabriel Souza Franco, Jacob Nisnevich, Jan Durovec, Jean-Noël Avila, Kazuki Yamaguchi, Keller Fuchs, Laurent Arnoud, Li Peng, Marios Titas, Mehul Jain, Michael Procter, Nikola Forró, Pablo Santiago Blum de Aguiar, Pranit Bauva, Ray Zhang, René Nyffenegger, Santiago Torres, Saurav Sachidanand, Shin Kojima, Sidhant Sharma [:tk], Stanislav Kolotinskiy, and Xiaolong Ye. Returning contributors who helped this release are as follows. Thanks for your continued support. Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Alexander Kuleshov, Alexander Shopov, brian m. carlson, Brian Norris, Changwoo Ryu, Christian Couder, David Aguilar, David Turner, Dennis Kaarsemaker, Dimitriy Ryazantcev, Elia Pinto, Elijah Newren, Eric Sunshine, Eric Wong, Felipe Contreras, Jacob Keller, Jean-Noel Avila, Jeff King, Jiang Xin, Johannes Schindelin, Johannes Sixt, John Keeping, Junio C Hamano, Karsten Blees, Lars Schneider, Linus Torvalds, Luke Diamand, Matthieu Moy, Michael Haggerty, Michael J Gruber, Michael Rappazzo, Nguyễn Thái Ngọc Duy, Ori Avtalion, Peter Krefting, Ralf Thielow, Ramsay Jones, Ray Chen, René Scharfe, Stefan Beller, Stephen P. Smith, Sven Strickroth, SZEDER Gábor, Torsten Bögershausen, Trần Ngọc Quân, and Vasco Almeida. Git 2.9 Release Notes = Backward compatibility notes The end-user facing Porcelain level commands in the "git diff" and "git log" family by default enable the rename detection; you can still use "diff.renames" configuration variable to disable this. Merging two branches that have no common ancestor with "git merge" is by default forbidden now to prevent creating such an unusual merge by mistake. The output formats of "git log" that indents the commit log message by 4 spaces now expands HT in the log message by default. You can use the "--no-expand-tabs" option to disable this. "git commit-tree" plumbing command required the user to always sign its result when the user sets the commit.gpgsign configuration variable, which was an ancient mistake, which this release corrects. A script that drives commit-tree, if it relies on this mistake, now needs to read commit.gpgsign and pass the -S option as necessary. Updates since v2.8 -- UI, Workflows & Features * Comes with git-multimail 1.3.1 (in contrib/). * The end-user facing commands like "git diff" and "git log" now enable the rename detection by default. * The credential.helper configuration variable is cumulative and there is no good way to override it from the command line. As a special case, giving an empty string as its value now serves as the signal to clear the values specified in various files. * A new "interactive.diffFilter" configuration can be used to customize the diff shown in "git add -i" sessions. * "git p4" now allows P4 author names to be mapped to Git author names. * "git rebase -x" can be used without passing "-i" option. * "git -c credential.= submodule" can now be used to propagate configuration variables related to credential helper down to the submodules. * "git tag" can create an annotated tag without explicitly given an "-a" (or "-s") option (i.e. when a tag message is given). A new configuration variable, tag.forceSignAnnotated, can be used to tell the command to create signed tag in such a situation. * "git merge" used to allow merging two branches that have no common base by default, which led to a brand new history of an existing project created and then get pulled by an unsuspecting maintainer, which allowed an unnecessary parallel history merged into the existing project. The command has been taught not to allow this by default, with an escape hatch "--allow-unrelated-histories" option to be used in a rare event that merges histories of two projects that started their lives independently. * "git pull" has been taught to pass the "--allow-unrelated-histories" option to underlying "git merge". * "git apply -v" learned to report paths
A note from the maintainer
Welcome to the Git development community.
This message is written by the maintainer and talks about how Git
project is managed, and how you can work with it.
* Mailing list and the community
The development is primarily done on the Git mailing list. Help
requests, feature proposals, bug reports and patches should be sent to
the list address . You don't have to be
subscribed to send messages. The convention on the list is to keep
everybody involved on Cc:, so it is unnecessary to say "Please Cc: me,
I am not subscribed".
Before sending patches, please read Documentation/SubmittingPatches
and Documentation/CodingGuidelines to familiarize yourself with the
project convention.
If you sent a patch and you did not hear any response from anybody for
several days, it could be that your patch was totally uninteresting,
but it also is possible that it was simply lost in the noise. Please
do not hesitate to send a reminder message in such a case. Messages
getting lost in the noise may be a sign that those who can evaluate
your patch don't have enough mental/time bandwidth to process them
right at the moment, and it often helps to wait until the list traffic
becomes calmer before sending such a reminder.
The list archive is available at a few public sites:
http://news.gmane.org/gmane.comp.version-control.git/
http://marc.info/?l=git
http://www.spinics.net/lists/git/
For those who prefer to read it over NNTP:
nntp://news.gmane.org/gmane.comp.version-control.git
When you point at a message in a mailing list archive, using
gmane is often the easiest to follow by readers, like this:
http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217
as it also allows people who subscribe to the mailing list as gmane
newsgroup to "jump to" the article.
Some members of the development community can sometimes be found on
the #git and #git-devel IRC channels on Freenode. Their logs are
available at:
http://colabti.org/irclogger/irclogger_log/git
http://colabti.org/irclogger/irclogger_log/git-devel
There is a volunteer-run newsletter to serve our community ("Git Rev
News" http://git.github.io/rev_news/rev_news.html).
Git is a member project of software freedom conservancy, a non-profit
organization (https://sfconservancy.org/). To reach a committee of
liaisons to the conservancy, contact them at .
* Reporting bugs
When you think git does not behave as you expect, please do not stop
your bug report with just "git does not work". "I used git in this
way, but it did not work" is not much better, neither is "I used git
in this way, and X happend, which is broken". It often is that git is
correct to cause X happen in such a case, and it is your expectation
that is broken. People would not know what other result Y you expected
to see instead of X, if you left it unsaid.
Please remember to always state
- what you wanted to achieve;
- what you did (the version of git and the command sequence to reproduce
the behavior);
- what you saw happen (X above);
- what you expected to see (Y above); and
- how the last two are different.
See http://www.chiark.greenend.org.uk/~sgtatham/bugs.html for further
hints.
If you think you found a security-sensitive issue and want to disclose
it to us without announcing it to wider public, please contact us at
our security mailing list . This is
a closed list that is limited to people who need to know early about
vulnerabilities, including:
- people triaging and fixing reported vulnerabilities
- people operating major git hosting sites with many users
- people packaging and distributing git to large numbers of people
where these issues are discussed without risk of the information
leaking out before we're ready to make public announcements.
* Repositories and documentation.
My public git.git repositories are at:
git://git.kernel.org/pub/scm/git/git.git/
https://kernel.googlesource.com/pub/scm/git/git
git://repo.or.cz/alt-git.git/
https://github.com/git/git/
git://git.sourceforge.jp/gitroot/git-core/git.git/
git://git-core.git.sourceforge.net/gitroot/git-core/git-core/
A few web interfaces are found at:
http://git.kernel.org/cgit/git/git.git
https://kernel.googlesource.com/pub/scm/git/git
http://repo.or.cz/w/alt-git.git
Preformatted documentation from the tip of the "master" branch can be
found in:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
and rendered in the browser if you visit this page:
https://git.github.io/htmldocs/git.html
Also GitHub shows the manual pages formatted in HTML (with a
formatting backend different from the one that is used to create the
above) at:
http://git-scm.com/docs/git
* How various branches are used.
There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "p
Re: [PATCHv4] Documentation: triangular workflow
"Philip Oakley" writes: > From: "Junio C Hamano" >> I do not think I agree. >> >> If you apriori know that you do want to hack on a project's code, then >> forking at GitHub first and then cloning the copy would be OK. > > You've clipped my other point: > > -One issue may be the different expectations of how the fork is > -created (it's only one click on the GitHub..) Not really. I said the same thing paraphrased: And at that point, you would create a publishing place, push into it, and tell others "Hey I did this interesting thing!". That "create a publishing place" step could be just a one click at GitHub. Anyway, let's step back a bit and agree that your local clone needs to be told 4 pieces of information to help you in a triangular workflow. They are: * The URL of the "upstream"; * The URL of the "publish" (aka "mine"); * Your push should go to "publish" by default; and * Your pull should come from "upstream" by default. Are we still on the same page, or have we already diverged? If you start from a clone of "upstream", then the local clone already knows two out of these four, namely, "The URL of the 'upstream' aka origin" and "Your pull should come from upstream by default". In order to go triangular, you still need to tell the other two to your local clone, namely "There is another remote called 'publish'" and "Your push should go to 'publish' not to 'origin'". When you start by forking at GitHub and then cloning that fork, would it make it easier to go triangular? I do not think so. The local clone gives you (different) two out of these four, namely "The URL of the 'publish' aka origin, which is your own fork" and "Your push should go to 'publish'". You still need to tell the other two, "The URL of the 'upstream'" and "Your pull should come from 'upstream', not from 'origin'" to your local repository. So I really do not see a point in arguing that "forking at GitHub is easy with one click" favors "your local clone should start by cloning your own fork". Between starting from a clone of upstream and starting from a clone of your own fork, it is the same amount of work to go triangular even for people who fork before having any clone locally, like you. And I do not have to repeat myself that it is far more helpful to give a recipe to go triangular starting from a clone of upstream for people who first clone three similar projects to evaluate them, discard two that do not suit their needs, and fork the best one, by adding that fork as "publish" aka "mine", than giving a recipe to start from a clone of one's own fork. Having said that, there are indeed two interesting numbers we may want to ask GitHub folks to help coming up with. Take any popular project with many public forks at GitHub, say rails with 13k forks. * How many "clone" from the upstream (i.e. rails/rails in this example) compared to 13k forks it has were made by the users? Many of them might be just following along (e.g. they want to build the tip of the tree that is ahead of any tagged released version), but there may be some among these local repositories cloned from upstream without forking that record their own commits. These are the people who will benefit a version of the documentation under discussion if it describes how to start from a clone of upstream, then add your own fork as the publishing repository so that the user can use a triangular workflow. I somehow suspect that there are a lot more than 13k (i.e. the number of public forks) of these people, but I do not think https://github.com/rails/rails page gives us the number. * What percentage of these 13k public forks are "empty forks", i.e. forked from the main project but has never been pushed into? It matters if the answer to this question is a non-trivial percentage. It indicates that the owners of these forks did the "fork first and then clone from there", and had to do an extra work of adding the real upstream as another remote and set up to pull from there, if they wanted to just keep up to date; they would have been better off if they started by cloning the upstream first. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] Don't free remote->name after fetch
From: Keith McGuigan The string_list gets populated with the names from the remotes[] array, which are not dup'd and the list does not own. Signed-of-by: Keith McGuigan --- builtin/fetch.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 630ae6a1bb78..181da5a2e7a3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) argv_array_clear(&options); } - /* All names were strdup()ed or strndup()ed */ - list.strdup_strings = 1; string_list_clear(&list, 0); close_all_packs(); -- 2.8.0.rc3.95.gc8e4e3a -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
merge committing staged deletions?
I have a case where git merge seems to include staged deletions into the merge commit. This seems pretty surprising, dunno if it's a bug. joey@darkstar:~/tmp/x/1>git rm 1 foo joey@darkstar:~/tmp/x/1>git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) deleted:1 deleted:foo joey@darkstar:~/tmp/x/1>git merge refs/heads/synced/master --no-ff Already up-to-date! Merge made by the 'recursive' strategy. 1 | 1 - foo | 1 - 2 files changed, 2 deletions(-) delete mode 100644 1 delete mode 100644 foo I thought that a merge would leave staged changes alone, unless they conflict in some way with the changes merged in. So why is merge looking at the staged deletions in this case? I'm using --no-ff because the commit being merged is itself a merge of HEAD and another commit. HEAD and the commit being merged in fact have the same tree, so the right merge solution, AFAICS, would be to keep that tree. I've attached a 1 kb git bundle that you can clone to reproduce this: git clone bundle b cd b git rm 1 git merge remotes/origin/synced/master --no-ff git version 2.8.1 -- see shy jo bundle Description: Binary data
Re: [PATCH 4/3] use string_list initializer consistently
On Mon, Jun 13, 2016 at 06:31:29PM +0700, Duy Nguyen wrote: > > This is a fairly mechanical conversion; I assumed each site > > was correct as-is, and just switched them all to NODUP. > > Looking good. If we still want to reduce noise level down (by a tiny > bit), we could remove _INIT because I think it's obvious from > > struct string_list var = STRING_LIST_NODUP; > > that's it's initialization (I wish we could write "auto var = > STRING_LIST_NODUP;") Yeah, I've always felt it's a bit long. The "INIT" does match other similar macros, but I agree it could probably go. Definitely something for a separate patch, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/27] nd/shallow-deepen updates
Eric Sunshine writes: > I agree with Junio that moving the sigchain_pop() into the error > handling code-path, if possible, would be a nice improvement. Yeah, "if possible" is really what I was not sure about---is it safe to do the _push() thing before start_command(), which presumably would affect both the main and the forked processes? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] lib-httpd.sh: print error.log on error
On Mon, Jun 13, 2016 at 07:35:09PM +0700, Nguyễn Thái Ngọc Duy wrote: > Failure to bring up httpd for testing is not considered an error, so the > trash directory, which contains this error.log file, is removed and we > don't know what made httpd fail to start. Improve the situation a bit, > print error.log but only in verbose mode. > > Signed-off-by: Nguyễn Thái Ngọc Duy Looks good to me. Thanks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 00/40] libify apply and use lib in am, part 2
On Mon, Jun 13, 2016 at 6:09 PM, Christian Couder
wrote:
>
> I will send a diff between this version and the previous one, as a
> reply to this email.
Here is the diff:
diff --git a/apply.c b/apply.c
index cd4cd01..98a 100644
--- a/apply.c
+++ b/apply.c
@@ -4386,7 +4386,7 @@ static int create_one_file(struct apply_state *state,
++nr;
}
}
- return error_errno(_("unable to write file '%s' mode %o: %s"),
+ return error_errno(_("unable to write file '%s' mode %o"),
path, mode);
}
diff --git a/builtin/am.c b/builtin/am.c
index 43f7316..8647298 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1584,7 +1584,6 @@ static int run_apply(const struct am_state
*state, const char *index_file)
* If we are allowed to fall back on 3-way merge, don't give false
* errors during the initial attempt.
*/
-
if (state->threeway && !index_file)
apply_state.be_silent = 1;
@@ -1600,6 +1599,7 @@ static int run_apply(const struct am_state
*state, const char *index_file)
argv_array_clear(&apply_paths);
argv_array_clear(&apply_opts);
+ clear_apply_state(&apply_state);
if (res)
return res;
diff --git a/builtin/apply.c b/builtin/apply.c
index 93744f8..ddd61de 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -74,8 +74,6 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
- OPT_BOOL(0, "silent", &state.be_silent,
- N_("do not print any output")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing
new-line at the end of file"),
APPLY_OPT_INACCURATE_EOF),
diff --git a/run-command.c b/run-command.c
index c09d6b0..af0c8a1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
}
#ifndef GIT_WINDOWS_NATIVE
-static void dup_devnull(int to)
+static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
if (fd < 0)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 22/40] builtin/apply: make create_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_file() should just return what
add_conflicted_stages_file() and add_index_file() are returning
instead of calling exit().
Signed-off-by: Christian Couder
---
builtin/apply.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 005ba78..76d473c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4258,7 +4258,7 @@ static int add_conflicted_stages_file(struct apply_state
*state,
return 0;
}
-static void create_file(struct apply_state *state, struct patch *patch)
+static int create_file(struct apply_state *state, struct patch *patch)
{
char *path = patch->new_name;
unsigned mode = patch->new_mode;
@@ -4269,13 +4269,10 @@ static void create_file(struct apply_state *state,
struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(state, path, mode, buf, size);
- if (patch->conflicted_threeway) {
- if (add_conflicted_stages_file(state, patch))
- exit(1);
- } else {
- if (add_index_file(state, path, mode, buf, size))
- exit(1);
- }
+ if (patch->conflicted_threeway)
+ return add_conflicted_stages_file(state, patch);
+ else
+ return add_index_file(state, path, mode, buf, size);
}
/* phase zero is to remove, phase one is to create */
@@ -4291,8 +4288,10 @@ static void write_out_one_result(struct apply_state
*state,
return;
}
if (patch->is_new > 0 || patch->is_copy) {
- if (phase == 1)
- create_file(state, patch);
+ if (phase == 1) {
+ if (create_file(state, patch))
+ exit(1);
+ }
return;
}
/*
@@ -4303,8 +4302,10 @@ static void write_out_one_result(struct apply_state
*state,
if (remove_file(state, patch, patch->is_rename))
exit(1);
}
- if (phase == 1)
- create_file(state, patch);
+ if (phase == 1) {
+ if (create_file(state, patch))
+ exit(1);
+ }
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 15/40] builtin/apply: make gitdiff_*() return 1 at end of header
The gitdiff_*() functions that are called as p->fn() in parse_git_header()
should return 1 instead of -1 in case of end of header or unrecognized
input, as these are not real errors. It just instructs the parser to break
out.
This makes it possible for gitdiff_*() functions to return -1 in case of a
real error. This will be done in a following patch.
Helped-by: Nguyễn Thái Ngọc Duy
Signed-off-by: Christian Couder
---
builtin/apply.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index eb98116..1142514 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -812,7 +812,7 @@ static int gitdiff_hdrend(struct apply_state *state,
const char *line,
struct patch *patch)
{
- return -1;
+ return 1;
}
/*
@@ -1016,7 +1016,7 @@ static int gitdiff_unrecognized(struct apply_state *state,
const char *line,
struct patch *patch)
{
- return -1;
+ return 1;
}
/*
@@ -1248,9 +1248,13 @@ static int parse_git_header(struct apply_state *state,
for (i = 0; i < ARRAY_SIZE(optable); i++) {
const struct opentry *p = optable + i;
int oplen = strlen(p->str);
+ int res;
if (len < oplen || memcmp(p->str, line, oplen))
continue;
- if (p->fn(state, line + oplen, patch) < 0)
+ res = p->fn(state, line + oplen, patch);
+ if (res < 0)
+ return -1;
+ if (res > 0)
return offset;
break;
}
@@ -1429,6 +1433,8 @@ static int find_header(struct apply_state *state,
*/
if (!memcmp("diff --git ", line, 11)) {
int git_hdr_len = parse_git_header(state, line, len,
size, patch);
+ if (git_hdr_len < 0)
+ return -1;
if (git_hdr_len <= len)
continue;
if (!patch->old_name && !patch->new_name) {
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 38/40] apply: change error_routine when be_silent is set
To avoid printing anything when applying with be_silent set,
let's save the existing warn and error routines before
applying and replace them with a routine that does nothing.
Then after applying, let's restore the saved routines.
Signed-off-by: Christian Couder
---
apply.c | 23 +--
apply.h | 4
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index 2529534..ef49709 100644
--- a/apply.c
+++ b/apply.c
@@ -109,6 +109,11 @@ void clear_apply_state(struct apply_state *state)
/* &state->fn_table is cleared at the end of apply_patch() */
}
+static void mute_routine(const char *bla, va_list params)
+{
+ /* do nothing */
+}
+
int check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
@@ -143,6 +148,13 @@ int check_apply_state(struct apply_state *state, int
force_apply)
if (!state->lock_file)
return error("BUG: state->lock_file should not be NULL");
+ if (state->be_silent) {
+ state->saved_error_routine = get_error_routine();
+ state->saved_warn_routine = get_warn_routine();
+ set_error_routine(mute_routine);
+ set_warn_routine(mute_routine);
+ }
+
return 0;
}
@@ -4760,6 +4772,7 @@ int apply_all_patches(struct apply_state *state,
{
int i;
int res;
+ int retval = -1;
int errs = 0;
int read_stdin = 1;
@@ -4838,12 +4851,18 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
- return !!errs;
+ retval = !!errs;
rollback_end:
if (state->newfd >= 0) {
rollback_lock_file(state->lock_file);
state->newfd = -1;
}
- return -1;
+
+ if (state->be_silent) {
+ set_error_routine(state->saved_error_routine);
+ set_warn_routine(state->saved_warn_routine);
+ }
+
+ return retval;
}
diff --git a/apply.h b/apply.h
index 034541a..c6cf33d 100644
--- a/apply.h
+++ b/apply.h
@@ -89,6 +89,10 @@ struct apply_state {
*/
struct string_list fn_table;
+ /* This is to save some reporting routines */
+ void (*saved_error_routine)(const char *err, va_list params);
+ void (*saved_warn_routine)(const char *warn, va_list params);
+
/* These control whitespace errors */
enum ws_error_action ws_error_action;
enum ws_ignore ws_ignore_action;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 28/40] apply: rename and move opt constants to apply.h
The constants for the "inaccurate-eof" and the "recount" options will
be used in both "apply.c" and "builtin/apply.c", so they need to go
into "apply.h", and therefore they need a name that is more specific
to the API they belong to.
Signed-off-by: Christian Couder
---
apply.h | 3 +++
builtin/apply.c | 11 ---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/apply.h b/apply.h
index 1f2277e..3cc3da6 100644
--- a/apply.h
+++ b/apply.h
@@ -108,4 +108,7 @@ extern int init_apply_state(struct apply_state *state,
extern void clear_apply_state(struct apply_state *state);
extern int check_apply_state(struct apply_state *state, int force_apply);
+#define APPLY_OPT_INACCURATE_EOF (1<<0)
+#define APPLY_OPT_RECOUNT (1<<1)
+
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index c84add0..a06ab55 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4449,9 +4449,6 @@ static int write_out_results(struct apply_state *state,
struct patch *list)
static struct lock_file lock_file;
-#define INACCURATE_EOF (1<<0)
-#define RECOUNT(1<<1)
-
/*
* Try to apply a patch.
*
@@ -4480,8 +4477,8 @@ static int apply_patch(struct apply_state *state,
int nr;
patch = xcalloc(1, sizeof(*patch));
- patch->inaccurate_eof = !!(options & INACCURATE_EOF);
- patch->recount = !!(options & RECOUNT);
+ patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);
+ patch->recount = !!(options & APPLY_OPT_RECOUNT);
nr = parse_chunk(state, buf.buf + offset, buf.len - offset,
patch);
if (nr < 0) {
free_patch(patch);
@@ -4785,10 +4782,10 @@ int cmd_apply(int argc, const char **argv, const char
*prefix)
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line at
the end of file"),
- INACCURATE_EOF),
+ APPLY_OPT_INACCURATE_EOF),
OPT_BIT(0, "recount", &options,
N_("do not trust the line counts in the hunk headers"),
- RECOUNT),
+ APPLY_OPT_RECOUNT),
{ OPTION_CALLBACK, 0, "directory", &state, N_("root"),
N_("prepend to all filenames"),
0, apply_option_parse_directory },
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 18/40] builtin/apply: make build_fake_ancestor() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", build_fake_ancestor() should return -1 instead
of calling die().
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 429fddd..e74b068 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3889,11 +3889,12 @@ static int preimage_sha1_in_gitlink_patch(struct patch
*p, unsigned char sha1[20
}
/* Build an index that contains the just the files needed for a 3way merge */
-static void build_fake_ancestor(struct patch *list, const char *filename)
+static int build_fake_ancestor(struct patch *list, const char *filename)
{
struct patch *patch;
struct index_state result = { NULL };
static struct lock_file lock;
+ int res;
/* Once we start supporting the reverse patch, it may be
* worth showing the new sha1 prefix, but until then...
@@ -3911,31 +3912,38 @@ static void build_fake_ancestor(struct patch *list,
const char *filename)
if (!preimage_sha1_in_gitlink_patch(patch, sha1))
; /* ok, the textual part looks sane */
else
- die("sha1 information is lacking or useless for
submodule %s",
- name);
+ return error("sha1 information is lacking or "
+"useless for submodule %s", name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
if (get_current_sha1(patch->old_name, sha1))
- die("mode change for %s, which is not "
- "in current HEAD", name);
+ return error("mode change for %s, which is not "
+"in current HEAD", name);
} else
- die("sha1 information is lacking or useless "
- "(%s).", name);
+ return error("sha1 information is lacking or useless "
+"(%s).", name);
ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);
if (!ce)
- die(_("make_cache_entry failed for path '%s'"), name);
- if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
- die ("Could not add %s to temporary index", name);
+ return error(_("make_cache_entry failed for path '%s'"),
+name);
+ if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD)) {
+ free(ce);
+ return error("Could not add %s to temporary index",
+name);
+ }
}
hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
- if (write_locked_index(&result, &lock, COMMIT_LOCK))
- die ("Could not write temporary index to %s", filename);
-
+ res = write_locked_index(&result, &lock, COMMIT_LOCK);
discard_index(&result);
+
+ if (res)
+ return error("Could not write temporary index to %s", filename);
+
+ return 0;
}
static void stat_patch_list(struct apply_state *state, struct patch *patch)
@@ -4476,8 +4484,11 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if (state->fake_ancestor)
- build_fake_ancestor(list, state->fake_ancestor);
+ if (state->fake_ancestor &&
+ build_fake_ancestor(list, state->fake_ancestor)) {
+ res = -1;
+ goto end;
+ }
if (state->diffstat)
stat_patch_list(state, list);
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 31/40] environment: add set_index_file()
Introduce set_index_file() to be able to temporarily change the index file.
It should be used like this:
/* Save current index file */
old_index_file = get_index_file();
set_index_file((char *)tmp_index_file);
/* Do stuff that will use tmp_index_file as the index file */
...
/* When finished reset the index file */
set_index_file(old_index_file);
Signed-off-by: Christian Couder
---
cache.h | 1 +
environment.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/cache.h b/cache.h
index 81d4ac3..28fc0bf 100644
--- a/cache.h
+++ b/cache.h
@@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);
extern const char *get_git_dir(void);
extern const char *get_git_common_dir(void);
extern char *get_object_directory(void);
+extern void set_index_file(char *index_file);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
diff --git a/environment.c b/environment.c
index ca72464..7a53799 100644
--- a/environment.c
+++ b/environment.c
@@ -292,6 +292,16 @@ int odb_pack_keep(char *name, size_t namesz, const
unsigned char *sha1)
return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
}
+/*
+ * Temporarily change the index file.
+ * Please save the current index file using get_index_file() before changing
+ * the index file. And when finished, reset it to the saved value.
+ */
+void set_index_file(char *index_file)
+{
+ git_index_file = index_file;
+}
+
char *get_index_file(void)
{
if (!git_index_file)
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 30/40] apply: make some parsing functions static again
Some parsing functions that were used in both "apply.c" and
"builtin/apply.c" are now only used in the former, so they
can be made static to "apply.c".
Signed-off-by: Christian Couder
---
apply.c | 6 +++---
apply.h | 5 -
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index 4920fa8..713d1c0 100644
--- a/apply.c
+++ b/apply.c
@@ -27,7 +27,7 @@ static void git_apply_config(void)
git_config(git_default_config, NULL);
}
-int parse_whitespace_option(struct apply_state *state, const char *option)
+static int parse_whitespace_option(struct apply_state *state, const char
*option)
{
if (!option) {
state->ws_error_action = warn_on_ws_error;
@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const
char *option)
return error(_("unrecognized whitespace option '%s'"), option);
}
-int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option)
{
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
diff --git a/apply.h b/apply.h
index 736f818..89e7982 100644
--- a/apply.h
+++ b/apply.h
@@ -97,11 +97,6 @@ struct apply_state {
int applied_after_fixing_ws;
};
-extern int parse_whitespace_option(struct apply_state *state,
- const char *option);
-extern int parse_ignorewhitespace_option(struct apply_state *state,
-const char *option);
-
extern int apply_option_parse_exclude(const struct option *opt,
const char *arg, int unset);
extern int apply_option_parse_include(const struct option *opt,
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 35/40] apply: don't print on stdout when be_silent is set
This variable should prevent anything to be printed on both stderr and stdout. Signed-off-by: Christian Couder --- apply.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apply.c b/apply.c index dd9b301..2529534 100644 --- a/apply.c +++ b/apply.c @@ -4679,13 +4679,13 @@ static int apply_patch(struct apply_state *state, goto end; } - if (state->diffstat) + if (state->diffstat && !state->be_silent) stat_patch_list(state, list); - if (state->numstat) + if (state->numstat && !state->be_silent) numstat_patch_list(state, list); - if (state->summary) + if (state->summary && !state->be_silent) summary_patch_list(list); end: -- 2.9.0.rc2.411.g3e2ca28 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 25/40] builtin/apply: make try_create_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", try_create_file() should return -1 in case of
error.
Unfortunately try_create_file() currently returns -1 to signal a
recoverable error. To fix that, let's make it return 1 in case of
a recoverable error and -1 in case of an unrecoverable error.
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 41 ++---
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index f35c901..37f0c2e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4139,38 +4139,45 @@ static int add_index_file(struct apply_state *state,
return 0;
}
+/*
+ * Returns:
+ * -1 if an unrecoverable error happened
+ * 0 if everything went well
+ * 1 if a recoverable error happened
+ */
static int try_create_file(const char *path, unsigned int mode, const char
*buf, unsigned long size)
{
- int fd;
+ int fd, res;
struct strbuf nbuf = STRBUF_INIT;
if (S_ISGITLINK(mode)) {
struct stat st;
if (!lstat(path, &st) && S_ISDIR(st.st_mode))
return 0;
- return mkdir(path, 0777);
+ return !!mkdir(path, 0777);
}
if (has_symlinks && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
* terminated.
*/
- return symlink(buf, path);
+ return !!symlink(buf, path);
fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 :
0666);
if (fd < 0)
- return -1;
+ return 1;
if (convert_to_working_tree(path, buf, size, &nbuf)) {
size = nbuf.len;
buf = nbuf.buf;
}
- write_or_die(fd, buf, size);
+ res = !write_or_whine_pipe(fd, buf, size, path);
strbuf_release(&nbuf);
- if (close(fd) < 0)
- die_errno(_("closing file '%s'"), path);
- return 0;
+ if (close(fd) < 0 && !res)
+ return error(_("closing file '%s': %s"), path, strerror(errno));
+
+ return res ? -1 : 0;
}
/*
@@ -4184,15 +4191,24 @@ static void create_one_file(struct apply_state *state,
const char *buf,
unsigned long size)
{
+ int res;
+
if (state->cached)
return;
- if (!try_create_file(path, mode, buf, size))
+
+ res = try_create_file(path, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res)
return;
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
return;
- if (!try_create_file(path, mode, buf, size))
+ res = try_create_file(path, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res)
return;
}
@@ -4211,7 +4227,10 @@ static void create_one_file(struct apply_state *state,
for (;;) {
char newpath[PATH_MAX];
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
- if (!try_create_file(newpath, mode, buf, size)) {
+ res = try_create_file(newpath, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res) {
if (!rename(newpath, path))
return;
unlink_or_warn(newpath);
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 14/40] builtin/apply: make parse_traditional_patch() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", parse_traditional_patch() should return -1
instead of calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c27be35..eb98116 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -755,10 +755,10 @@ static int has_epoch_timestamp(const char *nameline)
* files, we can happily check the index for a match, but for creating a
* new file we should try to match whatever "patch" does. I have no idea.
*/
-static void parse_traditional_patch(struct apply_state *state,
- const char *first,
- const char *second,
- struct patch *patch)
+static int parse_traditional_patch(struct apply_state *state,
+ const char *first,
+ const char *second,
+ struct patch *patch)
{
char *name;
@@ -803,7 +803,9 @@ static void parse_traditional_patch(struct apply_state
*state,
}
}
if (!name)
- die(_("unable to find filename in patch at line %d"),
state->linenr);
+ return error(_("unable to find filename in patch at line %d"),
state->linenr);
+
+ return 0;
}
static int gitdiff_hdrend(struct apply_state *state,
@@ -1462,7 +1464,8 @@ static int find_header(struct apply_state *state,
continue;
/* Ok, we'll consider it a patch */
- parse_traditional_patch(state, line, line+len, patch);
+ if (parse_traditional_patch(state, line, line+len, patch))
+ return -1;
*hdrsize = len + nextlen;
state->linenr += 2;
return offset;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 21/40] builtin/apply: make add_index_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_index_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 48 +++-
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0997384..005ba78 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4088,11 +4088,11 @@ static int remove_file(struct apply_state *state,
struct patch *patch, int rmdir
return 0;
}
-static void add_index_file(struct apply_state *state,
- const char *path,
- unsigned mode,
- void *buf,
- unsigned long size)
+static int add_index_file(struct apply_state *state,
+ const char *path,
+ unsigned mode,
+ void *buf,
+ unsigned long size)
{
struct stat st;
struct cache_entry *ce;
@@ -4100,7 +4100,7 @@ static void add_index_file(struct apply_state *state,
unsigned ce_size = cache_entry_size(namelen);
if (!state->update_index)
- return;
+ return 0;
ce = xcalloc(1, ce_size);
memcpy(ce->name, path, namelen);
@@ -4111,20 +4111,32 @@ static void add_index_file(struct apply_state *state,
const char *s;
if (!skip_prefix(buf, "Subproject commit ", &s) ||
- get_sha1_hex(s, ce->sha1))
- die(_("corrupt patch for submodule %s"), path);
+ get_sha1_hex(s, ce->sha1)) {
+ free(ce);
+ return error(_("corrupt patch for submodule %s"), path);
+ }
} else {
if (!state->cached) {
- if (lstat(path, &st) < 0)
- die_errno(_("unable to stat newly created file
'%s'"),
- path);
+ if (lstat(path, &st) < 0) {
+ free(ce);
+ return error(_("unable to stat newly "
+ "created file '%s': %s"),
+path, strerror(errno));
+ }
fill_stat_cache_info(ce, &st);
}
- if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
- die(_("unable to create backing store for newly created
file %s"), path);
+ if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0) {
+ free(ce);
+ return error(_("unable to create backing store "
+ "for newly created file %s"), path);
+ }
}
- if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
- die(_("unable to add cache entry for %s"), path);
+ if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+ free(ce);
+ return error(_("unable to add cache entry for %s"), path);
+ }
+
+ return 0;
}
static int try_create_file(const char *path, unsigned int mode, const char
*buf, unsigned long size)
@@ -4260,8 +4272,10 @@ static void create_file(struct apply_state *state,
struct patch *patch)
if (patch->conflicted_threeway) {
if (add_conflicted_stages_file(state, patch))
exit(1);
- } else
- add_index_file(state, path, mode, buf, size);
+ } else {
+ if (add_index_file(state, path, mode, buf, size))
+ exit(1);
+ }
}
/* phase zero is to remove, phase one is to create */
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 09/40] builtin/apply: move init_apply_state() to apply.c
To libify `git apply` functionality we must make init_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into a new "apply.c".
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
Makefile| 1 +
apply.c | 91 +
apply.h | 10 +++
builtin/apply.c | 88 ---
4 files changed, 102 insertions(+), 88 deletions(-)
create mode 100644 apply.c
diff --git a/Makefile b/Makefile
index 6b05249..6da1209 100644
--- a/Makefile
+++ b/Makefile
@@ -680,6 +680,7 @@ LIB_OBJS += abspath.o
LIB_OBJS += advice.o
LIB_OBJS += alias.o
LIB_OBJS += alloc.o
+LIB_OBJS += apply.o
LIB_OBJS += archive.o
LIB_OBJS += archive-tar.o
LIB_OBJS += archive-zip.o
diff --git a/apply.c b/apply.c
new file mode 100644
index 000..1f31bb4
--- /dev/null
+++ b/apply.c
@@ -0,0 +1,91 @@
+#include "cache.h"
+#include "lockfile.h"
+#include "apply.h"
+
+static void git_apply_config(void)
+{
+ git_config_get_string_const("apply.whitespace",
&apply_default_whitespace);
+ git_config_get_string_const("apply.ignorewhitespace",
&apply_default_ignorewhitespace);
+ git_config(git_default_config, NULL);
+}
+
+int parse_whitespace_option(struct apply_state *state, const char *option)
+{
+ if (!option) {
+ state->ws_error_action = warn_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "warn")) {
+ state->ws_error_action = warn_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "nowarn")) {
+ state->ws_error_action = nowarn_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "error")) {
+ state->ws_error_action = die_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "error-all")) {
+ state->ws_error_action = die_on_ws_error;
+ state->squelch_whitespace_errors = 0;
+ return 0;
+ }
+ if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
+ state->ws_error_action = correct_ws_error;
+ return 0;
+ }
+ return error(_("unrecognized whitespace option '%s'"), option);
+}
+
+int parse_ignorewhitespace_option(struct apply_state *state,
+ const char *option)
+{
+ if (!option || !strcmp(option, "no") ||
+ !strcmp(option, "false") || !strcmp(option, "never") ||
+ !strcmp(option, "none")) {
+ state->ws_ignore_action = ignore_ws_none;
+ return 0;
+ }
+ if (!strcmp(option, "change")) {
+ state->ws_ignore_action = ignore_ws_change;
+ return 0;
+ }
+ return error(_("unrecognized whitespace ignore option '%s'"), option);
+}
+
+void init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file)
+{
+ memset(state, 0, sizeof(*state));
+ state->prefix = prefix;
+ state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
+ state->lock_file = lock_file;
+ state->newfd = -1;
+ state->apply = 1;
+ state->line_termination = '\n';
+ state->p_value = 1;
+ state->p_context = UINT_MAX;
+ state->squelch_whitespace_errors = 5;
+ state->ws_error_action = warn_on_ws_error;
+ state->ws_ignore_action = ignore_ws_none;
+ state->linenr = 1;
+ strbuf_init(&state->root, 0);
+
+ git_apply_config();
+ if (apply_default_whitespace && parse_whitespace_option(state,
apply_default_whitespace))
+ exit(1);
+ if (apply_default_ignorewhitespace &&
parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ exit(1);
+}
+
+void clear_apply_state(struct apply_state *state)
+{
+ string_list_clear(&state->limit_by_name, 0);
+ string_list_clear(&state->symlink_changes, 0);
+ strbuf_release(&state->root);
+
+ /* &state->fn_table is cleared at the end of apply_patch() */
+}
diff --git a/apply.h b/apply.h
index 9a98eae..77502be 100644
--- a/apply.h
+++ b/apply.h
@@ -97,4 +97,14 @@ struct apply_state {
int applied_after_fixing_ws;
};
+extern int parse_whitespace_option(struct apply_state *state,
+ const char *option);
+extern int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option);
+
+extern void init_apply_state(struct apply_state *state,
+const char *prefix,
+struct lock_file *lock_file);
+extern void clear_apply_state(struct apply_state *state);
+
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index e2f970d..bc15545 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -27,52 +27,6 @@ static const char * const apply_usage[]
[PATCH v7 36/40] usage: add set_warn_routine()
There are already set_die_routine() and set_error_routine(),
so let's add set_warn_routine() as this will be needed in a
following commit.
Signed-off-by: Christian Couder
---
git-compat-util.h | 1 +
usage.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 49d4029..f78706a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,6 +440,7 @@ static inline int const_error(void)
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err,
va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
+extern void set_warn_routine(void (*routine)(const char *warn, va_list
params));
extern void set_die_is_recursing_routine(int (*routine)(void));
extern void set_error_handle(FILE *);
diff --git a/usage.c b/usage.c
index 1dad03f..67e5526 100644
--- a/usage.c
+++ b/usage.c
@@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err,
va_list params))
error_routine = routine;
}
+void set_warn_routine(void (*routine)(const char *warn, va_list params))
+{
+ warn_routine = routine;
+}
+
void set_die_is_recursing_routine(int (*routine)(void))
{
die_is_recursing = routine;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 24/40] builtin/apply: make write_out_results() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_results() should return -1 instead of
calling exit().
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 291e24e..f35c901 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4372,6 +4372,12 @@ static int write_out_one_reject(struct apply_state
*state, struct patch *patch)
return -1;
}
+/*
+ * Returns:
+ * -1 if an error happened
+ * 0 if the patch applied cleanly
+ * 1 if the patch did not apply cleanly
+ */
static int write_out_results(struct apply_state *state, struct patch *list)
{
int phase;
@@ -4385,8 +4391,10 @@ static int write_out_results(struct apply_state *state,
struct patch *list)
if (l->rejected)
errs = 1;
else {
- if (write_out_one_result(state, l, phase))
- exit(1);
+ if (write_out_one_result(state, l, phase)) {
+ string_list_clear(&cpath, 0);
+ return -1;
+ }
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
@@ -4498,10 +4506,17 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if (state->apply && write_out_results(state, list)) {
- /* with --3way, we still need to write the index out */
- res = state->apply_with_reject ? -1 : 1;
- goto end;
+ if (state->apply) {
+ int write_res = write_out_results(state, list);
+ if (write_res < 0) {
+ res = -1;
+ goto end;
+ }
+ if (write_res > 0) {
+ /* with --3way, we still need to write the index out */
+ res = state->apply_with_reject ? -1 : 1;
+ goto end;
+ }
}
if (state->fake_ancestor &&
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 33/40] apply: add 'be_silent' variable to 'struct apply_state'
This variable should prevent anything to be printed on both stderr
and stdout.
Let's not take care of stdout and apply_verbosely for now though,
as that will be taken care of in following patches.
Signed-off-by: Christian Couder
---
apply.c | 43 +--
apply.h | 1 +
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/apply.c b/apply.c
index 713d1c0..dbb2515 100644
--- a/apply.c
+++ b/apply.c
@@ -1612,8 +1612,9 @@ static void record_ws_error(struct apply_state *state,
return;
err = whitespace_error_string(result);
- fprintf(stderr, "%s:%d: %s.\n%.*s\n",
- state->patch_input_file, linenr, err, len, line);
+ if (!state->be_silent)
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ state->patch_input_file, linenr, err, len, line);
free(err);
}
@@ -1808,7 +1809,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"),
patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"),
patch->old_name);
- if (!patch->is_delete && !newlines && context)
+ if (!patch->is_delete && !newlines && context && !state->be_silent)
fprintf_ln(stderr,
_("** warning: "
"file %s becomes empty but is not deleted"),
@@ -3031,8 +3032,8 @@ static int apply_one_fragment(struct apply_state *state,
* Warn if it was necessary to reduce the number
* of context lines.
*/
- if ((leading != frag->leading) ||
- (trailing != frag->trailing))
+ if ((leading != frag->leading ||
+trailing != frag->trailing) && !state->be_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
" to apply fragment at %d"),
leading, trailing, applied_pos+1);
@@ -3529,7 +3530,8 @@ static int try_threeway(struct apply_state *state,
read_blob_object(&buf, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back
on 3-way merge.");
- fprintf(stderr, "Falling back to three-way merge...\n");
+ if (!state->be_silent)
+ fprintf(stderr, "Falling back to three-way merge...\n");
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3559,7 +3561,9 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
pre_sha1, our_sha1, post_sha1);
if (status < 0) {
- fprintf(stderr, "Failed to fall back on three-way merge...\n");
+ if (!state->be_silent)
+ fprintf(stderr,
+ "Failed to fall back on three-way merge...\n");
return status;
}
@@ -3571,9 +3575,15 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
- fprintf(stderr, "Applied patch to '%s' with conflicts.\n",
patch->new_name);
+ if (!state->be_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' with conflicts.\n",
+ patch->new_name);
} else {
- fprintf(stderr, "Applied patch to '%s' cleanly.\n",
patch->new_name);
+ if (!state->be_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' cleanly.\n",
+ patch->new_name);
}
return 0;
}
@@ -4472,7 +4482,8 @@ static int write_out_one_reject(struct apply_state
*state, struct patch *patch)
"Applying patch %%s with %d rejects...",
cnt),
cnt);
- say_patch_name(stderr, sb.buf, patch);
+ if (!state->be_silent)
+ say_patch_name(stderr, sb.buf, patch);
strbuf_release(&sb);
cnt = strlen(patch->new_name);
@@ -4499,10 +4510,12 @@ static int write_out_one_reject(struct apply_state
*state, struct patch *patch)
frag;
cnt++, frag = frag->next) {
if (!frag->rejected) {
- fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
+ if (!state->be_silent)
+ fprintf_ln(stderr, _("Hunk #%d applied
cleanly."), cnt);
continue;
}
- fprintf_ln(stderr, _("Rejected h
[PATCH v7 19/40] builtin/apply: make remove_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", remove_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index e74b068..694c65b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4074,17 +4074,18 @@ static void patch_stats(struct apply_state *state,
struct patch *patch)
}
}
-static void remove_file(struct apply_state *state, struct patch *patch, int
rmdir_empty)
+static int remove_file(struct apply_state *state, struct patch *patch, int
rmdir_empty)
{
if (state->update_index) {
if (remove_file_from_cache(patch->old_name) < 0)
- die(_("unable to remove %s from index"),
patch->old_name);
+ return error(_("unable to remove %s from index"),
patch->old_name);
}
if (!state->cached) {
if (!remove_or_warn(patch->old_mode, patch->old_name) &&
rmdir_empty) {
remove_path(patch->old_name);
}
}
+ return 0;
}
static void add_index_file(struct apply_state *state,
@@ -4263,8 +4264,10 @@ static void write_out_one_result(struct apply_state
*state,
int phase)
{
if (patch->is_delete > 0) {
- if (phase == 0)
- remove_file(state, patch, 1);
+ if (phase == 0) {
+ if (remove_file(state, patch, 1))
+ exit(1);
+ }
return;
}
if (patch->is_new > 0 || patch->is_copy) {
@@ -4276,8 +4279,10 @@ static void write_out_one_result(struct apply_state
*state,
* Rename or modification boils down to the same
* thing: remove the old, write the new
*/
- if (phase == 0)
- remove_file(state, patch, patch->is_rename);
+ if (phase == 0) {
+ if (remove_file(state, patch, patch->is_rename))
+ exit(1);
+ }
if (phase == 1)
create_file(state, patch);
}
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 06/40] builtin/apply: make parse_single_patch() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_single_patch() should return -1 instead of
calling die().
Let's do that by using error() and let's adjust the related test
cases accordingly.
Signed-off-by: Christian Couder
---
builtin/apply.c| 17 +
t/t4012-diff-binary.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a160338..2671586 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1666,6 +1666,10 @@ static int parse_fragment(struct apply_state *state,
*
* The (fragment->patch, fragment->size) pair points into the memory given
* by the caller, not a copy, when we return.
+ *
+ * Returns:
+ * -1 in case of error,
+ * the number of bytes in the patch otherwise.
*/
static int parse_single_patch(struct apply_state *state,
const char *line,
@@ -1683,8 +1687,10 @@ static int parse_single_patch(struct apply_state *state,
fragment = xcalloc(1, sizeof(*fragment));
fragment->linenr = state->linenr;
len = parse_fragment(state, line, size, patch, fragment);
- if (len <= 0)
- die(_("corrupt patch at line %d"), state->linenr);
+ if (len <= 0) {
+ free(fragment);
+ return error(_("corrupt patch at line %d"),
state->linenr);
+ }
fragment->patch = line;
fragment->size = len;
oldlines += fragment->oldlines;
@@ -1720,9 +1726,9 @@ static int parse_single_patch(struct apply_state *state,
patch->is_delete = 0;
if (0 < patch->is_new && oldlines)
- die(_("new file %s depends on old contents"), patch->new_name);
+ return error(_("new file %s depends on old contents"),
patch->new_name);
if (0 < patch->is_delete && newlines)
- die(_("deleted file %s still has contents"), patch->old_name);
+ return error(_("deleted file %s still has contents"),
patch->old_name);
if (!patch->is_delete && !newlines && context)
fprintf_ln(stderr,
_("** warning: "
@@ -2024,6 +2030,9 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
size - offset - hdrsize,
patch);
+ if (patchsize < 0)
+ return -1;
+
if (!patchsize) {
static const char git_binary[] = "GIT binary patch\n";
int hd = hdrsize + offset;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 643d729..0a8af76 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt
patch correctly' '
sed -e "s/-CIT/xCIT/" broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt
patch correctly' '
git diff --binary | sed -e "s/-CIT/xCIT/" >broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/40] builtin/apply: read_patch_file() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing. Let's do that by returning -1 instead of
die()ing in read_patch_file().
Signed-off-by: Christian Couder
---
builtin/apply.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 598e479..2ff8450 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -335,10 +335,10 @@ static void say_patch_name(FILE *output, const char *fmt,
struct patch *patch)
#define SLOP (16)
-static void read_patch_file(struct strbuf *sb, int fd)
+static int read_patch_file(struct strbuf *sb, int fd)
{
if (strbuf_read(sb, fd, 0) < 0)
- die_errno("git apply: failed to read");
+ return error_errno("git apply: failed to read");
/*
* Make sure that we have some slop in the buffer
@@ -347,6 +347,7 @@ static void read_patch_file(struct strbuf *sb, int fd)
*/
strbuf_grow(sb, SLOP);
memset(sb->buf + sb->len, 0, SLOP);
+ return 0;
}
static unsigned long linelen(const char *buffer, unsigned long size)
@@ -4424,7 +4425,8 @@ static int apply_patch(struct apply_state *state,
int res = 0;
state->patch_input_file = filename;
- read_patch_file(&buf, fd);
+ if (read_patch_file(&buf, fd))
+ return -1;
offset = 0;
while (offset < buf.len) {
struct patch *patch;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors
to the caller instead of die()ing.
As a first step in this direction, let's make apply_patch() return
-1 in case of errors instead of dying. For now its only caller
apply_all_patches() will exit(1) when apply_patch() return -1.
In a later patch, apply_all_patches() will return -1 too instead of
exiting.
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 54 +++---
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 3a0c53a..598e479 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4404,6 +4404,14 @@ static struct lock_file lock_file;
#define INACCURATE_EOF (1<<0)
#define RECOUNT(1<<1)
+/*
+ * Try to apply a patch.
+ *
+ * Returns:
+ * -1 if an error happened
+ * 0 if the patch applied
+ * 1 if the patch did not apply
+ */
static int apply_patch(struct apply_state *state,
int fd,
const char *filename,
@@ -4413,6 +4421,7 @@ static int apply_patch(struct apply_state *state,
struct strbuf buf = STRBUF_INIT; /* owns the patch text */
struct patch *list = NULL, **listp = &list;
int skipped_patch = 0;
+ int res = 0;
state->patch_input_file = filename;
read_patch_file(&buf, fd);
@@ -4445,8 +4454,10 @@ static int apply_patch(struct apply_state *state,
offset += nr;
}
- if (!list && !skipped_patch)
- die(_("unrecognized input"));
+ if (!list && !skipped_patch) {
+ res = error(_("unrecognized input"));
+ goto end;
+ }
if (state->whitespace_error && (state->ws_error_action ==
die_on_ws_error))
state->apply = 0;
@@ -4455,21 +4466,22 @@ static int apply_patch(struct apply_state *state,
if (state->update_index && state->newfd < 0)
state->newfd = hold_locked_index(state->lock_file, 1);
- if (state->check_index) {
- if (read_cache() < 0)
- die(_("unable to read index file"));
+ if (state->check_index && read_cache() < 0) {
+ res = error(_("unable to read index file"));
+ goto end;
}
if ((state->check || state->apply) &&
check_patch_list(state, list) < 0 &&
- !state->apply_with_reject)
- exit(1);
+ !state->apply_with_reject) {
+ res = -1;
+ goto end;
+ }
if (state->apply && write_out_results(state, list)) {
- if (state->apply_with_reject)
- exit(1);
/* with --3way, we still need to write the index out */
- return 1;
+ res = state->apply_with_reject ? -1 : 1;
+ goto end;
}
if (state->fake_ancestor)
@@ -4484,10 +4496,11 @@ static int apply_patch(struct apply_state *state,
if (state->summary)
summary_patch_list(list);
+end:
free_patch_list(list);
strbuf_release(&buf);
string_list_clear(&state->fn_table, 0);
- return 0;
+ return res;
}
static void git_apply_config(void)
@@ -4625,6 +4638,7 @@ static int apply_all_patches(struct apply_state *state,
int options)
{
int i;
+ int res;
int errs = 0;
int read_stdin = 1;
@@ -4633,7 +4647,10 @@ static int apply_all_patches(struct apply_state *state,
int fd;
if (!strcmp(arg, "-")) {
- errs |= apply_patch(state, 0, "", options);
+ res = apply_patch(state, 0, "", options);
+ if (res < 0)
+ exit(1);
+ errs |= res;
read_stdin = 0;
continue;
} else if (0 < state->prefix_length)
@@ -4646,12 +4663,19 @@ static int apply_all_patches(struct apply_state *state,
die_errno(_("can't open patch '%s'"), arg);
read_stdin = 0;
set_default_whitespace_mode(state);
- errs |= apply_patch(state, fd, arg, options);
+ res = apply_patch(state, fd, arg, options);
+ if (res < 0)
+ exit(1);
+ errs |= res;
close(fd);
}
set_default_whitespace_mode(state);
- if (read_stdin)
- errs |= apply_patch(state, 0, "", options);
+ if (read_stdin) {
+ res = apply_patch(state, 0, "", options);
+ if (res < 0)
+ exit(1);
+ errs |= res;
+ }
if (state->whitespace_error) {
if (state->squelch_whitespace_errors &&
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: se
[PATCH v7 10/40] apply: make init_apply_state() return -1 instead of exit()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", init_apply_state() should return -1 instead of
calling exit().
Signed-off-by: Christian Couder
---
apply.c | 11 ++-
apply.h | 6 +++---
builtin/apply.c | 3 ++-
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/apply.c b/apply.c
index 1f31bb4..c5a9ee2 100644
--- a/apply.c
+++ b/apply.c
@@ -55,9 +55,9 @@ int parse_ignorewhitespace_option(struct apply_state *state,
return error(_("unrecognized whitespace ignore option '%s'"), option);
}
-void init_apply_state(struct apply_state *state,
- const char *prefix,
- struct lock_file *lock_file)
+int init_apply_state(struct apply_state *state,
+const char *prefix,
+struct lock_file *lock_file)
{
memset(state, 0, sizeof(*state));
state->prefix = prefix;
@@ -76,9 +76,10 @@ void init_apply_state(struct apply_state *state,
git_apply_config();
if (apply_default_whitespace && parse_whitespace_option(state,
apply_default_whitespace))
- exit(1);
+ return -1;
if (apply_default_ignorewhitespace &&
parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
- exit(1);
+ return -1;
+ return 0;
}
void clear_apply_state(struct apply_state *state)
diff --git a/apply.h b/apply.h
index 77502be..7d3a03b 100644
--- a/apply.h
+++ b/apply.h
@@ -102,9 +102,9 @@ extern int parse_whitespace_option(struct apply_state
*state,
extern int parse_ignorewhitespace_option(struct apply_state *state,
const char *option);
-extern void init_apply_state(struct apply_state *state,
-const char *prefix,
-struct lock_file *lock_file);
+extern int init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file);
extern void clear_apply_state(struct apply_state *state);
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index bc15545..2ae1243 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4728,7 +4728,8 @@ int cmd_apply(int argc, const char **argv, const char
*prefix)
OPT_END()
};
- init_apply_state(&state, prefix, &lock_file);
+ if (init_apply_state(&state, prefix, &lock_file))
+ exit(1);
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 16/40] builtin/apply: make gitdiff_*() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", gitdiff_*() functions should return -1 instead
of calling die().
A previous patch made it possible for gitdiff_*() functions to
return -1 in case of error. Let's take advantage of that to
make gitdiff_verify_name() return -1 on error, and to have
gitdiff_oldname() and gitdiff_newname() directly return
what gitdiff_verify_name() returns.
Helped-by: Nguyễn Thái Ngọc Duy
Signed-off-by: Christian Couder
---
builtin/apply.c | 40 +---
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 1142514..b506369 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -827,54 +827,56 @@ static int gitdiff_hdrend(struct apply_state *state,
#define DIFF_OLD_NAME 0
#define DIFF_NEW_NAME 1
-static void gitdiff_verify_name(struct apply_state *state,
- const char *line,
- int isnull,
- char **name,
- int side)
+static int gitdiff_verify_name(struct apply_state *state,
+ const char *line,
+ int isnull,
+ char **name,
+ int side)
{
if (!*name && !isnull) {
*name = find_name(state, line, NULL, state->p_value, TERM_TAB);
- return;
+ return 0;
}
if (*name) {
int len = strlen(*name);
char *another;
if (isnull)
- die(_("git apply: bad git-diff - expected /dev/null,
got %s on line %d"),
- *name, state->linenr);
+ return error(_("git apply: bad git-diff - expected
/dev/null, got %s on line %d"),
+*name, state->linenr);
another = find_name(state, line, NULL, state->p_value,
TERM_TAB);
- if (!another || memcmp(another, *name, len + 1))
- die((side == DIFF_NEW_NAME) ?
+ if (!another || memcmp(another, *name, len + 1)) {
+ free(another);
+ return error((side == DIFF_NEW_NAME) ?
_("git apply: bad git-diff - inconsistent new
filename on line %d") :
_("git apply: bad git-diff - inconsistent old
filename on line %d"), state->linenr);
+ }
free(another);
} else {
/* expect "/dev/null" */
if (memcmp("/dev/null", line, 9) || line[9] != '\n')
- die(_("git apply: bad git-diff - expected /dev/null on
line %d"), state->linenr);
+ return error(_("git apply: bad git-diff - expected
/dev/null on line %d"), state->linenr);
}
+
+ return 0;
}
static int gitdiff_oldname(struct apply_state *state,
const char *line,
struct patch *patch)
{
- gitdiff_verify_name(state, line,
- patch->is_new, &patch->old_name,
- DIFF_OLD_NAME);
- return 0;
+ return gitdiff_verify_name(state, line,
+ patch->is_new, &patch->old_name,
+ DIFF_OLD_NAME);
}
static int gitdiff_newname(struct apply_state *state,
const char *line,
struct patch *patch)
{
- gitdiff_verify_name(state, line,
- patch->is_delete, &patch->new_name,
- DIFF_NEW_NAME);
- return 0;
+ return gitdiff_verify_name(state, line,
+ patch->is_delete, &patch->new_name,
+ DIFF_NEW_NAME);
}
static int gitdiff_oldmode(struct apply_state *state,
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 27/40] builtin/apply: rename option parsing functions
As these functions are going to be part of the libified
apply api, let's give them a name that is more specific
to the apply api.
Signed-off-by: Christian Couder
---
builtin/apply.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index f51dc60..c84add0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4566,16 +4566,16 @@ end:
return res;
}
-static int option_parse_exclude(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
}
-static int option_parse_include(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4583,9 +4583,9 @@ static int option_parse_include(const struct option *opt,
return 0;
}
-static int option_parse_p(const struct option *opt,
- const char *arg,
- int unset)
+static int apply_option_parse_p(const struct option *opt,
+ const char *arg,
+ int unset)
{
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4593,8 +4593,8 @@ static int option_parse_p(const struct option *opt,
return 0;
}
-static int option_parse_space_change(const struct option *opt,
-const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
if (unset)
@@ -4604,8 +4604,8 @@ static int option_parse_space_change(const struct option
*opt,
return 0;
}
-static int option_parse_whitespace(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+const char *arg, int unset)
{
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4614,8 +4614,8 @@ static int option_parse_whitespace(const struct option
*opt,
return 0;
}
-static int option_parse_directory(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
strbuf_reset(&state->root);
@@ -4729,13 +4729,13 @@ int cmd_apply(int argc, const char **argv, const char
*prefix)
struct option builtin_apply_options[] = {
{ OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
N_("don't apply changes matching the given path"),
- 0, option_parse_exclude },
+ 0, apply_option_parse_exclude },
{ OPTION_CALLBACK, 0, "include", &state, N_("path"),
N_("apply changes matching the given path"),
- 0, option_parse_include },
+ 0, apply_option_parse_include },
{ OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
N_("remove leading slashes from traditional diff
paths"),
- 0, option_parse_p },
+ 0, apply_option_parse_p },
OPT_BOOL(0, "no-add", &state.no_add,
N_("ignore additions made by the patch")),
OPT_BOOL(0, "stat", &state.diffstat,
@@ -4767,13 +4767,13 @@ int cmd_apply(int argc, const char **argv, const char
*prefix)
N_("ensure at least lines of context
match")),
{ OPTION_CALLBACK, 0, "whitespace", &state, N_("action"),
N_("detect new or modified lines that have whitespace
errors"),
- 0, option_parse_whitespace },
+ 0, apply_option_parse_whitespace },
{ OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_space_change },
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
{ OPTION_CALLBACK, 0, "ignore-whitespace", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_spa
[PATCH v7 34/40] apply: make 'be_silent' incompatible with 'apply_verbosely'
It should be an error to have both be_silent and apply_verbosely set,
so let's check that in check_apply_state().
And by the way let's not automatically set apply_verbosely when
be_silent is set.
Signed-off-by: Christian Couder
---
apply.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index dbb2515..dd9b301 100644
--- a/apply.c
+++ b/apply.c
@@ -122,8 +122,11 @@ int check_apply_state(struct apply_state *state, int
force_apply)
return error(_("--3way outside a repository"));
state->check_index = 1;
}
- if (state->apply_with_reject)
- state->apply = state->apply_verbosely = 1;
+ if (state->apply_with_reject) {
+ state->apply = 1;
+ if (!state->be_silent)
+ state->apply_verbosely = 1;
+ }
if (!force_apply && (state->diffstat || state->numstat ||
state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
@@ -135,6 +138,8 @@ int check_apply_state(struct apply_state *state, int
force_apply)
}
if (state->check_index)
state->unsafe_paths = 0;
+ if (state->be_silent && state->apply_verbosely)
+ return error(_("incompatible internal 'be_silent' and
'apply_verbosely' flags"));
if (!state->lock_file)
return error("BUG: state->lock_file should not be NULL");
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 08/40] builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", parse_ignorewhitespace_option() should return
-1 instead of calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index e56e754..e2f970d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -57,20 +57,20 @@ static int parse_whitespace_option(struct apply_state
*state, const char *option
return error(_("unrecognized whitespace option '%s'"), option);
}
-static void parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option)
{
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
!strcmp(option, "none")) {
state->ws_ignore_action = ignore_ws_none;
- return;
+ return 0;
}
if (!strcmp(option, "change")) {
state->ws_ignore_action = ignore_ws_change;
- return;
+ return 0;
}
- die(_("unrecognized whitespace ignore option '%s'"), option);
+ return error(_("unrecognized whitespace ignore option '%s'"), option);
}
static void set_default_whitespace_mode(struct apply_state *state)
@@ -4616,8 +4616,8 @@ static void init_apply_state(struct apply_state *state,
git_apply_config();
if (apply_default_whitespace && parse_whitespace_option(state,
apply_default_whitespace))
exit(1);
- if (apply_default_ignorewhitespace)
- parse_ignorewhitespace_option(state,
apply_default_ignorewhitespace);
+ if (apply_default_ignorewhitespace &&
parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ exit(1);
}
static void clear_apply_state(struct apply_state *state)
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 12/40] builtin/apply: move check_apply_state() to apply.c
To libify `git apply` functionality we must make check_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into "apply.c".
Signed-off-by: Christian Couder
---
apply.c | 32
apply.h | 1 +
builtin/apply.c | 32
3 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/apply.c b/apply.c
index c5a9ee2..84dae3d 100644
--- a/apply.c
+++ b/apply.c
@@ -90,3 +90,35 @@ void clear_apply_state(struct apply_state *state)
/* &state->fn_table is cleared at the end of apply_patch() */
}
+
+int check_apply_state(struct apply_state *state, int force_apply)
+{
+ int is_not_gitdir = !startup_info->have_repository;
+
+ if (state->apply_with_reject && state->threeway)
+ return error("--reject and --3way cannot be used together.");
+ if (state->cached && state->threeway)
+ return error("--cached and --3way cannot be used together.");
+ if (state->threeway) {
+ if (is_not_gitdir)
+ return error(_("--3way outside a repository"));
+ state->check_index = 1;
+ }
+ if (state->apply_with_reject)
+ state->apply = state->apply_verbosely = 1;
+ if (!force_apply && (state->diffstat || state->numstat ||
state->summary || state->check || state->fake_ancestor))
+ state->apply = 0;
+ if (state->check_index && is_not_gitdir)
+ return error(_("--index outside a repository"));
+ if (state->cached) {
+ if (is_not_gitdir)
+ return error(_("--cached outside a repository"));
+ state->check_index = 1;
+ }
+ if (state->check_index)
+ state->unsafe_paths = 0;
+ if (!state->lock_file)
+ return error("BUG: state->lock_file should not be NULL");
+
+ return 0;
+}
diff --git a/apply.h b/apply.h
index 7d3a03b..1f2277e 100644
--- a/apply.h
+++ b/apply.h
@@ -106,5 +106,6 @@ extern int init_apply_state(struct apply_state *state,
const char *prefix,
struct lock_file *lock_file);
extern void clear_apply_state(struct apply_state *state);
+extern int check_apply_state(struct apply_state *state, int force_apply);
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index d60ffce..a27fdd3 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4541,38 +4541,6 @@ static int option_parse_directory(const struct option
*opt,
return 0;
}
-static int check_apply_state(struct apply_state *state, int force_apply)
-{
- int is_not_gitdir = !startup_info->have_repository;
-
- if (state->apply_with_reject && state->threeway)
- return error("--reject and --3way cannot be used together.");
- if (state->cached && state->threeway)
- return error("--cached and --3way cannot be used together.");
- if (state->threeway) {
- if (is_not_gitdir)
- return error(_("--3way outside a repository"));
- state->check_index = 1;
- }
- if (state->apply_with_reject)
- state->apply = state->apply_verbosely = 1;
- if (!force_apply && (state->diffstat || state->numstat ||
state->summary || state->check || state->fake_ancestor))
- state->apply = 0;
- if (state->check_index && is_not_gitdir)
- return error(_("--index outside a repository"));
- if (state->cached) {
- if (is_not_gitdir)
- return error(_("--cached outside a repository"));
- state->check_index = 1;
- }
- if (state->check_index)
- state->unsafe_paths = 0;
- if (!state->lock_file)
- return error("BUG: state->lock_file should not be NULL");
-
- return 0;
-}
-
static int apply_all_patches(struct apply_state *state,
int argc,
const char **argv,
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 37/40] usage: add get_error_routine() and get_warn_routine()
Let's make it possible to get the current error_routine and warn_routine,
so that we can store them before using set_error_routine() or
set_warn_routine() to use new ones.
This way we will be able put back the original routines, when we are done
with using new ones.
Signed-off-by: Christian Couder
---
git-compat-util.h | 2 ++
usage.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index f78706a..92af27d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,7 +440,9 @@ static inline int const_error(void)
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err,
va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list
params));
+extern void (*get_error_routine(void))(const char *err, va_list params);
extern void set_warn_routine(void (*routine)(const char *warn, va_list
params));
+extern void (*get_warn_routine(void))(const char *warn, va_list params);
extern void set_die_is_recursing_routine(int (*routine)(void));
extern void set_error_handle(FILE *);
diff --git a/usage.c b/usage.c
index 67e5526..2fd3045 100644
--- a/usage.c
+++ b/usage.c
@@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err,
va_list params))
error_routine = routine;
}
+void (*get_error_routine(void))(const char *err, va_list params)
+{
+ return error_routine;
+}
+
void set_warn_routine(void (*routine)(const char *warn, va_list params))
{
warn_routine = routine;
}
+void (*get_warn_routine(void))(const char *warn, va_list params)
+{
+ return warn_routine;
+}
+
void set_die_is_recursing_routine(int (*routine)(void))
{
die_is_recursing = routine;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 04/40] builtin/apply: make find_header() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, find_header() should return -1 instead of calling
die().
Unfortunately find_header() already returns -1 when no header is found,
so let's make it return -2 instead in this case.
Signed-off-by: Christian Couder
---
builtin/apply.c | 33 ++---
t/t4254-am-corrupt.sh | 2 +-
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 2ff8450..630c01c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1419,6 +1419,14 @@ static int parse_fragment_header(const char *line, int
len, struct fragment *fra
return offset;
}
+/*
+ * Find file diff header
+ *
+ * Returns:
+ * -1 in case of error
+ * -2 if no header was found
+ * the size of the header in bytes (called "offset") otherwise
+ */
static int find_header(struct apply_state *state,
const char *line,
unsigned long size,
@@ -1452,8 +1460,8 @@ static int find_header(struct apply_state *state,
struct fragment dummy;
if (parse_fragment_header(line, len, &dummy) < 0)
continue;
- die(_("patch fragment without header at line %d: %.*s"),
- state->linenr, (int)len-1, line);
+ return error(_("patch fragment without header at line
%d: %.*s"),
+state->linenr, (int)len-1, line);
}
if (size < len + 6)
@@ -1469,18 +1477,18 @@ static int find_header(struct apply_state *state,
continue;
if (!patch->old_name && !patch->new_name) {
if (!patch->def_name)
- die(Q_("git diff header lacks filename
information when removing "
- "%d leading pathname component
(line %d)",
- "git diff header lacks filename
information when removing "
- "%d leading pathname components
(line %d)",
- state->p_value),
- state->p_value, state->linenr);
+ return error(Q_("git diff header lacks
filename information when removing "
+ "%d leading pathname
component (line %d)",
+ "git diff header lacks
filename information when removing "
+ "%d leading pathname
components (line %d)",
+ state->p_value),
+state->p_value,
state->linenr);
patch->old_name = xstrdup(patch->def_name);
patch->new_name = xstrdup(patch->def_name);
}
if (!patch->is_delete && !patch->new_name)
- die("git diff header lacks filename information
"
- "(line %d)", state->linenr);
+ return error("git diff header lacks filename
information "
+"(line %d)", state->linenr);
patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
@@ -1505,7 +1513,7 @@ static int find_header(struct apply_state *state,
state->linenr += 2;
return offset;
}
- return -1;
+ return -2;
}
static void record_ws_error(struct apply_state *state,
@@ -1996,6 +2004,9 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, &hdrsize, patch);
+ if (offset == -1)
+ exit(1);
+
if (offset < 0)
return offset;
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 85716dd..9bd7dd2 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -29,7 +29,7 @@ test_expect_success 'try to apply corrupted patch' '
'
test_expect_success 'compare diagnostic; ensure file is still here' '
- echo "fatal: git diff header lacks filename information (line 4)"
>expected &&
+ echo "error: git diff header lacks filename information (line 4)"
>expected &&
test_path_is_file f &&
test_cmp expected actual
'
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a me
[PATCH v7 40/40] apply: use error_errno() where possible
To avoid possible mistakes and to uniformly show the errno
related messages, let's use error_errno() where possible.
Signed-off-by: Christian Couder
---
apply.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/apply.c b/apply.c
index ef49709..98a 100644
--- a/apply.c
+++ b/apply.c
@@ -3505,7 +3505,7 @@ static int load_current(struct apply_state *state,
ce = active_cache[pos];
if (lstat(name, &st)) {
if (errno != ENOENT)
- return error(_("%s: %s"), name, strerror(errno));
+ return error_errno("%s", name);
if (checkout_target(&the_index, ce, &st))
return -1;
}
@@ -3664,7 +3664,7 @@ static int check_preimage(struct apply_state *state,
} else if (!state->cached) {
stat_ret = lstat(old_name, st);
if (stat_ret && errno != ENOENT)
- return error(_("%s: %s"), old_name, strerror(errno));
+ return error_errno("%s", old_name);
}
if (state->check_index && !previous) {
@@ -3686,7 +3686,7 @@ static int check_preimage(struct apply_state *state,
} else if (stat_ret < 0) {
if (patch->is_new < 0)
goto is_new;
- return error(_("%s: %s"), old_name, strerror(errno));
+ return error_errno("%s", old_name);
}
if (!state->cached && !previous)
@@ -3745,7 +3745,7 @@ static int check_to_create(struct apply_state *state,
return EXISTS_IN_WORKTREE;
} else if ((errno != ENOENT) && (errno != ENOTDIR)) {
- return error("%s: %s", new_name, strerror(errno));
+ return error_errno("%s", new_name);
}
return 0;
}
@@ -4260,9 +4260,9 @@ static int add_index_file(struct apply_state *state,
if (!state->cached) {
if (lstat(path, &st) < 0) {
free(ce);
- return error(_("unable to stat newly "
- "created file '%s': %s"),
-path, strerror(errno));
+ return error_errno(_("unable to stat newly "
+"created file '%s'"),
+ path);
}
fill_stat_cache_info(ce, &st);
}
@@ -4316,7 +4316,7 @@ static int try_create_file(const char *path, unsigned int
mode, const char *buf,
strbuf_release(&nbuf);
if (close(fd) < 0 && !res)
- return error(_("closing file '%s': %s"), path, strerror(errno));
+ return error_errno(_("closing file '%s'"), path);
return res ? -1 : 0;
}
@@ -4386,8 +4386,8 @@ static int create_one_file(struct apply_state *state,
++nr;
}
}
- return error(_("unable to write file '%s' mode %o: %s"),
-path, mode, strerror(errno));
+ return error_errno(_("unable to write file '%s' mode %o"),
+ path, mode);
}
static int add_conflicted_stages_file(struct apply_state *state,
@@ -4514,7 +4514,7 @@ static int write_out_one_reject(struct apply_state
*state, struct patch *patch)
rej = fopen(namebuf, "w");
if (!rej)
- return error(_("cannot open %s: %s"), namebuf, strerror(errno));
+ return error_errno(_("cannot open %s"), namebuf);
/* Normal git tools never deal with .rej, so do not pretend
* this is a git patch by saying --git or giving extended
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 32/40] write_or_die: use warning() instead of fprintf(stderr, ...)
In write_or_whine_pipe() and write_or_whine() when write_in_full()
returns an error, let's print the errno related error message using
warning() instead of fprintf(stderr, ...).
This makes it possible to change the way it is handled by changing
the current warn routine in usage.c.
Signed-off-by: Christian Couder
---
write_or_die.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/write_or_die.c b/write_or_die.c
index 49e80aa..c29f677 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t
count, const char *msg)
{
if (write_in_full(fd, buf, count) < 0) {
check_pipe(errno);
- fprintf(stderr, "%s: write error (%s)\n",
- msg, strerror(errno));
+ warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
@@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t
count, const char *msg)
int write_or_whine(int fd, const void *buf, size_t count, const char *msg)
{
if (write_in_full(fd, buf, count) < 0) {
- fprintf(stderr, "%s: write error (%s)\n",
- msg, strerror(errno));
+ warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 39/40] builtin/am: use apply api in run_apply()
This replaces run_apply() implementation with a new one that
uses the apply api that has been previously prepared in
apply.c and apply.h.
This shoud improve performance a lot in certain cases.
As the previous implementation was creating a new `git apply`
process to apply each patch, it could be slow on systems like
Windows where it is costly to create new processes.
Also the new `git apply` process had to read the index from
disk, and when the process was done the calling process
discarded its own index and read back from disk the new
index that had been created by the `git apply` process.
This could be very inefficient with big repositories that
have big index files, especially when the system decided
that it was a good idea to run the `git apply` processes on
a different processor core.
Also eliminating index reads enables further performance
improvements by using:
`git update-index --split-index`
For example here is a benchmark of a multi hundred commit
rebase on the Linux kernel on a Debian laptop with SSD:
command: git rebase --onto 1993b17 52bef0c 29dde7c
Vanilla "next" without split index:1m54.953s
Vanilla "next" with split index: 1m22.476s
This series on top of "next" without split index: 1m12.034s
This series on top of "next" with split index: 0m15.678s
(using branch "next" from mid April 2016.)
Benchmarked-by: Ævar Arnfjörð Bjarmason
Signed-off-by: Christian Couder
---
builtin/am.c | 91
1 file changed, 73 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..8647298 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -28,6 +28,7 @@
#include "rerere.h"
#include "prompt.h"
#include "mailinfo.h"
+#include "apply.h"
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1521,39 +1522,93 @@ static int parse_mail_rebase(struct am_state *state,
const char *mail)
*/
static int run_apply(const struct am_state *state, const char *index_file)
{
- struct child_process cp = CHILD_PROCESS_INIT;
+ struct argv_array apply_paths = ARGV_ARRAY_INIT;
+ struct argv_array apply_opts = ARGV_ARRAY_INIT;
+ struct apply_state apply_state;
+ int res, opts_left;
+ char *save_index_file;
+ static struct lock_file lock_file;
+
+ struct option am_apply_options[] = {
+ { OPTION_CALLBACK, 0, "whitespace", &apply_state, N_("action"),
+ N_("detect new or modified lines that have whitespace
errors"),
+ 0, apply_option_parse_whitespace },
+ { OPTION_CALLBACK, 0, "ignore-space-change", &apply_state, NULL,
+ N_("ignore changes in whitespace when finding context"),
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
+ { OPTION_CALLBACK, 0, "ignore-whitespace", &apply_state, NULL,
+ N_("ignore changes in whitespace when finding context"),
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
+ { OPTION_CALLBACK, 0, "directory", &apply_state, N_("root"),
+ N_("prepend to all filenames"),
+ 0, apply_option_parse_directory },
+ { OPTION_CALLBACK, 0, "exclude", &apply_state, N_("path"),
+ N_("don't apply changes matching the given path"),
+ 0, apply_option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", &apply_state, N_("path"),
+ N_("apply changes matching the given path"),
+ 0, apply_option_parse_include },
+ OPT_INTEGER('C', NULL, &apply_state.p_context,
+ N_("ensure at least lines of context
match")),
+ { OPTION_CALLBACK, 'p', NULL, &apply_state, N_("num"),
+ N_("remove leading slashes from traditional diff
paths"),
+ 0, apply_option_parse_p },
+ OPT_BOOL(0, "reject", &apply_state.apply_with_reject,
+ N_("leave the rejected hunks in corresponding *.rej
files")),
+ OPT_END()
+ };
- cp.git_cmd = 1;
+ if (index_file) {
+ save_index_file = get_index_file();
+ set_index_file((char *)index_file);
+ }
+
+ if (init_apply_state(&apply_state, NULL, &lock_file))
+ die("init_apply_state() failed");
+
+ argv_array_push(&apply_opts, "apply");
+ argv_array_pushv(&apply_opts, state->git_apply_opts.argv);
+
+ opts_left = parse_options(apply_opts.argc, apply_opts.argv,
+ NULL, am_apply_options, NULL, 0);
+
+ if (opts_left != 0)
+ die("unknown option passed thru to git apply");
if (index_file)
- argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s",
index_file);
+
[PATCH v7 26/40] builtin/apply: make create_one_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_one_file() should return -1 instead of
calling exit().
Signed-off-by: Christian Couder
---
builtin/apply.c | 36 +---
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 37f0c2e..f51dc60 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4184,32 +4184,36 @@ static int try_create_file(const char *path, unsigned
int mode, const char *buf,
* We optimistically assume that the directories exist,
* which is true 99% of the time anyway. If they don't,
* we create them and try again.
+ *
+ * Returns:
+ * -1 on error
+ * 0 otherwise
*/
-static void create_one_file(struct apply_state *state,
- char *path,
- unsigned mode,
- const char *buf,
- unsigned long size)
+static int create_one_file(struct apply_state *state,
+ char *path,
+ unsigned mode,
+ const char *buf,
+ unsigned long size)
{
int res;
if (state->cached)
- return;
+ return 0;
res = try_create_file(path, mode, buf, size);
if (res < 0)
- exit(1);
+ return -1;
if (!res)
- return;
+ return 0;
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
- return;
+ return 0;
res = try_create_file(path, mode, buf, size);
if (res < 0)
- exit(1);
+ return -1;
if (!res)
- return;
+ return 0;
}
if (errno == EEXIST || errno == EACCES) {
@@ -4229,10 +4233,10 @@ static void create_one_file(struct apply_state *state,
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
res = try_create_file(newpath, mode, buf, size);
if (res < 0)
- exit(1);
+ return -1;
if (!res) {
if (!rename(newpath, path))
- return;
+ return 0;
unlink_or_warn(newpath);
break;
}
@@ -4241,7 +4245,8 @@ static void create_one_file(struct apply_state *state,
++nr;
}
}
- die_errno(_("unable to write file '%s' mode %o"), path, mode);
+ return error(_("unable to write file '%s' mode %o: %s"),
+path, mode, strerror(errno));
}
static int add_conflicted_stages_file(struct apply_state *state,
@@ -4286,7 +4291,8 @@ static int create_file(struct apply_state *state, struct
patch *patch)
if (!mode)
mode = S_IFREG | 0644;
- create_one_file(state, path, mode, buf, size);
+ if (create_one_file(state, path, mode, buf, size))
+ return -1;
if (patch->conflicted_threeway)
return add_conflicted_stages_file(state, patch);
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 23/40] builtin/apply: make write_out_one_result() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_one_result() should just return what
remove_file() and create_file() are returning instead of calling
exit().
Signed-off-by: Christian Couder
---
builtin/apply.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 76d473c..291e24e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4276,36 +4276,29 @@ static int create_file(struct apply_state *state,
struct patch *patch)
}
/* phase zero is to remove, phase one is to create */
-static void write_out_one_result(struct apply_state *state,
-struct patch *patch,
-int phase)
+static int write_out_one_result(struct apply_state *state,
+ struct patch *patch,
+ int phase)
{
if (patch->is_delete > 0) {
- if (phase == 0) {
- if (remove_file(state, patch, 1))
- exit(1);
- }
- return;
+ if (phase == 0)
+ return remove_file(state, patch, 1);
+ return 0;
}
if (patch->is_new > 0 || patch->is_copy) {
- if (phase == 1) {
- if (create_file(state, patch))
- exit(1);
- }
- return;
+ if (phase == 1)
+ return create_file(state, patch);
+ return 0;
}
/*
* Rename or modification boils down to the same
* thing: remove the old, write the new
*/
- if (phase == 0) {
- if (remove_file(state, patch, patch->is_rename))
- exit(1);
- }
- if (phase == 1) {
- if (create_file(state, patch))
- exit(1);
- }
+ if (phase == 0)
+ return remove_file(state, patch, patch->is_rename);
+ if (phase == 1)
+ return create_file(state, patch);
+ return 0;
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
@@ -4392,7 +4385,8 @@ static int write_out_results(struct apply_state *state,
struct patch *list)
if (l->rejected)
errs = 1;
else {
- write_out_one_result(state, l, phase);
+ if (write_out_one_result(state, l, phase))
+ exit(1);
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/40] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_whitespace_option() should return -1 instead
of calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 2671586..e56e754 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -27,34 +27,34 @@ static const char * const apply_usage[] = {
NULL
};
-static void parse_whitespace_option(struct apply_state *state, const char
*option)
+static int parse_whitespace_option(struct apply_state *state, const char
*option)
{
if (!option) {
state->ws_error_action = warn_on_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "warn")) {
state->ws_error_action = warn_on_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "nowarn")) {
state->ws_error_action = nowarn_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "error")) {
state->ws_error_action = die_on_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "error-all")) {
state->ws_error_action = die_on_ws_error;
state->squelch_whitespace_errors = 0;
- return;
+ return 0;
}
if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
state->ws_error_action = correct_ws_error;
- return;
+ return 0;
}
- die(_("unrecognized whitespace option '%s'"), option);
+ return error(_("unrecognized whitespace option '%s'"), option);
}
static void parse_ignorewhitespace_option(struct apply_state *state,
@@ -4579,7 +4579,8 @@ static int option_parse_whitespace(const struct option
*opt,
{
struct apply_state *state = opt->value;
state->whitespace_option = arg;
- parse_whitespace_option(state, arg);
+ if (parse_whitespace_option(state, arg))
+ exit(1);
return 0;
}
@@ -4613,8 +4614,8 @@ static void init_apply_state(struct apply_state *state,
strbuf_init(&state->root, 0);
git_apply_config();
- if (apply_default_whitespace)
- parse_whitespace_option(state, apply_default_whitespace);
+ if (apply_default_whitespace && parse_whitespace_option(state,
apply_default_whitespace))
+ exit(1);
if (apply_default_ignorewhitespace)
parse_ignorewhitespace_option(state,
apply_default_ignorewhitespace);
}
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 11/40] builtin/apply: make check_apply_state() return -1 instead of die()ing
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", check_apply_state() should return -1 instead of
calling die().
Signed-off-by: Christian Couder
---
builtin/apply.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 2ae1243..d60ffce 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4541,17 +4541,17 @@ static int option_parse_directory(const struct option
*opt,
return 0;
}
-static void check_apply_state(struct apply_state *state, int force_apply)
+static int check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
if (state->apply_with_reject && state->threeway)
- die("--reject and --3way cannot be used together.");
+ return error("--reject and --3way cannot be used together.");
if (state->cached && state->threeway)
- die("--cached and --3way cannot be used together.");
+ return error("--cached and --3way cannot be used together.");
if (state->threeway) {
if (is_not_gitdir)
- die(_("--3way outside a repository"));
+ return error(_("--3way outside a repository"));
state->check_index = 1;
}
if (state->apply_with_reject)
@@ -4559,16 +4559,18 @@ static void check_apply_state(struct apply_state
*state, int force_apply)
if (!force_apply && (state->diffstat || state->numstat ||
state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
- die(_("--index outside a repository"));
+ return error(_("--index outside a repository"));
if (state->cached) {
if (is_not_gitdir)
- die(_("--cached outside a repository"));
+ return error(_("--cached outside a repository"));
state->check_index = 1;
}
if (state->check_index)
state->unsafe_paths = 0;
if (!state->lock_file)
- die("BUG: state->lock_file should not be NULL");
+ return error("BUG: state->lock_file should not be NULL");
+
+ return 0;
}
static int apply_all_patches(struct apply_state *state,
@@ -4734,7 +4736,8 @@ int cmd_apply(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
- check_apply_state(&state, force_apply);
+ if (check_apply_state(&state, force_apply))
+ exit(1);
ret = apply_all_patches(&state, argc, argv, options);
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 13/40] builtin/apply: make apply_all_patches() return -1 on error
To finish libifying the apply functionality, apply_all_patches() should not
die() or exit() in case of error, but return -1.
While doing that we must take care that file descriptors are properly closed
and, if needed, reset a sensible value.
Also, according to the lockfile API, when finished with a lockfile, one
should either commit it or roll it back.
This is even more important now that the same lockfile can be passed
to init_apply_state() many times to be reused by series of calls to
the apply lib functions.
Helped-by: Nguyễn Thái Ngọc Duy
Helped-by: Johannes Schindelin
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 40 +++-
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a27fdd3..c27be35 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4558,7 +4558,7 @@ static int apply_all_patches(struct apply_state *state,
if (!strcmp(arg, "-")) {
res = apply_patch(state, 0, "", options);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
read_stdin = 0;
continue;
@@ -4568,21 +4568,23 @@ static int apply_all_patches(struct apply_state *state,
arg);
fd = open(arg, O_RDONLY);
- if (fd < 0)
- die_errno(_("can't open patch '%s'"), arg);
+ if (fd < 0) {
+ error(_("can't open patch '%s': %s"), arg,
strerror(errno));
+ goto rollback_end;
+ }
read_stdin = 0;
set_default_whitespace_mode(state);
res = apply_patch(state, fd, arg, options);
+ close(fd);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
- close(fd);
}
set_default_whitespace_mode(state);
if (read_stdin) {
res = apply_patch(state, 0, "", options);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
}
@@ -4596,11 +4598,13 @@ static int apply_all_patches(struct apply_state *state,
squelched),
squelched);
}
- if (state->ws_error_action == die_on_ws_error)
- die(Q_("%d line adds whitespace errors.",
- "%d lines add whitespace errors.",
- state->whitespace_error),
- state->whitespace_error);
+ if (state->ws_error_action == die_on_ws_error) {
+ error(Q_("%d line adds whitespace errors.",
+"%d lines add whitespace errors.",
+state->whitespace_error),
+ state->whitespace_error);
+ goto rollback_end;
+ }
if (state->applied_after_fixing_ws && state->apply)
warning("%d line%s applied after"
" fixing whitespace errors.",
@@ -4614,12 +4618,22 @@ static int apply_all_patches(struct apply_state *state,
}
if (state->update_index) {
- if (write_locked_index(&the_index, state->lock_file,
COMMIT_LOCK))
- die(_("Unable to write new index file"));
+ res = write_locked_index(&the_index, state->lock_file,
COMMIT_LOCK);
+ if (res) {
+ error(_("Unable to write new index file"));
+ goto rollback_end;
+ }
state->newfd = -1;
}
return !!errs;
+
+rollback_end:
+ if (state->newfd >= 0) {
+ rollback_lock_file(state->lock_file);
+ state->newfd = -1;
+ }
+ return -1;
}
int cmd_apply(int argc, const char **argv, const char *prefix)
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 05/40] builtin/apply: make parse_chunk() return a negative integer on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing or exit()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_chunk() should return -1 instead of calling
die() or exit().
As parse_chunk() is called only by apply_patch() which already
returns -1 when an error happened, let's make apply_patch() return -1
when parse_chunk() returns -1.
If find_header() returns -2 because no patch header has been found, it
is ok for parse_chunk() to also return -2. If find_header() returns -1
because an error happened, it is ok for parse_chunk() to do the same.
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 630c01c..a160338 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1991,22 +1991,22 @@ static int use_patch(struct apply_state *state, struct
patch *p)
return !state->has_include;
}
-
/*
* Read the patch text in "buffer" that extends for "size" bytes; stop
* reading after seeing a single patch (i.e. changes to a single file).
* Create fragments (i.e. patch hunks) and hang them to the given patch.
- * Return the number of bytes consumed, so that the caller can call us
- * again for the next patch.
+ *
+ * Returns:
+ * -1 on error,
+ * -2 if no header was found,
+ * the number of bytes consumed otherwise,
+ * so that the caller can call us again for the next patch.
*/
static int parse_chunk(struct apply_state *state, char *buffer, unsigned long
size, struct patch *patch)
{
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, &hdrsize, patch);
- if (offset == -1)
- exit(1);
-
if (offset < 0)
return offset;
@@ -2067,7 +2067,7 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
*/
if ((state->apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
- die(_("patch with only garbage at line %d"),
state->linenr);
+ return error(_("patch with only garbage at line %d"),
state->linenr);
}
return offset + hdrsize + patchsize;
@@ -4449,6 +4449,10 @@ static int apply_patch(struct apply_state *state,
nr = parse_chunk(state, buf.buf + offset, buf.len - offset,
patch);
if (nr < 0) {
free_patch(patch);
+ if (nr == -1) {
+ res = -1;
+ goto end;
+ }
break;
}
if (state->apply_in_reverse)
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 17/40] builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", die_on_unsafe_path() should return -1 using
error() instead of calling die(), so while doing that let's change
its name to check_unsafe_path().
Signed-off-by: Christian Couder
---
builtin/apply.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index b506369..429fddd 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3697,7 +3697,7 @@ static int path_is_beyond_symlink(struct apply_state
*state, const char *name_)
return ret;
}
-static void die_on_unsafe_path(struct patch *patch)
+static int check_unsafe_path(struct patch *patch)
{
const char *old_name = NULL;
const char *new_name = NULL;
@@ -3709,9 +3709,10 @@ static void die_on_unsafe_path(struct patch *patch)
new_name = patch->new_name;
if (old_name && !verify_path(old_name))
- die(_("invalid path '%s'"), old_name);
+ return error(_("invalid path '%s'"), old_name);
if (new_name && !verify_path(new_name))
- die(_("invalid path '%s'"), new_name);
+ return error(_("invalid path '%s'"), new_name);
+ return 0;
}
/*
@@ -3801,8 +3802,8 @@ static int check_patch(struct apply_state *state, struct
patch *patch)
}
}
- if (!state->unsafe_paths)
- die_on_unsafe_path(patch);
+ if (!state->unsafe_paths && check_unsafe_path(patch))
+ return -1;
/*
* An attempt to read from or delete a path that is beyond a
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/40] apply: move 'struct apply_state' to apply.h
To libify `git apply` functionality we must make 'struct apply_state'
usable outside "builtin/apply.c".
Let's do that by creating a new "apply.h" and moving
'struct apply_state' there.
Signed-off-by: Christian Couder
---
apply.h | 100
builtin/apply.c | 98 +-
2 files changed, 101 insertions(+), 97 deletions(-)
create mode 100644 apply.h
diff --git a/apply.h b/apply.h
new file mode 100644
index 000..9a98eae
--- /dev/null
+++ b/apply.h
@@ -0,0 +1,100 @@
+#ifndef APPLY_H
+#define APPLY_H
+
+enum ws_error_action {
+ nowarn_ws_error,
+ warn_on_ws_error,
+ die_on_ws_error,
+ correct_ws_error
+};
+
+enum ws_ignore {
+ ignore_ws_none,
+ ignore_ws_change
+};
+
+/*
+ * We need to keep track of how symlinks in the preimage are
+ * manipulated by the patches. A patch to add a/b/c where a/b
+ * is a symlink should not be allowed to affect the directory
+ * the symlink points at, but if the same patch removes a/b,
+ * it is perfectly fine, as the patch removes a/b to make room
+ * to create a directory a/b so that a/b/c can be created.
+ *
+ * See also "struct string_list symlink_changes" in "struct
+ * apply_state".
+ */
+#define SYMLINK_GOES_AWAY 01
+#define SYMLINK_IN_RESULT 02
+
+struct apply_state {
+ const char *prefix;
+ int prefix_length;
+
+ /* These are lock_file related */
+ struct lock_file *lock_file;
+ int newfd;
+
+ /* These control what gets looked at and modified */
+ int apply; /* this is not a dry-run */
+ int cached; /* apply to the index only */
+ int check; /* preimage must match working tree, don't actually apply */
+ int check_index; /* preimage must match the indexed version */
+ int update_index; /* check_index && apply */
+
+ /* These control cosmetic aspect of the output */
+ int diffstat; /* just show a diffstat, and don't actually apply */
+ int numstat; /* just show a numeric diffstat, and don't actually apply
*/
+ int summary; /* just report creation, deletion, etc, and don't actually
apply */
+
+ /* These boolean parameters control how the apply is done */
+ int allow_overlap;
+ int apply_in_reverse;
+ int apply_with_reject;
+ int apply_verbosely;
+ int no_add;
+ int threeway;
+ int unidiff_zero;
+ int unsafe_paths;
+
+ /* Other non boolean parameters */
+ const char *fake_ancestor;
+ const char *patch_input_file;
+ int line_termination;
+ struct strbuf root;
+ int p_value;
+ int p_value_known;
+ unsigned int p_context;
+
+ /* Exclude and include path parameters */
+ struct string_list limit_by_name;
+ int has_include;
+
+ /* Various "current state" */
+ int linenr; /* current line number */
+ struct string_list symlink_changes; /* we have to track symlinks */
+
+ /*
+* For "diff-stat" like behaviour, we keep track of the biggest change
+* we've seen, and the longest filename. That allows us to do simple
+* scaling.
+*/
+ int max_change;
+ int max_len;
+
+ /*
+* Records filenames that have been touched, in order to handle
+* the case where more than one patches touch the same file.
+*/
+ struct string_list fn_table;
+
+ /* These control whitespace errors */
+ enum ws_error_action ws_error_action;
+ enum ws_ignore ws_ignore_action;
+ const char *whitespace_option;
+ int whitespace_error;
+ int squelch_whitespace_errors;
+ int applied_after_fixing_ws;
+};
+
+#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index ecb7f1b..3a0c53a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -20,103 +20,7 @@
#include "xdiff-interface.h"
#include "ll-merge.h"
#include "rerere.h"
-
-enum ws_error_action {
- nowarn_ws_error,
- warn_on_ws_error,
- die_on_ws_error,
- correct_ws_error
-};
-
-
-enum ws_ignore {
- ignore_ws_none,
- ignore_ws_change
-};
-
-/*
- * We need to keep track of how symlinks in the preimage are
- * manipulated by the patches. A patch to add a/b/c where a/b
- * is a symlink should not be allowed to affect the directory
- * the symlink points at, but if the same patch removes a/b,
- * it is perfectly fine, as the patch removes a/b to make room
- * to create a directory a/b so that a/b/c can be created.
- *
- * See also "struct string_list symlink_changes" in "struct
- * apply_state".
- */
-#define SYMLINK_GOES_AWAY 01
-#define SYMLINK_IN_RESULT 02
-
-struct apply_state {
- const char *prefix;
- int prefix_length;
-
- /* These are lock_file related */
- struct lock_file *lock_file;
- int newfd;
-
- /* These control what gets looked at and modified */
- int apply; /* this is not a dry-ru
[PATCH v7 20/40] builtin/apply: make add_conflicted_stages_file() return -1 on error
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_conflicted_stages_file() should return -1
instead of calling die().
Helped-by: Eric Sunshine
Signed-off-by: Christian Couder
---
builtin/apply.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 694c65b..0997384 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4213,7 +4213,7 @@ static void create_one_file(struct apply_state *state,
die_errno(_("unable to write file '%s' mode %o"), path, mode);
}
-static void add_conflicted_stages_file(struct apply_state *state,
+static int add_conflicted_stages_file(struct apply_state *state,
struct patch *patch)
{
int stage, namelen;
@@ -4221,7 +4221,7 @@ static void add_conflicted_stages_file(struct apply_state
*state,
struct cache_entry *ce;
if (!state->update_index)
- return;
+ return 0;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
mode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);
@@ -4236,9 +4236,14 @@ static void add_conflicted_stages_file(struct
apply_state *state,
ce->ce_flags = create_ce_flags(stage);
ce->ce_namelen = namelen;
hashcpy(ce->sha1, patch->threeway_stage[stage - 1].hash);
- if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
- die(_("unable to add cache entry for %s"),
patch->new_name);
+ if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+ free(ce);
+ return error(_("unable to add cache entry for %s"),
+patch->new_name);
+ }
}
+
+ return 0;
}
static void create_file(struct apply_state *state, struct patch *patch)
@@ -4252,9 +4257,10 @@ static void create_file(struct apply_state *state,
struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(state, path, mode, buf, size);
- if (patch->conflicted_threeway)
- add_conflicted_stages_file(state, patch);
- else
+ if (patch->conflicted_threeway) {
+ if (add_conflicted_stages_file(state, patch))
+ exit(1);
+ } else
add_index_file(state, path, mode, buf, size);
}
--
2.9.0.rc2.411.g3e2ca28
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/40] libify apply and use lib in am, part 2
Goal
This is a patch series about libifying `git apply` functionality, and
using this libified functionality in `git am`, so that no 'git apply'
process is spawn anymore. This makes `git am` significantly faster, so
`git rebase`, when it uses the am backend, is also significantly
faster.
Previous discussions and patches series
~~~
This has initially been discussed in the following thread:
http://thread.gmane.org/gmane.comp.version-control.git/287236/
Then the following patch series were sent:
RFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/
v1: http://thread.gmane.org/gmane.comp.version-control.git/292324/
v2: http://thread.gmane.org/gmane.comp.version-control.git/294248/
v3: http://thread.gmane.org/gmane.comp.version-control.git/295429/
v4: http://thread.gmane.org/gmane.comp.version-control.git/296350/
v5: http://thread.gmane.org/gmane.comp.version-control.git/296490/
v6: http://thread.gmane.org/gmane.comp.version-control.git/297024/
Highlevel view of the patches in the series
~~~
This new patch series is built on top of the above previous work.
More precisely, this is "part 2" of the full patch series which is
built on top of the "part 1" of the full patch series. And as the
"part 1" is now in "next", this "part 2" is built on top of "next".
- Patches 01/40 to 31/40 were in v1, v2 and v6.
They finish libifying the apply functionality that was in
builtin/apply.c and move it into apply.{c,h}.
The libified functionality is not yet used in `git am` in those
patches, as the patch that does that (previously 33/44 and now 39/40)
has been been moved towards the end of the series (see below).
The only other change in those patches is that the patch that was
making dup_devnull() non static (previously 31/44) has been
removed. It was reverted anyway towards the end of v6.
- Patches 32/40 to 38/40 were in v2 and v6.
They implement a way to make the libified apply silent by adding a new
'be_silent' flag into 'struct apply_state'. It is a new feature in the
libified apply functionality.
This could be in a separate series, but unfortunately using the
libified apply in "git am" unmasks the fact that "git am", since it
was a shell script, has been silencing the apply functionality by
redirecting file descriptors to /dev/null and it looks like this is
not acceptable in C.
The patch which reverted the patch that made dup_devnull() non static
has been removed too, as the patch it reverted has been removed.
The patch that made `git am` use the 'be_silent' new feature
(previously 41/44) has been squashed into a following patch (see
below).
The patch (previously 43/44) that added --silent to `git apply` has
been removed too as already planned in v6.
Other than that some commit messages have been improved.
- Patch 39/40 was in v1, v2 and v6.
This patch (which was 33/44 in v6) makes `git am` use the libified
functionality. It has been been moved towards the end of the series
following many reviewers' suggestion to avoid temporarily using
dup_devnull() to silence the libified apply functionality.
The patch that made `git am` use the 'be_silent' new feature
(previously 41/44) has been squashed into this patch.
Also a call to clear_apply_state() has been added into this patch to
avoid memory leaks.
- Patch 44/44 was new in v6.
It replaces some calls to error() with calls to error_errno().
One line has been changed to fix a warning.
General comments
Sorry if this patch series is still long. I can split it into two or
more series if it is prefered.
I will send a diff between this version and the previous one, as a
reply to this email.
The benefits are not just related to not creating new processes. When
`git am` launched a `git apply` process, this new process had to read
the index from disk. Then after the `git apply`process had terminated,
`git am` dropped its index and read the index from disk to get the
index that had been modified by the `git apply`process. This was
inefficient and also prevented the split-index mechanism to provide
many performance benefits.
Using this series as rebase material, Duy explains it like this:
> Without the series, the picture is not so surprising. We run git-apply
> 80+ times, each consists of this sequence
>
> read index
> write index (cache tree updates only)
> read index again
> optionally initialize name hash (when new entries are added, I guess)
> read packed-refs
> write index
>
> With this series, we run a single git-apply which does
>
> read index (and sharedindex too if in split-index mode)
> initialize name hash
> write index 80+ times
(See:
http://thread.gmane.org/gmane.comp.version-control.git/292324/focus=292460)
Links
~
This patch series is available here:
https://github.com/chriscool/git/commits/libify-apply-use-in-am
The previous versions are available there:
v1: https://github.com/chriscool/git/comm
Re: [PATCH] Document the 'svn propset' command.
Hey Alfred, On Mon, Jun 13, 2016 at 7:54 PM, Pranit Bauva wrote: > Hey Alfred, > > On Mon, Jun 13, 2016 at 6:22 PM, Alfred Perlstein wrote: >> Thank you Pranit. I thought that "signed off by" is used once someone >> approved my patch as opposed to when it's in "proposal" stage. This was my >> first email with a patch for this issue, who should/could I have used for >> "signoff"? > > Signoff is used to indicate that you are OKAY with releasing your > patch according to git's license. For more details see the > Documentation/SubmittingPatches[1]. To summarize you will have to add > this in the end : > >Signed-off-by: Alfred Perlstein > > Though I will still recommend you to go through [1] properly. Oops I forgot to put the link. [1]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L239-L307 Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document the 'svn propset' command.
Hey Alfred, On Mon, Jun 13, 2016 at 6:22 PM, Alfred Perlstein wrote: > Thank you Pranit. I thought that "signed off by" is used once someone > approved my patch as opposed to when it's in "proposal" stage. This was my > first email with a patch for this issue, who should/could I have used for > "signoff"? Signoff is used to indicate that you are OKAY with releasing your patch according to git's license. For more details see the Documentation/SubmittingPatches[1]. To summarize you will have to add this in the end : Signed-off-by: Alfred Perlstein Though I will still recommend you to go through [1] properly. Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document the 'svn propset' command.
Thank you Pranit. I thought that "signed off by" is used once someone approved my patch as opposed to when it's in "proposal" stage. This was my first email with a patch for this issue, who should/could I have used for "signoff"? -Alfred On 6/12/16 11:59 PM, Pranit Bauva wrote: Hey Alfred, On Mon, Jun 13, 2016 at 12:45 AM, Alfred Perlstein wrote: Junio + all, A week ago I was requested to provide documentation for the 'svn propset' command. I have attached a diff off of the 'maint' branch for this, however it seems to apply cleanly to 'master' as well. Thank you for your patience. This is also available on my github here: https://github.com/splbio/git/tree/document_propset I am not particularly sure whether the above could form a good commit message. I think you wanted to include this as a comment. git-am picks up these patches. The title commit is taken from the subject stripping the '[PATCH]'. Then the body before '---' is taken as the rest of the commit message. Then the diff is applied. To include comments add them after ---. Also please have a look at Documentation/SubmittingPatches. Also missing signoff. For the patch to actually be accept you need to follow those instructions. Thanks for contributing. --- Documentation/git-svn.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index fb23a98..e104824 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -459,6 +459,20 @@ Any other arguments are passed directly to 'git log' Gets the Subversion property given as the first argument, for a file. A specific revision can be specified with -r/--revision. +'propset':: + Sets the Subversion property given as the first argument, to the + value given as the second argument for the file given as the + third argument. ++ +Example: ++ + +git svn propset svn:keywords "FreeBSD=%H" devel/py-tipper/Makefile + ++ +This will set the property 'svn:keywords' to 'FreeBSD=%H' for the file +'devel/py-tipper/Makefile'. + 'show-externals':: Shows the Subversion externals. Use -r/--revision to specify a specific revision. Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] lib-httpd.sh: print error.log on error
Failure to bring up httpd for testing is not considered an error, so the
trash directory, which contains this error.log file, is removed and we
don't know what made httpd fail to start. Improve the situation a bit,
print error.log but only in verbose mode.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Redirect stdout to "verbose stderr" instead of testing $verbose.
t/lib-httpd.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index f9f3e5f..ac2cbee 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -180,6 +180,7 @@ start_httpd() {
if test $? -ne 0
then
trap 'die' EXIT
+ cat "$HTTPD_ROOT_PATH"/error.log >&4 2>/dev/null
test_skip_or_die $GIT_TEST_HTTPD "web server setup failed"
fi
}
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/6] worktree.c: add find_worktree()
On Fri, Jun 3, 2016 at 10:00 PM, Ramsay Jones
wrote:
>
>
> On 03/06/16 13:19, Nguyễn Thái Ngọc Duy wrote:
>> So far we haven't needed to identify an existing worktree from command
>> line. Future commands such as lock or move will need it. The current
>> implementation identifies worktrees by path (*). In future, the function
>> could learn to identify by $(basename $path) or tags...
>>
>> (*) We could probably go cheaper with comparing inode number (and
>> probably more reliable than paths when unicode enters the game). But not
>> all systems have good inode that so let's stick to something simple for
>> now.
>>
>> Helped-by: Eric Sunshine
>> Signed-off-by: Nguyễn Thái Ngọc Duy
>> ---
>> worktree.c | 15 +++
>> worktree.h | 8
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/worktree.c b/worktree.c
>> index e2a94e0..554f566 100644
>> --- a/worktree.c
>> +++ b/worktree.c
>> @@ -214,6 +214,21 @@ const char *get_worktree_git_dir(const struct worktree
>> *wt)
>> return git_common_path("worktrees/%s", wt->id);
>> }
>>
>> +struct worktree *find_worktree(struct worktree **list,
>> +const char *prefix,
>> +const char *arg)
>> +{
>> + char *path;
>> +
>> + arg = prefix_filename(prefix, strlen(prefix), arg);
>> + path = xstrdup(real_path(arg));
>> + for (; *list; list++)
>> + if (!fspathcmp(path, real_path((*list)->path)))
>
> The results of the call to real_path() should probably be cached
> in the worktree structure, since real_path() is relatively expensive
> (it calls chdir(), lstat(), readlink() etc.), so you don't want to
> re-compute the same result time-after-time ...
Urgh.. I missed this after sending out v5. Because find_worktree is
probably called once or twice per process, I don't think we need to
optimize this yet. If nr. worktrees goes up to hundreds then this is
one of many items we need to do to make worktree list fast.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/6] worktree: add "lock" command
Helped-by: Eric Sunshine
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-worktree.txt | 26 +-
builtin/worktree.c | 38 +++
contrib/completion/git-completion.bash | 5 +++-
t/t2028-worktree-move.sh (new +x) | 48 ++
4 files changed, 110 insertions(+), 7 deletions(-)
create mode 100755 t/t2028-worktree-move.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 27feff6..b49b25b 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [--checkout] [-b ]
[]
'git worktree list' [--porcelain]
+'git worktree lock' [--reason ]
'git worktree prune' [-n] [-v] [--expire ]
DESCRIPTION
@@ -38,9 +39,8 @@ section "DETAILS" for more information.
If a linked working tree is stored on a portable device or network share
which is not always mounted, you can prevent its administrative files from
-being pruned by creating a file named 'locked' alongside the other
-administrative files, optionally containing a plain text reason that
-pruning should be suppressed. See section "DETAILS" for more information.
+being pruned by issuing the `git worktree lock` command, optionally
+specifying `--reason` to explain why the working tree is locked.
COMMANDS
@@ -61,6 +61,14 @@ each of the linked worktrees. The output details include if
the worktree is
bare, the revision currently checked out, and the branch currently checked out
(or 'detached HEAD' if none).
+lock::
+
+If a working tree is on a portable device or network share which
+is not always mounted, lock it to prevent its administrative
+files from being pruned automatically. This also prevents it from
+being moved or deleted. Optionally, specify a reason for the lock
+with `--reason`.
+
prune::
Prune working tree information in $GIT_DIR/worktrees.
@@ -110,6 +118,13 @@ OPTIONS
--expire ::
With `prune`, only expire unused working trees older than .
+--reason ::
+ With `lock`, an explanation why the working tree is locked.
+
+::
+ Working trees can be identified by path, either relative or
+ absolute.
+
DETAILS
---
Each linked working tree has a private sub-directory in the repository's
@@ -150,7 +165,8 @@ instead.
To prevent a $GIT_DIR/worktrees entry from being pruned (which
can be useful in some situations, such as when the
-entry's working tree is stored on a portable device), add a file named
+entry's working tree is stored on a portable device), use the
+`git worktree lock` command, which adds a file named
'locked' to the entry's directory. The file contains the reason in
plain text. For example, if a linked working tree's `.git` file points
to `/path/main/.git/worktrees/test-next` then a file named
@@ -226,8 +242,6 @@ performed manually, such as:
- `remove` to remove a linked working tree and its administrative files (and
warn if the working tree is dirty)
- `mv` to move or rename a working tree and update its administrative files
-- `lock` to prevent automatic pruning of administrative files (for instance,
- for a working tree on a portable device)
GIT
---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index f9dac37..3b9220f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -14,6 +14,7 @@
static const char * const worktree_usage[] = {
N_("git worktree add [] []"),
N_("git worktree list []"),
+ N_("git worktree lock [] "),
N_("git worktree prune []"),
NULL
};
@@ -459,6 +460,41 @@ static int list(int ac, const char **av, const char
*prefix)
return 0;
}
+static int lock_worktree(int ac, const char **av, const char *prefix)
+{
+ const char *reason = "", *old_reason;
+ struct option options[] = {
+ OPT_STRING(0, "reason", &reason, N_("string"),
+ N_("reason for locking")),
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 1)
+ usage_with_options(worktree_usage, options);
+
+ worktrees = get_worktrees();
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working tree"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("The main working tree cannot be locked or unlocked"));
+
+ old_reason = is_worktree_locked(wt);
+ if (old_reason) {
+ if (*old_reason)
+ die(_("'%s' is already locked, reason: %s"),
+ av[0], old_reason);
+ die(_("'%s' is already locked"), av[0]);
+ }
+
+ write_file(git_common_path("worktrees/%s/locked", wt->id),
+ "%s", reason);
+ free_worktrees(worktrees);
+ return 0;
+}
+
int cmd_w
[PATCH v5 2/6] worktree.c: add is_main_worktree()
Main worktree _is_ different. You can lock (*) a linked worktree but not
the main one, for example. Provide an API for checking that.
(*) Add the file $GIT_DIR/worktrees/xxx/locked to avoid worktree xxx
from being removed or moved.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
worktree.c | 5 +
worktree.h | 5 +
2 files changed, 10 insertions(+)
diff --git a/worktree.c b/worktree.c
index 0782e00..12a766a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -229,6 +229,11 @@ struct worktree *find_worktree(struct worktree **list,
return *list;
}
+int is_main_worktree(const struct worktree *wt)
+{
+ return !wt->id;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index 7ad15da..e1c4715 100644
--- a/worktree.h
+++ b/worktree.h
@@ -37,6 +37,11 @@ extern struct worktree *find_worktree(struct worktree **list,
const char *prefix,
const char *arg);
+/*
+ * Return true if the given worktree is the main one.
+ */
+extern int is_main_worktree(const struct worktree *wt);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/6] worktree.c: find_worktree() search by path suffix
This allows the user to do something like "worktree lock foo" or
"worktree lock to/foo" instead of "worktree lock /long/path/to/foo" if
it's unambiguous.
With completion support it could be quite convenient. While this base
name search can be done in the same worktree iteration loop, the code is
split into a separate function for clarity.
Suggested-by: Eric Sunshine
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-worktree.txt | 5 +
worktree.c | 29 +
2 files changed, 34 insertions(+)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 27330c5..7850dee 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -129,6 +129,11 @@ OPTIONS
::
Working trees can be identified by path, either relative or
absolute.
++
+If the last path components in the working tree's path is unique among
+working trees, it can be used to identify worktrees. For example if
+you only have to working trees at "/abc/def/ghi" and "/abc/def/ggg",
+then "ghi" or "def/ghi" is enough to point to the former working tree.
DETAILS
---
diff --git a/worktree.c b/worktree.c
index 2bcfff3..2107c06 100644
--- a/worktree.c
+++ b/worktree.c
@@ -219,12 +219,41 @@ const char *get_worktree_git_dir(const struct worktree
*wt)
return git_common_path("worktrees/%s", wt->id);
}
+static struct worktree *find_worktree_by_suffix(struct worktree **list,
+ const char *suffix)
+{
+ struct worktree *found = NULL;
+ int nr_found = 0, suffixlen;
+
+ suffixlen = strlen(suffix);
+ if (!suffixlen)
+ return NULL;
+
+ for (; *list && nr_found < 2; list++) {
+ const char *path= (*list)->path;
+ int pathlen = strlen(path);
+ int start = pathlen - suffixlen;
+
+ /* suffix must start at directory boundary */
+ if ((!start || (start > 0 && is_dir_sep(path[start - 1]))) &&
+ !fspathcmp(suffix, path + start)) {
+ found = *list;
+ nr_found++;
+ }
+ }
+ return nr_found == 1 ? found : NULL;
+}
+
struct worktree *find_worktree(struct worktree **list,
const char *prefix,
const char *arg)
{
+ struct worktree *wt;
char *path;
+ if ((wt = find_worktree_by_suffix(list, arg)))
+ return wt;
+
arg = prefix_filename(prefix, strlen(prefix), arg);
path = xstrdup(real_path(arg));
for (; *list; list++)
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/6] worktree lock/unlock
v5 fixes some error messages mentioning "working directory" instead of
"working tree" and split the double use of "lock_reason" field in
"struct worktree". This series depends on
nd/worktree-cleanup-post-head-protection.
Diff from v4
-- 8< --
diff --git a/builtin/worktree.c b/builtin/worktree.c
index cb5026d..4877421 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -478,9 +478,9 @@ static int lock_worktree(int ac, const char **av, const
char *prefix)
worktrees = get_worktrees();
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
- die(_("'%s' is not a working directory"), av[0]);
+ die(_("'%s' is not a working tree"), av[0]);
if (is_main_worktree(wt))
- die(_("'%s' is a main working directory"), av[0]);
+ die(_("The main working tree cannot be locked or unlocked"));
old_reason = is_worktree_locked(wt);
if (old_reason) {
@@ -511,9 +511,9 @@ static int unlock_worktree(int ac, const char **av, const
char *prefix)
worktrees = get_worktrees();
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
- die(_("'%s' is not a working directory"), av[0]);
+ die(_("'%s' is not a working tree"), av[0]);
if (is_main_worktree(wt))
- die(_("'%s' is a main working directory"), av[0]);
+ die(_("The main working tree cannot be locked or unlocked"));
if (!is_worktree_locked(wt))
die(_("'%s' is not locked"), av[0]);
ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
diff --git a/worktree.c b/worktree.c
index b16262b..2107c06 100644
--- a/worktree.c
+++ b/worktree.c
@@ -5,8 +5,6 @@
#include "dir.h"
#include "wt-status.h"
-static const char *lock_field_uninitialized = "value is not important";
-
void free_worktrees(struct worktree **worktrees)
{
int i = 0;
@@ -15,8 +13,7 @@ void free_worktrees(struct worktree **worktrees)
free(worktrees[i]->path);
free(worktrees[i]->id);
free(worktrees[i]->head_ref);
- if (worktrees[i]->lock_reason != lock_field_uninitialized)
- free(worktrees[i]->lock_reason);
+ free(worktrees[i]->lock_reason);
free(worktrees[i]);
}
free (worktrees);
@@ -102,7 +99,8 @@ static struct worktree *get_main_worktree(void)
worktree->is_detached = is_detached;
worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = (char *)lock_field_uninitialized;
+ worktree->lock_reason = NULL;
+ worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -148,7 +146,8 @@ static struct worktree *get_linked_worktree(const char *id)
worktree->is_detached = is_detached;
worktree->is_current = 0;
add_head_info(&head_ref, worktree);
- worktree->lock_reason = (char *)lock_field_uninitialized;
+ worktree->lock_reason = NULL;
+ worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -271,7 +270,9 @@ int is_main_worktree(const struct worktree *wt)
const char *is_worktree_locked(struct worktree *wt)
{
- if (wt->lock_reason == lock_field_uninitialized) {
+ assert(!is_main_worktree(wt));
+
+ if (!wt->lock_reason_valid) {
struct strbuf path = STRBUF_INIT;
strbuf_addstr(&path, worktree_git_path(wt, "locked"));
@@ -283,6 +284,7 @@ const char *is_worktree_locked(struct worktree *wt)
wt->lock_reason = strbuf_detach(&lock_reason, NULL);
} else
wt->lock_reason = NULL;
+ wt->lock_reason_valid = 1;
strbuf_release(&path);
}
diff --git a/worktree.h b/worktree.h
index 263b61d..90e1311 100644
--- a/worktree.h
+++ b/worktree.h
@@ -10,6 +10,7 @@ struct worktree {
int is_detached;
int is_bare;
int is_current;
+ int lock_reason_valid;
};
/* Functions for acting on the information about worktrees. */
-- >8 --
Nguyễn Thái Ngọc Duy (6):
worktree.c: add find_worktree()
worktree.c: add is_main_worktree()
worktree.c: add is_worktree_locked()
worktree: add "lock" command
worktree: add "unlock" command
worktree.c: find_worktree() search by path suffix
Documentation/git-worktree.txt | 36 +---
builtin/worktree.c | 66 +
contrib/completion/git-completion.bash | 5 ++-
t/t2028-worktree-move.sh (new +x) | 62 +++
worktree.c | 77 ++
worktree.h | 21 ++
6 files changed, 260 insertions(+), 7 deletions(-)
create mode 100755 t/t2028-worktree-move.sh
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe
[PATCH v5 1/6] worktree.c: add find_worktree()
So far we haven't needed to identify an existing worktree from command
line. Future commands such as lock or move will need it. The current
implementation identifies worktrees by path (*). In future, the function
could learn to identify by $(basename $path) or tags...
(*) We could probably go cheaper with comparing inode number (and
probably more reliable than paths when unicode enters the game). But not
all systems have good inode that so let's stick to something simple for
now.
Helped-by: Eric Sunshine
Signed-off-by: Nguyễn Thái Ngọc Duy
---
worktree.c | 15 +++
worktree.h | 8
2 files changed, 23 insertions(+)
diff --git a/worktree.c b/worktree.c
index f4a4f38..0782e00 100644
--- a/worktree.c
+++ b/worktree.c
@@ -214,6 +214,21 @@ const char *get_worktree_git_dir(const struct worktree *wt)
return git_common_path("worktrees/%s", wt->id);
}
+struct worktree *find_worktree(struct worktree **list,
+ const char *prefix,
+ const char *arg)
+{
+ char *path;
+
+ arg = prefix_filename(prefix, strlen(prefix), arg);
+ path = xstrdup(real_path(arg));
+ for (; *list; list++)
+ if (!fspathcmp(path, real_path((*list)->path)))
+ break;
+ free(path);
+ return *list;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index 1394909..7ad15da 100644
--- a/worktree.h
+++ b/worktree.h
@@ -29,6 +29,14 @@ extern struct worktree **get_worktrees(void);
*/
extern const char *get_worktree_git_dir(const struct worktree *wt);
+/*
+ * Search a worktree that can be unambiguously identified by
+ * "arg". "prefix" must not be NULL.
+ */
+extern struct worktree *find_worktree(struct worktree **list,
+ const char *prefix,
+ const char *arg);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/6] worktree.c: add is_worktree_locked()
We need this later to avoid double locking a worktree, or unlocking one
when it's not even locked.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
worktree.c | 28
worktree.h | 8
2 files changed, 36 insertions(+)
diff --git a/worktree.c b/worktree.c
index 12a766a..2bcfff3 100644
--- a/worktree.c
+++ b/worktree.c
@@ -13,6 +13,7 @@ void free_worktrees(struct worktree **worktrees)
free(worktrees[i]->path);
free(worktrees[i]->id);
free(worktrees[i]->head_ref);
+ free(worktrees[i]->lock_reason);
free(worktrees[i]);
}
free (worktrees);
@@ -98,6 +99,8 @@ static struct worktree *get_main_worktree(void)
worktree->is_detached = is_detached;
worktree->is_current = 0;
add_head_info(&head_ref, worktree);
+ worktree->lock_reason = NULL;
+ worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -143,6 +146,8 @@ static struct worktree *get_linked_worktree(const char *id)
worktree->is_detached = is_detached;
worktree->is_current = 0;
add_head_info(&head_ref, worktree);
+ worktree->lock_reason = NULL;
+ worktree->lock_reason_valid = 0;
done:
strbuf_release(&path);
@@ -234,6 +239,29 @@ int is_main_worktree(const struct worktree *wt)
return !wt->id;
}
+const char *is_worktree_locked(struct worktree *wt)
+{
+ assert(!is_main_worktree(wt));
+
+ if (!wt->lock_reason_valid) {
+ struct strbuf path = STRBUF_INIT;
+
+ strbuf_addstr(&path, worktree_git_path(wt, "locked"));
+ if (file_exists(path.buf)) {
+ struct strbuf lock_reason = STRBUF_INIT;
+ if (strbuf_read_file(&lock_reason, path.buf, 0) < 0)
+ die_errno(_("failed to read '%s'"), path.buf);
+ strbuf_trim(&lock_reason);
+ wt->lock_reason = strbuf_detach(&lock_reason, NULL);
+ } else
+ wt->lock_reason = NULL;
+ wt->lock_reason_valid = 1;
+ strbuf_release(&path);
+ }
+
+ return wt->lock_reason;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index e1c4715..90e1311 100644
--- a/worktree.h
+++ b/worktree.h
@@ -5,10 +5,12 @@ struct worktree {
char *path;
char *id;
char *head_ref;
+ char *lock_reason; /* internal use */
unsigned char head_sha1[20];
int is_detached;
int is_bare;
int is_current;
+ int lock_reason_valid;
};
/* Functions for acting on the information about worktrees. */
@@ -42,6 +44,12 @@ extern struct worktree *find_worktree(struct worktree **list,
*/
extern int is_main_worktree(const struct worktree *wt);
+/*
+ * Return the reason string if the given worktree is locked or NULL
+ * otherwise.
+ */
+extern const char *is_worktree_locked(struct worktree *wt);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/6] worktree: add "unlock" command
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/git-worktree.txt | 5 +
builtin/worktree.c | 28
contrib/completion/git-completion.bash | 2 +-
t/t2028-worktree-move.sh | 14 ++
4 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b49b25b..27330c5 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -13,6 +13,7 @@ SYNOPSIS
'git worktree list' [--porcelain]
'git worktree lock' [--reason ]
'git worktree prune' [-n] [-v] [--expire ]
+'git worktree unlock'
DESCRIPTION
---
@@ -73,6 +74,10 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+unlock::
+
+Unlock a working tree, allowing it to be pruned, moved or deleted.
+
OPTIONS
---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 3b9220f..4877421 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -16,6 +16,7 @@ static const char * const worktree_usage[] = {
N_("git worktree list []"),
N_("git worktree lock [] "),
N_("git worktree prune []"),
+ N_("git worktree unlock "),
NULL
};
@@ -495,6 +496,31 @@ static int lock_worktree(int ac, const char **av, const
char *prefix)
return 0;
}
+static int unlock_worktree(int ac, const char **av, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ int ret;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 1)
+ usage_with_options(worktree_usage, options);
+
+ worktrees = get_worktrees();
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working tree"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("The main working tree cannot be locked or unlocked"));
+ if (!is_worktree_locked(wt))
+ die(_("'%s' is not locked"), av[0]);
+ ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
+ free_worktrees(worktrees);
+ return ret;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -513,5 +539,7 @@ int cmd_worktree(int ac, const char **av, const char
*prefix)
return list(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "lock"))
return lock_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "unlock"))
+ return unlock_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index f88727d..0e3841d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2597,7 +2597,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock prune"
+ local subcommands="add list lock prune unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 87afc2e..68d3fe8 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -45,4 +45,18 @@ test_expect_success 'lock worktree twice (from the locked
worktree)' '
test_cmp expected .git/worktrees/source/locked
'
+test_expect_success 'unlock main worktree' '
+ test_must_fail git worktree unlock .
+'
+
+test_expect_success 'unlock linked worktree' '
+ git worktree unlock source &&
+ test_path_is_missing .git/worktrees/source/locked
+'
+
+test_expect_success 'unlock worktree twice' '
+ test_must_fail git worktree unlock source &&
+ test_path_is_missing .git/worktrees/source/locked
+'
+
test_done
--
2.8.2.524.g6ff3d78
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lib-httpd.sh: print error.log on error
On Mon, Jun 13, 2016 at 06:40:14PM +0700, Duy Nguyen wrote:
> I like the verbose route, so here's v2
I think this is OK, though...
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index f9f3e5f..67bc7ad 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -180,6 +180,8 @@ start_httpd() {
> if test $? -ne 0
> then
> trap 'die' EXIT
> + test "$verbose" = t && \
> + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null
I think you can just spell this:
cat >&4 ...
(I had originally thought that we set up those fds only inside the
test_expect blocks, but it is the redirection 2>&4 that we set up there;
you can always use fd 4 as necessary).
It does incur a useless "cat" when we are not verbose, but I don't think
that's a big deal for the error path like this.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lib-httpd.sh: print error.log on error
On Sun, Jun 12, 2016 at 08:59:21AM -0400, Jeff King wrote:
> On Sun, Jun 12, 2016 at 05:41:54PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
> > Failure to bring up httpd for testing is not considered an error, so the
> > trash directory, which contains this error.log file, is removed and we
> > don't know what made httpd fail to start. Improve the situation a bit.
> >
> > Signed-off-by: Nguyễn Thái Ngọc Duy
> > ---
> > t/lib-httpd.sh | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> > index f9f3e5f..5b8de38 100644
> > --- a/t/lib-httpd.sh
> > +++ b/t/lib-httpd.sh
> > @@ -180,6 +180,7 @@ start_httpd() {
> > if test $? -ne 0
> > then
> > trap 'die' EXIT
> > + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null
> > test_skip_or_die $GIT_TEST_HTTPD "web server setup failed"
> > fi
>
> I like the idea of giving more data on error, but I think this will
> break the TAP output and confuse anything parsing the output of the
> tests, like prove (I think arbitrary output should have "#" prepended).
>
> Also (or alternatively), it should probably only happen when we are in
> verbose mode (it's not taken care of for us as usual because tests call
> start_httpd outside of a test_expect_ block). I think this eliminates
> the need to deal with the TAP thing (because our usual "-v" output is
> not TAP-compliant).
I like the verbose route, so here's v2
-- 8< --
Subject: [PATCH v2] lib-httpd.sh: print error.log on error
Failure to bring up httpd for testing is not considered an error, so
the trash directory, which contains this error.log file, is removed
and we don't know why httpd failed. Improve the situation a bit, print
error.log but only in verbose mode.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
t/lib-httpd.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index f9f3e5f..67bc7ad 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -180,6 +180,8 @@ start_httpd() {
if test $? -ne 0
then
trap 'die' EXIT
+ test "$verbose" = t && \
+ cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null
test_skip_or_die $GIT_TEST_HTTPD "web server setup failed"
fi
}
--
2.8.2.524.g6ff3d78
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/3] use string_list initializer consistently
On Mon, Jun 13, 2016 at 5:04 PM, Jeff King wrote: > -- >8 -- > Subject: use string_list initializer consistently > > There are two types of string_lists: those that own the > string memory, and those that don't. You can tell the > difference by the strdup_strings flag, and one should use > either STRING_LIST_INIT_DUP, or STRING_LIST_INIT_NODUP as an > initializer. > > Historically, the normal all-zeros initialization has > corresponded to the NODUP case. Many sites use no > initializer at all, and that works as a shorthand for that > case. But for a reader of the code, it can be hard to > remember which is which. Let's be more explicit and actually > have each site declare which type it means to use. > > This is a fairly mechanical conversion; I assumed each site > was correct as-is, and just switched them all to NODUP. Looking good. If we still want to reduce noise level down (by a tiny bit), we could remove _INIT because I think it's obvious from struct string_list var = STRING_LIST_NODUP; that's it's initialization (I wish we could write "auto var = STRING_LIST_NODUP;") -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/8] Add initial experimental external ODB support
On Mon, Jun 13, 2016 at 3:55 PM, Christian Couder wrote: > Future work > ~~~ > > From the discussions it appear that using the bundle v3 mechanism to > tranfer external ODB data could work, but only if the server has access > to its own external ODB. > > Another possible mechanism to transfer external ODB data would be some > kind of replace refs. This would be slower but the mechanism for the > transfer already fully exists. > > So I think I am going to experiment with some kind of replace refs. Or we can go "fault-in" all the way. 1) Extend rev-list to support large blob exclusion, e.g. blobs whose size above a certain limit are excluded, or blobs in certain paths are excluded (maybe except those at the tip) 2) Add a new command, similar to "shallow", in the git protocol for the client to tell the server that it does not want the server to send certain large blobs (and the server probably should acknowledge with "ok you can get them from this url...") 3) upload-pack passes the relevant rev-list blob filtering arguments to pack-objects, no big blobs are transferred 4) External odb setting must be done (temporarily) before index-pack is run (as part of the clone/fetch process). index-pack will notice that certain blobs are missing from the received pack, but it will also notice that it already has them via standard has_sha1_file() (we just need to make sure it does not read blob content back, i think we're ok here). I'm assuming here that EODB in fact contains _all_ blobs, so no extra setup is required at the server side. 5) If index-pack says "ok" then you can make this new external odb permanent, else remove it. Subsequent fetches need to send the same "do not include these blobs" instructions, of course. Not sure how well it interacts with pack-bitmap, but I guess when you use this, you probably already use shallow clone (which disables pack bitmap). > One interesting thing also would be to use the streaming api when > reading from or writing to the external ODB. If EODB is about large blobs only, you probably want to stream them directly to a pack as you get them and not keep in loose object form. Infrastructure is already in place (I think it's used by git-add), but hooking it at sha1_file.c layer might be tricky. This is just cherry on top, not strictly needed of course. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] lock_ref_for_update(): make error handling more uniform
On 06/13/2016 09:16 AM, Michael Haggerty wrote: > On 06/10/2016 09:01 PM, David Turner wrote: >> On Fri, 2016-06-10 at 10:14 +0200, Michael Haggerty wrote: >> >>> /* >>> + * Check whether the REF_HAVE_OLD and old_oid values stored in update >>> + * are consistent with the result read for the reference. error is >>> + * true iff there was an error reading the reference; otherwise, oid >> >> "error" is not a thing here? > > You're right; thanks for the feedback. I'll include it in the reroll > that I'm about to do. I have changed the docstring as follows: > /* > * Check whether the REF_HAVE_OLD and old_oid values stored in update > - * are consistent with the result read for the reference. error is > - * true iff there was an error reading the reference; otherwise, oid > - * is the value read for the reference. > - * > - * If there was a problem, write an error message to err and return > - * -1. > + * are consistent with oid, which is the reference's current value. If > + * everything is OK, return 0; otherwise, write an error message to > + * err and return -1. > */ I've folded that change into the patch series on branch update-ref-errors on my GitHub fork [1]. I won't sent a re-roll to the mailing list unless other changes are necessary (or unless somebody requests it). Michael [1] https://github.com/mhagger/git -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/3] use string_list initializer consistently
On Mon, Jun 13, 2016 at 04:36:14PM +0700, Duy Nguyen wrote:
> > So I'd suggest these patches:
> >
> > [1/3]: parse_opt_string_list: stop allocating new strings
> > [2/3]: interpret-trailers: don't duplicate option strings
> > [3/3]: blame,shortlog: don't make local option variables static
>
> As usual, it's hard to find things to complain in your patches.
That's only because I work on the easy bits. I'm afraid of things like
shallow-fetch refactoring. :)
> > The first one is what we've been discussing, and the others are just
> > follow-on cleanups. I stopped short of a fourth patch to convert more
> > cases of:
> >
> > static struct string_list foo;
> >
> > to:
> >
> > static struct string_list foo = STRING_LIST_INIT_NODUP;
> >
> > The two are equivalent (mostly due to historical reasons). I tend to
> > think explicit is better than implicit for something like this (not
> > because BSS auto-initialization isn't OK, but because there is an
> > explicit choice of dup/nodup that the writer made, and it is good to
> > communicate that). But maybe people don't want the extra noise.
>
> I'm on the explicit side. Unless you work a lot with string lists, I
> don't think you can remember whether "all zero" initialization is dup
> or no dup.
Here's the patch for that.
-- >8 --
Subject: use string_list initializer consistently
There are two types of string_lists: those that own the
string memory, and those that don't. You can tell the
difference by the strdup_strings flag, and one should use
either STRING_LIST_INIT_DUP, or STRING_LIST_INIT_NODUP as an
initializer.
Historically, the normal all-zeros initialization has
corresponded to the NODUP case. Many sites use no
initializer at all, and that works as a shorthand for that
case. But for a reader of the code, it can be hard to
remember which is which. Let's be more explicit and actually
have each site declare which type it means to use.
This is a fairly mechanical conversion; I assumed each site
was correct as-is, and just switched them all to NODUP.
Signed-off-by: Jeff King
---
builtin/apply.c | 6 +++---
builtin/blame.c | 2 +-
builtin/clone.c | 4 ++--
builtin/log.c | 6 +++---
builtin/remote.c | 2 +-
notes.c | 2 +-
submodule.c | 2 +-
t/helper/test-parse-options.c | 2 +-
8 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c770d7d..f27520f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -270,7 +270,7 @@ struct image {
* the case where more than one patches touch the same file.
*/
-static struct string_list fn_table;
+static struct string_list fn_table = STRING_LIST_INIT_NODUP;
static uint32_t hash_line(const char *cp, size_t len)
{
@@ -1936,7 +1936,7 @@ static void prefix_patch(struct patch *p)
* include/exclude
*/
-static struct string_list limit_by_name;
+static struct string_list limit_by_name = STRING_LIST_INIT_NODUP;
static int has_include;
static void add_name_limit(const char *name, int exclude)
{
@@ -3582,7 +3582,7 @@ static int check_to_create(const char *new_name, int
ok_if_exists)
* it is perfectly fine, as the patch removes a/b to make room
* to create a directory a/b so that a/b/c can be created.
*/
-static struct string_list symlink_changes;
+static struct string_list symlink_changes = STRING_LIST_INIT_NODUP;
#define SYMLINK_GOES_AWAY 01
#define SYMLINK_IN_RESULT 02
diff --git a/builtin/blame.c b/builtin/blame.c
index 80d2431..de70153 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -56,7 +56,7 @@ static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
-static struct string_list mailmap;
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
#ifndef DEBUG
#define DEBUG 0
diff --git a/builtin/clone.c b/builtin/clone.c
index 5f867e6..70d8213 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -49,8 +49,8 @@ static char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
static int option_progress = -1;
static enum transport_family family;
-static struct string_list option_config;
-static struct string_list option_reference;
+static struct string_list option_config = STRING_LIST_INIT_NODUP;
+static struct string_list option_reference = STRING_LIST_INIT_NODUP;
static int option_dissociate;
static int max_jobs = -1;
diff --git a/builtin/log.c b/builtin/log.c
index 099f4f7..8eef94f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,9 +674,9 @@ static int auto_number = 1;
static char *default_attach = NULL;
-static struct string_list extra_hdr;
-static struct string_list extra_to;
-static struct string_list extra_cc;
+static struct string_list extra_hdr = STRING_LIST_INIT_NODUP;
+static struct string_list extra_to = STRING_LIST_INIT_NODUP;
+static struct string_list extra_cc = STRING_LIST_INIT_NODUP;
static void add
[ADDENDUM v4] Yet more preparation for reference backends
Below is the delta needed to fixed the bug in the mh/split-under-lock
patch series that I mentioned in an earlier email [1], plus a little
tweak to make the docstring for lock_ref_for_update() clearer.
I actually fixed the bug in preparatory commit
ref_transaction_commit(): remove local variable n
, whose subject was accordingly changed to
ref_transaction_commit(): remove local variables n and updates
[2]. See branch "split-under-lock" in my GitHub fork [3] for the full
revised patch series (including the changes to a later patch that are
necessary when it is rebased onto the fix). In the same repo I've also
rebased dependent branches update-ref-errors, ref-iterators, and
ref-store on top of this one; all of those rebases were trivial.
Here, for the convenience of reviewers, I present the delta between
the old and new end states of the split-under-lock patch series. (It
didn't seem justified to re-send the whole 33-patch series for this
logically small change.)
Michael
[1] http://article.gmane.org/gmane.comp.version-control.git/297002
[2]
https://github.com/gitster/git/commit/efe472813d60befd72d6e2797934c90b22a26c93
[3] https://github.com/mhagger/git
---
refs/files-backend.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1230dfb..bbf96ad 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3393,7 +3393,8 @@ static const char *original_update_refname(struct
ref_update *update)
* - Lock the reference referred to by update.
* - Read the reference under lock.
* - Check that its old SHA-1 value (if specified) is correct, and in
- * any case record it for later use in the reflog.
+ * any case record it in update->lock->old_oid for later use when
+ * writing the reflog.
* - If it is a symref update without REF_NODEREF, split it up into a
* REF_LOG_ONLY update of the symref and add a separate update for
* the referent to transaction.
@@ -3556,7 +3557,6 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
- struct ref_update **updates = transaction->updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
struct string_list_item *ref_to_delete;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
@@ -3582,7 +3582,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
* same refname as any existing ones.)
*/
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
struct string_list_item *item =
string_list_append(&affected_refnames, update->refname);
@@ -3632,7 +3632,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
* open at a time to avoid running out of file descriptors.
*/
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
ret = lock_ref_for_update(update, transaction, head_ref,
&affected_refnames, err);
@@ -3642,7 +3642,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
/* Perform updates first so live commits remain referenced */
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
struct ref_lock *lock = update->lock;
if (update->flags & REF_NEEDS_COMMIT ||
@@ -3674,7 +3674,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
}
/* Perform deletes now that updates are safely completed */
for (i = 0; i < transaction->nr; i++) {
- struct ref_update *update = updates[i];
+ struct ref_update *update = transaction->updates[i];
if (update->flags & REF_DELETING &&
!(update->flags & REF_LOG_ONLY)) {
@@ -3701,8 +3701,8 @@ cleanup:
transaction->state = REF_TRANSACTION_CLOSED;
for (i = 0; i < transaction->nr; i++)
- if (updates[i]->lock)
- unlock_ref(updates[i]->lock);
+ if (transaction->updates[i]->lock)
+ unlock_ref(transaction->updates[i]->lock);
string_list_clear(&refs_to_delete, 0);
free(head_ref);
string_list_clear(&affected_refnames, 0);
@@ -3722,7 +3722,6 @@ int initial_ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
- struct ref_update **updates = transaction->updates;
struct string_list affected_refnames = STRING_LIS
Re: [PATCH 0/3] fix parse-opt string_list leaks
On Mon, Jun 13, 2016 at 12:32 PM, Jeff King wrote:
> On Mon, Jun 13, 2016 at 07:08:55AM +0700, Duy Nguyen wrote:
>
>> > So if we are doing the conservative thing, then I think the resulting
>> > code should either look like:
>> >
>> > if (!v->strdup_strings)
>> > die("BUG: OPT_STRING_LIST should always use strdup_strings");
>> > string_list_append(v, arg);
>>
>> I agree with the analysis. But this die() would hit all callers
>> (except interpret-trailers) because they all initialize with _NODUP
>> and setting strdup_strings may require auditing all access to the
>> string list in question, e.g. to change string_list_append(v,
>> xstrdup(xxx)) to string_list_append(xxx). it may cause side effects if
>> we are not careful.
>
> Yep. It is not really fixing anything, so much as alerting us to broken
> callers. We'd still have to fix the callers. :)
>
>> So far all callers are in builtin/, I think it will not take much time
>> to verify that they all call parse_options() with global argv, then we
>> can just lose extra xstrdup() and stick to string_list_append().
>> OPTION_STRING already assumes that argument strings are stable because
>> they are passed back as-is. Can we go with an easier route, adding a
>> comment on top of parse_options() stating that argv[] pointers may be
>> passed back as-is and it's up to the caller to xstrdup() appropriately
>> before argv[] memory is freed?
>
> Yeah, the two options I laid out were the "conservative" side, where we
> didn't make any assumptions about what is in passed into parse_options.
> But I agree in practice that it's not likely to be a problem to just
> point to the existing strings, and the fact that OPTION_STRING does so
> already makes me even more confident.
>
> So I'd suggest these patches:
>
> [1/3]: parse_opt_string_list: stop allocating new strings
> [2/3]: interpret-trailers: don't duplicate option strings
> [3/3]: blame,shortlog: don't make local option variables static
As usual, it's hard to find things to complain in your patches.
> The first one is what we've been discussing, and the others are just
> follow-on cleanups. I stopped short of a fourth patch to convert more
> cases of:
>
> static struct string_list foo;
>
> to:
>
> static struct string_list foo = STRING_LIST_INIT_NODUP;
>
> The two are equivalent (mostly due to historical reasons). I tend to
> think explicit is better than implicit for something like this (not
> because BSS auto-initialization isn't OK, but because there is an
> explicit choice of dup/nodup that the writer made, and it is good to
> communicate that). But maybe people don't want the extra noise.
I'm on the explicit side. Unless you work a lot with string lists, I
don't think you can remember whether "all zero" initialization is dup
or no dup.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/8] external odb foreach
From: Jeff King
---
external-odb.c | 14 ++
external-odb.h | 6 ++
odb-helper.c | 15 +++
odb-helper.h | 4
4 files changed, 39 insertions(+)
diff --git a/external-odb.c b/external-odb.c
index 1ccfa99..42978a3 100644
--- a/external-odb.c
+++ b/external-odb.c
@@ -113,3 +113,17 @@ int external_odb_fetch_object(const unsigned char *sha1)
return -1;
}
+
+int external_odb_for_each_object(each_external_object_fn fn, void *data)
+{
+ struct odb_helper *o;
+
+ external_odb_init();
+
+ for (o = helpers; o; o = o->next) {
+ int r = odb_helper_for_each_object(o, fn, data);
+ if (r)
+ return r;
+ }
+ return 0;
+}
diff --git a/external-odb.h b/external-odb.h
index 2397477..cea8570 100644
--- a/external-odb.h
+++ b/external-odb.h
@@ -5,4 +5,10 @@ const char *external_odb_root(void);
int external_odb_has_object(const unsigned char *sha1);
int external_odb_fetch_object(const unsigned char *sha1);
+typedef int (*each_external_object_fn)(const unsigned char *sha1,
+ enum object_type type,
+ unsigned long size,
+ void *data);
+int external_odb_for_each_object(each_external_object_fn, void *);
+
#endif /* EXTERNAL_ODB_H */
diff --git a/odb-helper.c b/odb-helper.c
index 7029a59..045cf6f 100644
--- a/odb-helper.c
+++ b/odb-helper.c
@@ -235,5 +235,20 @@ int odb_helper_fetch_object(struct odb_helper *o, const
unsigned char *sha1,
return -1;
}
+ return 0;
+}
+
+int odb_helper_for_each_object(struct odb_helper *o,
+ each_external_object_fn fn,
+ void *data)
+{
+ int i;
+ for (i = 0; i < o->have_nr; i++) {
+ struct odb_helper_object *obj = &o->have[i];
+ int r = fn(obj->sha1, obj->type, obj->size, data);
+ if (r)
+ return r;
+ }
+
return 0;
}
diff --git a/odb-helper.h b/odb-helper.h
index 0f704f9..8c3916d 100644
--- a/odb-helper.h
+++ b/odb-helper.h
@@ -1,6 +1,8 @@
#ifndef ODB_HELPER_H
#define ODB_HELPER_H
+#include "external-odb.h"
+
struct odb_helper {
const char *name;
const char *cmd;
@@ -21,5 +23,7 @@ struct odb_helper *odb_helper_new(const char *name, int
namelen);
int odb_helper_has_object(struct odb_helper *o, const unsigned char *sha1);
int odb_helper_fetch_object(struct odb_helper *o, const unsigned char *sha1,
int fd);
+int odb_helper_for_each_object(struct odb_helper *o,
+ each_external_object_fn, void *);
#endif /* ODB_HELPER_H */
--
2.9.0.rc2.362.g3cd93d0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/8] Add initial experimental external ODB support
Goal
Git can store its objects only in the form of loose objects in
separate files or packed objects in a pack file.
To be able to better handle some kind of objects, for example big
blobs, it would be nice if Git could store its objects in other object
databases (ODB).
To do that, this patch series makes it possible to register commands,
using "odb..command" config variables, to access external
ODBs where objects can be stored and retrieved.
Design
~~
Each registered command manages access to one external ODB and will be
called the following ways:
- " have": the command should output the sha1, size and
type of all the objects the external ODB contains, one object per
line.
- " get ": the command should then read from the
external ODB the content of the object corresponding to and
output it on stdout.
- " put ": the command should then read
from stdin an object and store it in the external ODB.
This RFC patch series for now does not address the following important
parts of a complete solution:
- There is no way to transfer external ODB content using Git.
- No real external ODB has been interfaced with Git. The tests use
another git repo in a separate directory for this purpose which is
probably useless in the real world.
Design discussion about performance
~~~
Yeah, it is not efficient to fork/exec a command to just read or write
one object to or from the external ODB. Batch calls and/or using a
daemon and/or RPC should be used instead to be able to store regular
objects in an external ODB. But for now the external ODB would be all
about really big files, where the cost of a fork+exec should not
matter much. If we later want to extend usage of external ODBs, yeah
we will probably need to design other mechanisms.
Here are some related explanations from Peff:
{{{
Because this "external odb" essentially acts as a git alternate, we
would hit it only when we couldn't find an object through regular means.
Git would then make the object available in the usual on-disk format
(probably as a loose object).
So in most processes, we would not need to consult the odb command at
all. And when we do, the first thing would be to get its "have" list,
which would at most run once per process.
So the per-object cost is really calling "get", and my assumption there
was that the cost of actually retrieving the object over the network
would dwarf the fork/exec cost.
I also waffled on having git cache the output of " have" in
some fast-lookup format to save even the single fork/exec. But I figured
that was something that could be added later if needed.
You'll note that this is sort of a "fault-in" model. Another model would
be to treat external odb updates similar to fetches. I.e., we touch the
network only during a special update operation, and then try to work
locally with whatever the external odb has. IMHO this policy could
actually be up to the external odb itself (i.e., its "have" command
could serve from a local cache if it likes).
}}}
Implementation
~~
This series adds a set of function in external-odb.{c,h} that are
called by the rest of Git to manage all the external ODBs.
These functions use 'struct odb_helper' and its associated functions
defined in odb-helper.{c,h} to talk to the different external ODBs by
launching the configured "odb..command" commands and writing
to or reading from them.
The tests in this series creates an odb-helper script that is
registered using the "odb.magic.command" config variable, and then
called to read from and write to the external ODB.
Highlevel view of the patches in the series
~~~
- Patches 01/08 and 02/08 are Peff's initial work fixed a little
bit so that it compiles and pass tests.
- Patches 03/08 is an optimization in the odb-helper script that
is used for testing. I will probably squash it into 01/08.
- Patches 04/08 and 05/08 are adding "put" support in the
odb-helper script and testing that.
- Patches 06/08 and 08/08 are enhancing external-odb.{c,h} and
odb-helper.{c,h}, so that Git can write into an external ODB.
- Patches 07/08 limits write support to "blobs" for now to
simplify things.
Future work
~~~
>From the discussions it appear that using the bundle v3 mechanism to
tranfer external ODB data could work, but only if the server has access
to its own external ODB.
Another possible mechanism to transfer external ODB data would be some
kind of replace refs. This would be slower but the mechanism for the
transfer already fully exists.
So I think I am going to experiment with some kind of replace refs.
One interesting thing also would be to use the streaming api when
reading from or writing to the external ODB.
Previous work and discussions
~
Peff started to work on this and discuss this some years ago:
http://thread.gmane.org/gmane.comp.version-control.
[RFC/PATCH 5/8] t0400: add test for 'put' command
Signed-off-by: Christian Couder --- t/t0400-external-odb.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh index 0f1bb97..6c6da5c 100755 --- a/t/t0400-external-odb.sh +++ b/t/t0400-external-odb.sh @@ -57,4 +57,13 @@ test_expect_success 'helper can retrieve alt objects' ' test_cmp expect actual ' +test_expect_success 'helper can add objects to alt repo' ' + hash=$(echo "Hello odb!" | git hash-object -w -t blob --stdin) && + test -f .git/objects/$(echo $hash | sed "s#..#&/#") && + size=$(git cat-file -s "$hash") && + git cat-file blob "$hash" | ./odb-helper put "$hash" "$size" blob && + alt_size=$(cd alt-repo && git cat-file -s "$hash") && + test "$size" -eq "$alt_size" +' + test_done -- 2.9.0.rc2.362.g3cd93d0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 3/8] t0400: use --batch-all-objects to get all objects
Signed-off-by: Christian Couder
---
t/t0400-external-odb.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh
index 2b01617..fe85413 100755
--- a/t/t0400-external-odb.sh
+++ b/t/t0400-external-odb.sh
@@ -10,9 +10,7 @@ write_script odb-helper <<\EOF
GIT_DIR=$ALT_SOURCE; export GIT_DIR
case "$1" in
have)
- git rev-list --all --objects |
- cut -d' ' -f1 |
- git cat-file --batch-check |
+ git cat-file --batch-check --batch-all-objects |
awk '{print $1 " " $3 " " $2}'
;;
get)
--
2.9.0.rc2.362.g3cd93d0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
