Re: [PATCH v10 00/40] libify apply and use lib in am, part 2
Christian Couder writes:
> diff --git a/apply.h b/apply.h
> index 27a3a7a..e2b89e8 100644
> --- a/apply.h
> +++ b/apply.h
> @@ -16,7 +16,7 @@ enum apply_ws_ignore {
> enum apply_verbosity {
> verbosity_silent = -1,
> verbosity_normal = 0,
> -verbosity_verbose = 1,
> +verbosity_verbose = 1
> };
Thanks for not forgetting this.
> @@ -107,20 +111,6 @@ struct apply_state {
> int applied_after_fixing_ws;
> };
>
> -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,
> - const char *arg, int unset);
> -extern int apply_option_parse_p(const struct option *opt,
> -const char *arg,
> -int unset);
> -extern int apply_option_parse_whitespace(const struct option *opt,
> - const char *arg, int unset);
> -extern int apply_option_parse_directory(const struct option *opt,
> -const char *arg, int unset);
> -extern int apply_option_parse_space_change(const struct option *opt,
> - const char *arg, int unset);
> -
> extern int apply_parse_options(int argc, const char **argv,
> struct apply_state *state,
> int *force_apply, int *options,
Also these.
--
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 v10 00/40] libify apply and use lib in am, part 2
On Mon, Aug 8, 2016 at 11:02 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 a73889e..2ec2a8a 100644
--- a/apply.c
+++ b/apply.c
@@ -4324,7 +4324,10 @@ static int try_create_file(const char *path,
unsigned int mode, const char *buf,
size = nbuf.len;
buf = nbuf.buf;
}
-res = !write_or_whine_pipe(fd, buf, size, path);
+
+res = write_in_full(fd, buf, size) < 0;
+if (res)
+error_errno(_("failed to write to '%s'"), path);
strbuf_release(&nbuf);
if (close(fd) < 0 && !res)
@@ -4626,7 +4629,7 @@ static int apply_patch(struct apply_state *state,
int res = 0;
state->patch_input_file = filename;
-if (read_patch_file(&buf, fd))
+if (read_patch_file(&buf, fd) < 0)
return -128;
offset = 0;
while (offset < buf.len) {
@@ -4727,16 +4730,16 @@ static int apply_patch(struct apply_state *state,
return res;
}
-int apply_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;
}
-int apply_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);
@@ -4744,9 +4747,9 @@ int apply_option_parse_include(const struct option *opt,
return 0;
}
-int apply_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);
@@ -4754,8 +4757,8 @@ int apply_option_parse_p(const struct option *opt,
return 0;
}
-int apply_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)
@@ -4765,8 +4768,8 @@ int apply_option_parse_space_change(const struct
option *opt,
return 0;
}
-int apply_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;
@@ -4775,8 +4778,8 @@ int apply_option_parse_whitespace(const struct
option *opt,
return 0;
}
-int apply_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);
diff --git a/apply.h b/apply.h
index 27a3a7a..e2b89e8 100644
--- a/apply.h
+++ b/apply.h
@@ -16,7 +16,7 @@ enum apply_ws_ignore {
enum apply_verbosity {
verbosity_silent = -1,
verbosity_normal = 0,
-verbosity_verbose = 1,
+verbosity_verbose = 1
};
/*
@@ -94,7 +94,11 @@ struct apply_state {
*/
struct string_list fn_table;
-/* This is to save some reporting routines */
+/*
+ * This is to save reporting routines before using
+ * set_error_routine() or set_warn_routine() to install muting
+ * routines when in verbosity_silent mode.
+ */
void (*saved_error_routine)(const char *err, va_list params);
void (*saved_warn_routine)(const char *warn, va_list params);
@@ -107,20 +111,6 @@ struct apply_state {
int applied_after_fixing_ws;
};
-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,
- const char *arg, int unset);
-extern int apply_option_parse_p(const struct option *opt,
-const char *arg,
-int unset);
-extern int apply_option_parse_whitespace(const struct option *opt,
- const char *arg, int unset);
-extern int apply_option_parse_directory(const struct option *opt,
-const char *arg, int unset);
-extern int apply_option_parse_space_change(const struct option *opt,
- const char *arg, int unset);
-
extern int apply_parse_options(int argc, const char **argv,
struct apply_state *state,
int *force_apply, int *options,
diff --git a/cache.h b/cache.h
[PATCH v10 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/
v7: http://thread.gmane.org/gmane.comp.version-control.git/297193/
v8:
https://public-inbox.org/git/20160627182429.31550-1-chriscool%40tuxfamily.org/
v9:
https://public-inbox.org/git/20160730172509.22939-1-chriscool%40tuxfamily.org/
Highlevel view of the patches in the series
~~~
This is "part 2" of the full patch series. This is built on top of the
"part 1" and as the "part 1" is now in "master", this "part 2" is
built on top of "master".
- Patch 01/40 was in v8 and v9, and hasn't changed.
It renames some structs and constants that will be moved into apply.h
to give them a more specific name.
- Patches 02/40 to 31/40 were in v1, v2, v6, v7, v8 and v9.
They finish libifying the apply functionality that was in
builtin/apply.c and move it into apply.{c,h}, but the libified
functionality is not yet used in `git am`.
There are a few minor changes in these patches. In 04/40 we now
consider that we have an error if read_patch_file() returns a negative
integer instead of just a non 0 integer, as Stefan suggested. In 26/40
we now use write_in_full() instead of write_or_whine_pipe(), as
suggested by Peff.
- Patch 32/40 was in v6, v7, v8 and v9, and hasn't changed.
It replaces some calls to error() with calls to error_errno().
- Patch 33/40 was in v2, v6, v7, v8 and v9.
It makes it possible to temporarily change the current index.
This is a hack to make it possible for `git am` to use the libified
apply functionality on a different index file.
`git am` used to do that by setting the GIT_INDEX_FILE env variable
before calling `git apply`.
The commit message has been improved again to explain more why we are
using this short cut and more comments have been added, especially in
apply.h, as suggested by Junio and Stefan.
- Patches 34/40 to 38/40 were in v2, v6, v7, v8 and v9.
They implement a way to make the libified apply code silent by
changing the bool `apply_verbosely` into a tristate enum called
`apply_verbosity`, that can be one of `verbosity_verbose`,
`verbosity_normal` or `verbosity_silent`.
This is because "git am", since it was a shell script, has been
silencing the apply functionality by redirecting file descriptors to
/dev/null, but this is not acceptable in C.
The most significant change in these patches is that one patch (34/41
in v9: write_or_die: use warning() instead of fprintf(stderr, ...))
has been removed, as it was not needed anymore, since we don't use
write_or_whine_pipe(), as suggested by Peff.
Another change is that in 34/40, a spurious coma has been removed just
after the last element in an enum, as suggested by Junio.
The last change is that a comment has been improved in 38/40 as
suggested by Stefan.
- Patch 39/40 was new in v9.
It refactors `git apply` option parsing to make it possible for `git
am` to easily pass some command line options to the libified applied
code as suggested by Junio.
Compared to v9, we now remove some useless function declarations from
"apply.h", and make the related functions static again in "apply.c",
as suggested by Ramsay.
- Patch 40/40 was in v1, v2, v6, v7, v8 and v9, and hasn't changed.
This patch makes `git am` use the libified functionality. It now uses
the refactored code from the new patch 40/40 to parse `git apply`
options.
General comments
Sorry if this patch series is still long. Hopefully the early part of
this series until 32/40 will be ready soon to be moved to next and
master, and I may only need to resend the rest.
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 a
