Re: [PATCH/WIP v3 08/31] am: apply patch with git-apply

[Junio C Hamano]

Paul Tan  writes:

> Implement applying the patch to the index using git-apply.
>
> Signed-off-by: Paul Tan 
> ---
>  builtin/am.c | 57 -
>  1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index d6434e4..296a5fc 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -27,6 +27,18 @@ static int is_empty_file(const char *filename)
>   return !st.st_size;
>  }
>  
> +/**
> + * Returns the first line of msg
> + */
> +static const char *firstline(const char *msg)
> +{
> + static struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_reset(&sb);
> + strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg);
> + return sb.buf;
> +}

Hmm.  This is not wrong per-se but a more efficient way to do it may
be to have a helper function that returns a bytecount of the first
line of the msg, i.e. strchrnul(msg, '\n') - msg.  Then a caller can
do

printf("Applying: %.*s", linelen(msg), msg);

instead of

printf("Applying: %s", firstline(msg));

relying on that the firstline() copies the contents to a static
strbuf that does not have to be freed.

> + struct child_process cp = CHILD_PROCESS_INIT;
> +
> + cp.git_cmd = 1;
> +
> + argv_array_push(&cp.args, "apply");
> +
> + argv_array_push(&cp.args, "--index");
> +
> + argv_array_push(&cp.args, am_path(state, "patch"));

You seem to like blank lines a lot ;-)  While it is a good tool to
separate different groups while grouping related things together,
these three argv-push calls are intimately related, and reads better
without blanks in between.

Looks nicely done so far...
--
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/WIP v3 08/31] am: apply patch with git-apply

[Paul Tan]

Implement applying the patch to the index using git-apply.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 57 -
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index d6434e4..296a5fc 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -27,6 +27,18 @@ static int is_empty_file(const char *filename)
return !st.st_size;
 }
 
+/**
+ * Returns the first line of msg
+ */
+static const char *firstline(const char *msg)
+{
+   static struct strbuf sb = STRBUF_INIT;
+
+   strbuf_reset(&sb);
+   strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg);
+   return sb.buf;
+}
+
 enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX
@@ -519,6 +531,31 @@ static int parse_patch(struct am_state *state, const char 
*patch)
return 0;
 }
 
+/*
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ */
+static int run_apply(const struct am_state *state)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   cp.git_cmd = 1;
+
+   argv_array_push(&cp.args, "apply");
+
+   argv_array_push(&cp.args, "--index");
+
+   argv_array_push(&cp.args, am_path(state, "patch"));
+
+   if (run_command(&cp))
+   return -1;
+
+   /* Reload index as git-apply will have modified it. */
+   discard_cache();
+   read_cache();
+
+   return 0;
+}
+
 /**
  * Applies all queued patches.
  */
@@ -536,7 +573,25 @@ static void am_run(struct am_state *state)
write_author_script(state);
write_file(am_path(state, "final-commit"), 1, "%s", 
state->msg.buf);
 
-   /* TODO: Patch application not implemented yet */
+   printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+
+   if (run_apply(state) < 0) {
+   int value;
+
+   printf_ln(_("Patch failed at %s %s"), msgnum(state),
+   firstline(state->msg.buf));
+
+   if (!git_config_get_bool("advice.amworkdir", &value) && 
!value)
+   printf_ln(_("The copy of the patch that failed 
is found in: %s"),
+   am_path(state, "patch"));
+
+   exit(128);
+   }
+
+   /*
+* TODO: After the patch has been applied to the index with
+* git-apply, we need to make commit as well.
+*/
 
 next:
am_next(state);
-- 
2.1.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