On Thursday 26 March 2009, Junio C Hamano wrote:
> To fix the loose object codepath, the earlier patch added a call to
> adjust_shared_perm() to write_loose_object() function, but after looking
> at your 7th patch, I realized that the pattern of file creation inside
> $GIT_DIR typically is to first create a temporary file, write to it, and
> then finish it off by calling move_temp_to_file(). The true purpose of
> the function is to "finalize the file being created", and it is misnamed
> in that it describes how its implementation does it currently (i.e. by
> renaming the temporary file to its final name), but it makes perfect
> sense to call adjust_shared_perm() inside it as a part of finalization.
> I think this should cover the codepaths your 7th patch fixed without
> actually touching them.
Yes, with one exception:
For the two cases index-pack.c, the chmod(foo, 0444) happens AFTER the
corresponding call to move_temp_to_file(xyzzy, foo). The chmod() in
adjust_shared_perms() would thus be overridden by the chmod(foo, 0444),
which is not what we want. In both cases, I think the chmod(foo, 0444)
can safely be moved up above the call to move_temp_to_file(). Something
like this (although I'm not sure about the semantics of 'from_stdin'):
diff --git a/index-pack.c b/index-pack.c
index 7546822..d289b6a 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -815,6 +815,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
}
}
+ if (from_stdin)
+ chmod(final_pack_name, 0444);
if (final_pack_name != curr_pack_name) {
if (!final_pack_name) {
snprintf(name, sizeof(name), "%s/pack/pack-%s.pack",
@@ -824,9 +826,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
if (move_temp_to_file(curr_pack_name, final_pack_name))
die("cannot store pack file");
}
- if (from_stdin)
- chmod(final_pack_name, 0444);
+ chmod(final_index_name, 0444);
if (final_index_name != curr_index_name) {
if (!final_index_name) {
snprintf(name, sizeof(name), "%s/pack/pack-%s.idx",
@@ -836,7 +837,6 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
if (move_temp_to_file(curr_index_name, final_index_name))
die("cannot store index file");
}
- chmod(final_index_name, 0444);
if (!from_stdin) {
printf("%s\n", sha1_to_hex(sha1));
> Could you eyeball and re-test it?
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
Tested-by: Johan Herland <johan@xxxxxxxxxxx>
> --- a/builtin-init-db.c
> +++ b/builtin-init-db.c
> @@ -195,6 +195,8 @@ static int create_default_files(const char
> *template_path)
>
> git_config(git_default_config, NULL);
> is_bare_repository_cfg = init_is_bare_repository;
> +
> + /* reading existing config may have overwrote it */
s/overwrote/overwritten/
Otherwise OK, AFAICS.
Have fun! :)
...Johan
--
Johan Herland, <johan@xxxxxxxxxxx>
www.herland.net
--
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
