Re: [PATCH] repack: rewrite the shell script in C (squashing proposal)
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:29
Stefan Beller [off-list ref] writes:
quoted hunk
@@ -41,18 +35,16 @@ static void remove_temporary_files(void) DIR *dir; struct dirent *e; + dir = opendir(packdir); + if (!dir) return; + strbuf_addstr(&buf, packdir); + + /* dirlen holds the length of the path before the file name */ dirlen = buf.len + 1; + strbuf_addf(&buf, "%s", packtmp); + /* prefixlen holds the length of the prefix */
Thanks to the name of the variable that is self-describing, this
comment does not add much value.
But it misses the whole point of my suggestion in the earlier
message to phrase these like so:
/* Point at the slash at the end of ".../objects/pack/" */
dirlen = strlen(packdir) + 1;
/* Point at the dash at the end of ".../.tmp-%d-pack-" */
prefixlen = buf.len - dirlen;
to clarify what the writer considers as "the prefix" is, which may
be quite different from what the readers think "the prefix" is. In
".tmp-2342-pack-0d8beaa5b76e824c9869f0d1f1b19ec7acf4982f.pack", is
the prefix ".tmp-2342-", ".tmp-2342-pack", or ".tmp-2342-pack-"?
int cmd_repack(int argc, const char **argv, const char *prefix)
{
...
packdir = mkpathdup("%s/pack", get_object_directory());
packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
+ sigchain_push_common(remove_pack_on_signal);
+
argv_array_push(&cmd_args, "pack-objects");
argv_array_push(&cmd_args, "--keep-true-parents");
argv_array_push(&cmd_args, "--honor-pack-keep");
...
+ rollback_failure.items[i].string,
+ rollback_failure.items[i].string);
}
exit(1);
}
The scripted version uses
trap 'rm -f "$PACKTMP"-*' 0 1 2 3 15
so remove_temporary_files() needs to be called before exiting from
the program without getting killed by a signal.
Thanks.