[PATCH] am: terminate state files with a newline
- From
Paul Tan <pyokagan@gmail.com>
- Date
- Aug 23, 2015, 05:50 UTC
- Message-ID
- <20150823055053.GA15849@yoshi.chippynet.com>
- In-Reply-To
- <xmqqa8tl7qi3.fsf@gitster.dls.corp.google.com>
On Thu, Aug 20, 2015 at 11:40:20AM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> SZEDER Gábor <szeder@ira.uka.de> writes:
>
> > The format of the files '.git/rebase-apply/{next,last}' changed slightly
> > with the recent builtin 'git am' conversion: while these files were
> > newline-terminated when written by the scripted version, the ones written
> > by the builtin are not.
>
> Thanks for noticing; that should be corrected, I think.Okay then, this patch should correct this.
Did we ever explictly allow external programs to poke around the contents of the .git/rebase-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/ .
Regards, Paul
-- >8 -- Subject: [PATCH] am: terminate state files with a newline
Since builtin/am.c replaced git-am.sh in 783d7e8 (builtin-am: remove redirection to git-am.sh, 2015-08-04), the state files written by git-am did not terminate with a newline.
This is because the code in builtin/am.c did not write the newline to the state files.
While the git codebase has no problems with the missing newline, external software which read the contents of the state directory may be strict about the existence of the terminating newline, and would thus break.
Fix this by correcting the relevant calls to write_file() to ensure that the state files written terminate with a newline, matching how git-am.sh behaves.
While we are fixing the write_file() calls, fix the writing of the "dirtyindex" file as well -- we should be creating an empty file to match the behavior of git-am.sh.
Reported-by: SZEDER Gábor <szeder@ira.uka.de> Signed-off-by: Paul Tan <pyokagan@gmail.com> --- builtin/am.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c index 1399c8d..2e57fad 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -994,13 +994,13 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, if (state->rebasing) state->threeway = 1; - write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f"); + write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t" : "f"); - write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f"); + write_file(am_path(state, "quiet"), 1, "%s\n", state->quiet ? "t" : "f"); - write_file(am_path(state, "sign"), 1, state->signoff ? "t" : "f"); + write_file(am_path(state, "sign"), 1, "%s\n", state->signoff ? "t" : "f"); - write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f"); + write_file(am_path(state, "utf8"), 1, "%s\n", state->utf8 ? "t" : "f"); switch (state->keep) { case KEEP_FALSE: @@ -1016,9 +1016,9 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die("BUG: invalid value for state->keep"); } - write_file(am_path(state, "keep"), 1, "%s", str); + write_file(am_path(state, "keep"), 1, "%s\n", str); - write_file(am_path(state, "messageid"), 1, state->message_id ? "t" : "f"); + write_file(am_path(state, "messageid"), 1, "%s\n", state->message_id ? "t" : "f"); switch (state->scissors) { case SCISSORS_UNSET: @@ -1034,10 +1034,10 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, die("BUG: invalid value for state->scissors"); } - write_file(am_path(state, "scissors"), 1, "%s", str); + write_file(am_path(state, "scissors"), 1, "%s\n", str); sq_quote_argv(&sb, state->git_apply_opts.argv, 0); - write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf); + write_file(am_path(state, "apply-opt"), 1, "%s\n", sb.buf); if (state->rebasing) write_file(am_path(state, "rebasing"), 1, "%s", ""); @@ -1045,7 +1045,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, write_file(am_path(state, "applying"), 1, "%s", ""); if (!get_sha1("HEAD", curr_head)) { - write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(curr_head)); + write_file(am_path(state, "abort-safety"), 1, "%s\n", sha1_to_hex(curr_head)); if (!state->rebasing) update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, UPDATE_REFS_DIE_ON_ERR); @@ -1060,9 +1060,9 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, * session is in progress, they should be written last. */ - write_file(am_path(state, "next"), 1, "%d", state->cur); + write_file(am_path(state, "next"), 1, "%d\n", state->cur); - write_file(am_path(state, "last"), 1, "%d", state->last); + write_file(am_path(state, "last"), 1, "%d\n", state->last); strbuf_release(&sb); } @@ -1095,12 +1095,12 @@ static void am_next(struct am_state *state) unlink(am_path(state, "original-commit")); if (!get_sha1("HEAD", head)) - write_file(am_path(state, "abort-safety"), 1, "%s", sha1_to_hex(head)); + write_file(am_path(state, "abort-safety"), 1, "%s\n", sha1_to_hex(head)); else write_file(am_path(state, "abort-safety"), 1, "%s", ""); state->cur++; - write_file(am_path(state, "next"), 1, "%d", state->cur); + write_file(am_path(state, "next"), 1, "%d\n", state->cur); } /** @@ -1461,7 +1461,7 @@ static int parse_mail_rebase(struct am_state *state, const char *mail) write_commit_patch(state, commit); hashcpy(state->orig_commit, commit_sha1); - write_file(am_path(state, "original-commit"), 1, "%s", + write_file(am_path(state, "original-commit"), 1, "%s\n", sha1_to_hex(commit_sha1)); return 0; @@ -1764,7 +1764,7 @@ static void am_run(struct am_state *state, int resume) refresh_and_write_cache(); if (index_has_changes(&sb)) { - write_file(am_path(state, "dirtyindex"), 1, "t"); + write_file(am_path(state, "dirtyindex"), 1, "%s", ""); die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf); }
-- 2.5.0.400.gff86faf.dirty