This comment in builtin/repack.c:
First see if there are packs of the same name and if so
if we can move them out of the way (this can happen if we
repacked immediately after packing fully).
When a repo was fully repacked, and is repacked again, we may run
into the situation that "new" packfiles have the same name as
already existing ones (traditionally packfiles have been named after
the list of names of objects in them, so repacking all the objects
in a single pack would have produced a packfile with the same name).
The logic is to rename the existing ones into filename like
"old-XXX", create the new ones and then remove the "old-" ones.
When something went wrong in the middle, this sequence is rolled
back by renaming the "old-" files back.
The renaming into "old-" did not work as designed, because
file_exists() was done on the wrong file name. This could give
problems for a repo on a network share under Windows, as reported by
Jochen Haag <zwanzig12@xxxxxxxxxxxxxx>.
Formulate the filenames objects/pack/pack-XXXX.{pack,idx} correctly
(the code originally lacked "pack-" prefix when checking if the file
exists).
Also rename the variables to match what they are used for:
fname for the final name of the new packfile, fname_old for the name
of the existing one, and fname_tmp for the temporary name pack-objects
created the new packfile in.
Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
* Somehow this came to my private mailbox without Cc to list, so I
am forwarding it.
I think with 1190a1ac (pack-objects: name pack files after
trailer hash, 2013-12-05), repacking the same set of objects may
have less chance of producing colliding names, especially if you
are on a box with more than one core, but it still would be a
good idea to get this part right in the upcoming release.
The bug in the first hunk will cause us not to find any existing
file, even when there is a name clash. And then we try to
immediately the final rename without any provision for rolling
back. The other two hunks are pure noise renaming variables.
I think the patch looks correct, but I'd appreciate a second set
of eyeballs.
Thanks.
builtin/repack.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index bca7710..3b01353 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -260,7 +260,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
for_each_string_list_item(item, &names) {
for (ext = 0; ext < 2; ext++) {
char *fname, *fname_old;
- fname = mkpathdup("%s/%s%s", packdir,
+ fname = mkpathdup("%s/pack-%s%s", packdir,
item->string, exts[ext]);
if (!file_exists(fname)) {
free(fname);
@@ -316,33 +316,33 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
/* Now the ones with the same name are out of the way... */
for_each_string_list_item(item, &names) {
for (ext = 0; ext < 2; ext++) {
- char *fname, *fname_old;
+ char *fname, *fname_tmp;
struct stat statbuffer;
fname = mkpathdup("%s/pack-%s%s",
packdir, item->string, exts[ext]);
- fname_old = mkpathdup("%s-%s%s",
+ fname_tmp = mkpathdup("%s-%s%s",
packtmp, item->string, exts[ext]);
- if (!stat(fname_old, &statbuffer)) {
+ if (!stat(fname_tmp, &statbuffer)) {
statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
- chmod(fname_old, statbuffer.st_mode);
+ chmod(fname_tmp, statbuffer.st_mode);
}
- if (rename(fname_old, fname))
- die_errno(_("renaming '%s' failed"), fname_old);
+ if (rename(fname_tmp, fname))
+ die_errno(_("renaming '%s' into '%s' failed"), fname_tmp, fname);
free(fname);
- free(fname_old);
+ free(fname_tmp);
}
}
/* Remove the "old-" files */
for_each_string_list_item(item, &names) {
for (ext = 0; ext < 2; ext++) {
- char *fname;
- fname = mkpath("%s/old-pack-%s%s",
+ char *fname_old;
+ fname_old = mkpath("%s/old-%s%s",
packdir,
item->string,
exts[ext]);
- if (remove_path(fname))
- warning(_("removing '%s' failed"), fname);
+ if (remove_path(fname_old))
+ warning(_("removing '%s' failed"), fname_old);
}
}
--
1.9-rc2-217-g24a8b2e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html