Re: init --shared=0666 isn't
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:24
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · init --shared=0666 isn't · Steve Folly <hidden>
Steve Folly [off-list ref] writes:
$ git --git-dir=/tmp/shared.git fetch origin error: cannot open /tmp/shared.git/FETCH_HEAD: Permission denied FETCH_HEAD is owned by the original user, with 0644 permissions,
The shared repository support was [*1*] designed to help people setting up
a "shared" repository like CVS/SVN, a central place people meet and
exchange their histories by pushing into it and fetching from it. It was
never designed to be used with a repository with a working tree, hence it
was never designed to be used in repositories you would run "git pull" or
"git merge" in.
However, it is not implausible for a sane workflow that you have to fetch
into a central meeting place. You might be setting up a shared repository
that also serves as a redistribution center for an external source, and in
such a set-up, you would
(1) clone/fetch from external "upstream";
(2) allow people to fetch from it;
(3) allow people to push into it.
So in that sense, allowing "git fetch" in such a shared central meeting
place is not something we would want to actively forbid. But I thought we
unlinked existing FETCH_HEAD and then opened to create it anew, so I am a
bit puzzled why it matters who owns it and who can write into it.
... goes and looks ...
Ah, truncate_fetch_head() just opens it without unlinking. Probably a
better fix might be to unlink and create the file, like the attached patch
does, instead of running adjust_shared_perm().
But, I wonder if there are other files written that will need fixing also - ORIG_HEAD, MERGE_HEAD?
These are created by "merge" and "reset", so it shouldn't matter. By the way, the same applies to files in the working tree. We never call adjust_shared_perm() when creating them, and I do not think this is likely to change. [Footnote] *1* Notice the past tense---this is merely a statement of historical fact to help you understand _why_ the current system behaves that way and the conclusion does not necessarily have to be "hence it must remain that way".
diff --git a/builtin-fetch.c b/builtin-fetch.c
index b6c5b34..d73fa19 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c@@ -654,8 +654,11 @@ static void check_not_current_branch(struct ref *ref_map) static int truncate_fetch_head(void) { char *filename = git_path("FETCH_HEAD"); - FILE *fp = fopen(filename, "w"); + FILE *fp; + if (unlink(filename) && errno != ENOENT) + return error("cannot unlink %s\n", filename); + fp = fopen(filename, "w"); if (!fp) return error("cannot open %s: %s\n", filename, strerror(errno)); fclose(fp);