Re: [PATCH v7 00/40] libify apply and use lib in am, part 2

[Christian Couder]

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(_paths);
argv_array_clear(_opts);
+   clear_apply_state(_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", _overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(_verbosely, N_("be verbose")),
-   OPT_BOOL(0, "silent", _silent,
-   N_("do not print any output")),
OPT_BIT(0, "inaccurate-eof", ,
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 00/40] libify apply and use lib in am, part 2

[Christian Couder]

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: