Re: [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save()

On Wed, Mar 16, 2016 at 5:04 AM, Paul Tan [off-list ref] wrote:

quoted

How is this specific to the state file? All it does is create the
leading directory
if it doesn't exist? (So I'd expect file_exists(concat(dir, file)) to
have the same
result without actually creating the directory if it doesn't exist as
a side effect?

I don't quite understand, AFAIK mkpath() does not create any
directories as a side-effect. And yes, I just wanted a short way to
say file_exists(concat(dir, file)) or file_exists(mkpath("%s/%s", dir,
file)) without cluttering up the code.

My bad. I should not assume functions doing stuff as their name might suggest.
(It "makes the path" but only in terms of creating the right string, not on the
file system, where you'd use functions like safe_create_leading_directories.
I thought all that was implied in mkpath).

quoted

If the dir doesn't exist it can be created in rebase_options_load explicitly?

I don't intend to create any directories if they do not exist.

quoted

quoted

+
+static int read_state_file(struct strbuf *sb, const char *dir, const char *file)
+{
+       const char *path = mkpath("%s/%s", dir, file);
+       strbuf_reset(sb);
+       if (strbuf_read_file(sb, path, 0) >= 0)
+               return sb->len;
+       else
+               return error(_("could not read '%s'"), path);
+}
+
+int rebase_options_load(struct rebase_options *opts, const char *dir)
+{
+       struct strbuf sb = STRBUF_INIT;
+       const char *filename;
+
+       /* opts->orig_refname */
+       if (read_state_file(&sb, dir, "head-name") < 0)
+               return -1;
+       strbuf_trim(&sb);
+       if (starts_with(sb.buf, "refs/heads/"))
+               opts->orig_refname = strbuf_detach(&sb, NULL);
+       else if (!strcmp(sb.buf, "detached HEAD"))
+               opts->orig_refname = NULL;
+       else
+               return error(_("could not parse %s"), mkpath("%s/%s", dir, "head-name"));
+
+       /* opts->onto */
+       if (read_state_file(&sb, dir, "onto") < 0)
+               return -1;
+       strbuf_trim(&sb);
+       if (get_oid_hex(sb.buf, &opts->onto) < 0)
+               return error(_("could not parse %s"), mkpath("%s/%s", dir, "onto"));
+
+       /*
+        * We always write to orig-head, but interactive rebase used to write
+        * to head. Fall back to reading from head to cover for the case that
+        * the user upgraded git with an ongoing interactive rebase.
+        */
+       filename = state_file_exists(dir, "orig-head") ? "orig-head" : "head";
+       if (read_state_file(&sb, dir, filename) < 0)
+               return -1;

So from here on we always use "orig-head" instead of "head" for
interactive rebase.
Would people ever rely on the (internal) file name and have e.g.
scripts which operate
on the "head" file ?

This backwards-compatibility code is just a straight port from the
code in git-rebase.sh.

The usage of orig-head has been around since 2011 with 84df456
(rebase: extract code for writing basic state, 2011-02-06), so I guess
if people had issues with it, it would have been reported.

I did not read the rebase shell code, but commented on the C code only.
If this is already in there, let's keep it.
Sorry for the confusion.

Thanks,
Stefan