Re: [PATCH v2 02/10] sequencer: introduce new commands to reset the revision
From: Eric Sunshine <hidden>
Date: 2018-01-30 08:06:25
On Mon, Jan 29, 2018 at 5:54 PM, Johannes Schindelin [off-list ref] wrote:
quoted hunk ↗ jump to hunk
[...] This commit implements the commands to label, and to reset to, given revisions. The syntax is: label <name> reset <name> [...] Signed-off-by: Johannes Schindelin <redacted> ---diff --git a/sequencer.c b/sequencer.c@@ -1253,7 +1266,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) if (skip_prefix(bol, todo_command_info[i].str, &bol)) { item->command = i; break; - } else if (bol[1] == ' ' && *bol == todo_command_info[i].c) { + } else if ((bol + 1 == eol || bol[1] == ' ') && + *bol == todo_command_info[i].c) {
This adds support for commands which have no arguments, however, now that the "bud" command has been retired, this can go away too, right?
quoted hunk ↗ jump to hunk
bol++; item->command = i; break;@@ -1919,6 +1934,144 @@ static int do_exec(const char *command_line) +static int safe_append(const char *filename, const char *fmt, ...) +{ + va_list ap; + struct lock_file lock = LOCK_INIT; + int fd = hold_lock_file_for_update(&lock, filename, 0); + struct strbuf buf = STRBUF_INIT; + + if (fd < 0) + return error_errno(_("could not lock '%s'"), filename);
Minor: unable_to_lock_message() can provide a more detailed explanation of the failure.
+
+ if (strbuf_read_file(&buf, filename, 0) < 0 && errno != ENOENT)
+ return error_errno(_("could not read '%s'"), filename);
+ strbuf_complete(&buf, '\n');
+ va_start(ap, fmt);
+ strbuf_vaddf(&buf, fmt, ap);
+ va_end(ap);
Would it make sense to also
strbuf_complete(&buf, '\n')
here, as well, to be a bit more robust against lazy callers?
+
+ if (write_in_full(fd, buf.buf, buf.len) < 0) {
+ rollback_lock_file(&lock);
+ return error_errno(_("could not write to '%s'"), filename);Reading lockfile.h & tempfile.c, I see that rollback_lock_file() clobbers write_in_full()'s errno before error_errno() is called.
+ }
+ if (commit_lock_file(&lock) < 0) {
+ rollback_lock_file(&lock);
+ return error(_("failed to finalize '%s'"), filename);
+ }
+
+ return 0;
+}
+
+static int do_reset(const char *name, int len)
+{
+ [...]
+ strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
+ if (get_oid(ref_name.buf, &oid) &&
+ get_oid(ref_name.buf + strlen("refs/rewritten/"), &oid)) {
+ error(_("could not read '%s'"), ref_name.buf);
Checking my understanding: The two get_oid() calls allow the argument
to 'reset' to be a label created with the 'label' command or any other
way to name an object, right? If so, then I wonder if the error
invocation should instead be:
error(_("could not read '%.*s'"), len, name);
+ rollback_lock_file(&lock); + strbuf_release(&ref_name); + return -1; + }