Re: [PATCH/WIP v2 04/19] am: implement patch queue mechanism
On Fri, Jun 12, 2015 at 1:39 AM, Stefan Beller wrote:
> On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan wrote:
>> Notes:
>> v2
>>
>> * Declare struct am_state as static
>>
>> builtin/am.c | 164
>> +++
>> 1 file changed, 164 insertions(+)
>>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index 0ccbe33..f061d21 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -6,6 +6,154 @@
>> #include "cache.h"
>> #include "builtin.h"
>> #include "exec_cmd.h"
>> +#include "parse-options.h"
>> +#include "dir.h"
>> +
>> +struct am_state {
>
> Did you mean to declare all the functions below to be static or the
> struct as well?
Well, everything in this file, with the exception of the cmd_am()
entry point, should have static linkage.
The changelog comment was referring to [1], but I should have made it
clearer. Sorry if it was confusing.
[1] http://thread.gmane.org/gmane.comp.version-control.git/270048/focus=270205
> Reading further, you declared it static below. I thought maybe it'd be
> useful to have definition
> and declaration up here, but having all declarations further below may
> be even better.
Right, I aimed to have a strict separation between "git-am: the
functionality" and "git-am: the command-line interface", where the
latter depends on the former, and not the other way round (or have
circular dependencies). The former perhaps could even be moved into
libgit.a in the future.
Thanks,
Paul
--
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/WIP v2 04/19] am: implement patch queue mechanism
On Thu, Jun 11, 2015 at 3:21 AM, Paul Tan wrote:
> git-am applies a series of patches. If the process terminates
> abnormally, we want to be able to resume applying the series of patches.
> This requires the session state to be saved in a persistent location.
>
> Implement the mechanism of a "patch queue", represented by 2 integers --
> the index of the current patch we are applying and the index of the last
> patch, as well as its lifecycle through the following functions:
>
> * am_setup(), which will set up the state directory
> $GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
> the last-known state will still persist.
>
> * am_load(), which is called if there is an am session in
> progress, to load the last known state from the state directory so we
> can resume applying patches.
>
> * am_run(), which will do the actual patch application. After applying a
> patch, it calls am_next() to increment the current patch index. The
> logic for applying and committing a patch is not implemented yet.
>
> * am_destroy(), which is finally called when we successfully applied all
> the patches in the queue, to clean up by removing the state directory
> and its contents.
>
> Signed-off-by: Paul Tan
> ---
>
> Notes:
> v2
>
> * Declare struct am_state as static
>
> builtin/am.c | 164
> +++
> 1 file changed, 164 insertions(+)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 0ccbe33..f061d21 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -6,6 +6,154 @@
> #include "cache.h"
> #include "builtin.h"
> #include "exec_cmd.h"
> +#include "parse-options.h"
> +#include "dir.h"
> +
> +struct am_state {
Did you mean to declare all the functions below to be static or the
struct as well?
Reading further, you declared it static below. I thought maybe it'd be
useful to have definition
and declaration up here, but having all declarations further below may
be even better.
> + /* state directory path */
> + struct strbuf dir;
> +
> + /* current and last patch numbers, 1-indexed */
> + int cur;
> + int last;
> +};
> +
> +/**
> + * Initializes am_state with the default values.
> + */
> +static void am_state_init(struct am_state *state)
> +{
> + memset(state, 0, sizeof(*state));
> +
> + strbuf_init(&state->dir, 0);
> +}
> +
> +/**
> + * Release memory allocated by an am_state.
> + */
> +static void am_state_release(struct am_state *state)
> +{
> + strbuf_release(&state->dir);
> +}
> +
> +/**
> + * Returns path relative to the am_state directory.
> + */
> +static inline const char *am_path(const struct am_state *state, const char
> *path)
> +{
> + return mkpath("%s/%s", state->dir.buf, path);
> +}
> +
> +/**
> + * Returns 1 if there is an am session in progress, 0 otherwise.
> + */
> +static int am_in_progress(const struct am_state *state)
> +{
> + struct stat st;
> +
> + if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode))
> + return 0;
> + if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode))
> + return 0;
> + if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode))
> + return 0;
> + return 1;
> +}
> +
> +/**
> + * Reads the contents of `file`. The third argument can be used to give a
> hint
> + * about the file size, to avoid reallocs. Returns number of bytes read on
> + * success, -1 if the file does not exist. If trim is set, trailing
> whitespace
> + * will be removed from the file contents.
> + */
> +static int read_state_file(struct strbuf *sb, const char *file, size_t hint,
> int trim)
> +{
> + strbuf_reset(sb);
> + if (strbuf_read_file(sb, file, hint) >= 0) {
> + if (trim)
> + strbuf_rtrim(sb);
> +
> + return sb->len;
> + }
> +
> + if (errno == ENOENT)
> + return -1;
> +
> + die_errno(_("could not read '%s'"), file);
> +}
> +
> +/**
> + * Loads state from disk.
> + */
> +static void am_load(struct am_state *state)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + read_state_file(&sb, am_path(state, "next"), 8, 1);
> + state->cur = strtol(sb.buf, NULL, 10);
> +
> + read_state_file(&sb, am_path(state, "last"), 8, 1);
> + state->last = strtol(sb.buf, NULL, 10);
> +
> + strbuf_release(&sb);
> +}
> +
> +/**
> + * Remove the am_state directory.
> + */
> +static void am_destroy(const struct am_state *state)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_addstr(&sb, state->dir.buf);
> + remove_dir_recursively(&sb, 0);
> + strbuf_release(&sb);
> +}
> +
> +/**
> + * Setup a new am session for applying patches
> + */
> +static void am_setup(struct am_state *state)
> +{
> + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
> + die_errno(_("failed to create directory '%s'"),
> state->dir.
[PATCH/WIP v2 04/19] am: implement patch queue mechanism
git-am applies a series of patches. If the process terminates
abnormally, we want to be able to resume applying the series of patches.
This requires the session state to be saved in a persistent location.
Implement the mechanism of a "patch queue", represented by 2 integers --
the index of the current patch we are applying and the index of the last
patch, as well as its lifecycle through the following functions:
* am_setup(), which will set up the state directory
$GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
the last-known state will still persist.
* am_load(), which is called if there is an am session in
progress, to load the last known state from the state directory so we
can resume applying patches.
* am_run(), which will do the actual patch application. After applying a
patch, it calls am_next() to increment the current patch index. The
logic for applying and committing a patch is not implemented yet.
* am_destroy(), which is finally called when we successfully applied all
the patches in the queue, to clean up by removing the state directory
and its contents.
Signed-off-by: Paul Tan
---
Notes:
v2
* Declare struct am_state as static
builtin/am.c | 164 +++
1 file changed, 164 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index 0ccbe33..f061d21 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,6 +6,154 @@
#include "cache.h"
#include "builtin.h"
#include "exec_cmd.h"
+#include "parse-options.h"
+#include "dir.h"
+
+struct am_state {
+ /* state directory path */
+ struct strbuf dir;
+
+ /* current and last patch numbers, 1-indexed */
+ int cur;
+ int last;
+};
+
+/**
+ * Initializes am_state with the default values.
+ */
+static void am_state_init(struct am_state *state)
+{
+ memset(state, 0, sizeof(*state));
+
+ strbuf_init(&state->dir, 0);
+}
+
+/**
+ * Release memory allocated by an am_state.
+ */
+static void am_state_release(struct am_state *state)
+{
+ strbuf_release(&state->dir);
+}
+
+/**
+ * Returns path relative to the am_state directory.
+ */
+static inline const char *am_path(const struct am_state *state, const char
*path)
+{
+ return mkpath("%s/%s", state->dir.buf, path);
+}
+
+/**
+ * Returns 1 if there is an am session in progress, 0 otherwise.
+ */
+static int am_in_progress(const struct am_state *state)
+{
+ struct stat st;
+
+ if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode))
+ return 0;
+ return 1;
+}
+
+/**
+ * Reads the contents of `file`. The third argument can be used to give a hint
+ * about the file size, to avoid reallocs. Returns number of bytes read on
+ * success, -1 if the file does not exist. If trim is set, trailing whitespace
+ * will be removed from the file contents.
+ */
+static int read_state_file(struct strbuf *sb, const char *file, size_t hint,
int trim)
+{
+ strbuf_reset(sb);
+ if (strbuf_read_file(sb, file, hint) >= 0) {
+ if (trim)
+ strbuf_rtrim(sb);
+
+ return sb->len;
+ }
+
+ if (errno == ENOENT)
+ return -1;
+
+ die_errno(_("could not read '%s'"), file);
+}
+
+/**
+ * Loads state from disk.
+ */
+static void am_load(struct am_state *state)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ read_state_file(&sb, am_path(state, "next"), 8, 1);
+ state->cur = strtol(sb.buf, NULL, 10);
+
+ read_state_file(&sb, am_path(state, "last"), 8, 1);
+ state->last = strtol(sb.buf, NULL, 10);
+
+ strbuf_release(&sb);
+}
+
+/**
+ * Remove the am_state directory.
+ */
+static void am_destroy(const struct am_state *state)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ strbuf_addstr(&sb, state->dir.buf);
+ remove_dir_recursively(&sb, 0);
+ strbuf_release(&sb);
+}
+
+/**
+ * Setup a new am session for applying patches
+ */
+static void am_setup(struct am_state *state)
+{
+ if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
+ die_errno(_("failed to create directory '%s'"), state->dir.buf);
+
+ write_file(am_path(state, "next"), 1, "%d", state->cur);
+
+ write_file(am_path(state, "last"), 1, "%d", state->last);
+}
+
+/**
+ * Increments the patch pointer, and cleans am_state for the application of the
+ * next patch.
+ */
+static void am_next(struct am_state *state)
+{
+ state->cur++;
+ write_file(am_path(state, "next"), 1, "%d", state->cur);
+}
+
+/**
+ * Applies all queued patches.
+ */
+static void am_run(struct am_state *state)
+{
+ while (state->cur <= state->last)
+ am_next(state);
+
+ am_destroy(state);
+}
+
+static struct am_state state;
+
+static c
