* Re: [PATCH v2] pull: Document the "--[no-]recurse-submodules" options
From: Junio C Hamano @ 2011-02-07 21:42 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Jonathan Nieder, Johannes Sixt, Git Mailing List
In-Reply-To: <4D5047BD.6030304@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 695696d..ab0dbfc 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -64,11 +64,11 @@ ifndef::git-pull[]
> downloaded. The default behavior for a remote may be
> specified with the remote.<name>.tagopt setting. See
> linkgit:git-config[1].
> -endif::git-pull[]
>
> --[no-]recurse-submodules::
> This option controls if new commits of all populated submodules should
> be fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).
> +endif::git-pull[]
>
> ifndef::git-pull[]
> --submodule-prefix=<path>::
Hmph, why not enclose the three of them inside a single ifndef here?
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 3046691..b33e6be 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -84,6 +84,15 @@ must be given before the options meant for 'git fetch'.
> --verbose::
> Pass --verbose to git-fetch and git-merge.
>
> +--[no-]recurse-submodules::
> + This option controls if new commits of all populated submodules should
> + be fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).
> + That might be necessary to get the data needed for merging submodule
> + commits, a feature git learned in 1.7.3. Notice that the result of a
> + merge will not be checked out in the submodule, "git submodule update"
> + has to be called afterwards to bring the work tree up to date with the
> + merge result.
Ok.
^ permalink raw reply
* Re: [1.8.0] git checkout refs/heads/foo checks out branch foo
From: Sverre Rabbelier @ 2011-02-07 21:36 UTC (permalink / raw)
To: Jeff King; +Cc: Martin von Zweigbergk, git
In-Reply-To: <20110207205934.GD13461@sigill.intra.peff.net>
Heya,
On Mon, Feb 7, 2011 at 21:59, Jeff King <peff@peff.net> wrote:
> As cool and clever as the foo^0 behavior is once you understand it, I
> think it is a horribly confusing thing for non-experts. As part of this
> proposal, should we perhaps offer "git checkout --detach" as the
> easy-on-the-eyes way of intentionally detaching?
Now _that_ is an excellent usability improvement, assuming we want to
encourage detaching HEAD... do we?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH v2] commit: fix memory-leak
From: Erik Faye-Lund @ 2011-02-07 21:31 UTC (permalink / raw)
To: git; +Cc: matthieu.moy, msysgit, blees
In-Reply-To: <AANLkTikr2+OVRU6n+0tA752_x80ir9dQh65RjUp3BxPR@mail.gmail.com>
On Mon, Feb 7, 2011 at 10:12 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Mon, Feb 7, 2011 at 9:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> The name, email and date strings are some times allocated on the
>> heap, but not free'd. Fix this by making sure they are allways
>> heap-allocated, so we can safely free the memory.
>>
>> At the same time, this fixes a problem with strict-POSIX getenv
>> implementations. POSIX says "The return value from getenv() may
>> point to static data which may be overwritten by subsequent calls
>> to getenv()", so not duplicating the strings is a potential bug.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>> Fixed typo in commit message, as pointed out by Matthieu Moy.
>>
>> builtin/commit.c | 9 ++++++---
>> git-compat-util.h | 1 +
>> wrapper.c | 6 ++++++
>> 3 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 03cff5a..e5a649e 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -465,9 +465,9 @@ static void determine_author_info(struct strbuf *author_ident)
>> {
>> char *name, *email, *date;
>>
>> - name = getenv("GIT_AUTHOR_NAME");
>> - email = getenv("GIT_AUTHOR_EMAIL");
>> - date = getenv("GIT_AUTHOR_DATE");
>> + name = xgetenv("GIT_AUTHOR_NAME");
>> + email = xgetenv("GIT_AUTHOR_EMAIL");
>> + date = xgetenv("GIT_AUTHOR_DATE");
>>
>> if (use_message && !renew_authorship) {
>> const char *a, *lb, *rb, *eol;
>> @@ -507,6 +507,9 @@ static void determine_author_info(struct strbuf *author_ident)
>> date = force_date;
>> strbuf_addstr(author_ident, fmt_ident(name, email, date,
>> IDENT_ERROR_ON_NO_NAME));
>> + free(name);
>> + free(email);
>> + free(date);
>
> Hmm, but I'm getting a crash here on Linux. Guess I need to debug a bit...
>
Ah, it was the force_date-assignment:
---8<---
diff --git a/builtin/commit.c b/builtin/commit.c
index e5a649e..1416c13 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -504,7 +504,7 @@ static void determine_author_info(struct strbuf
*author_ident)
}
if (force_date)
- date = force_date;
+ date = xstrdup(force_date);
strbuf_addstr(author_ident, fmt_ident(name, email, date,
IDENT_ERROR_ON_NO_NAME));
free(name);
---8<---
But now I see that I was temporarily(?) struck with insanity:
overwriting a heap-allocated pointer with another heap-allocated
pointer doesn't fix a memory-leak. So let's add some more calls to
free:
diff --git a/builtin/commit.c b/builtin/commit.c
index e5a649e..bdd0cfb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -482,6 +482,10 @@ static void determine_author_info(struct strbuf
*author_ident)
if (!*lb || !*rb || !*eol)
die("invalid commit: %s", use_message);
+ free(name);
+ free(email);
+ free(date);
+
if (lb == a + strlen("\nauthor "))
/* \nauthor <foo@example.com> */
name = xcalloc(1, 1);
@@ -497,14 +501,19 @@ static void determine_author_info(struct strbuf
*author_ident)
const char *lb = strstr(force_author, " <");
const char *rb = strchr(force_author, '>');
+ free(name);
+ free(email);
+
if (!lb || !rb)
die("malformed --author parameter");
name = xstrndup(force_author, lb - force_author);
email = xstrndup(lb + 2, rb - (lb + 2));
}
- if (force_date)
- date = force_date;
+ if (force_date) {
+ free(date);
+ date = xstrdup(force_date);
+ }
strbuf_addstr(author_ident, fmt_ident(name, email, date,
IDENT_ERROR_ON_NO_NAME));
free(name);
^ permalink raw reply related
* Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Erik Faye-Lund @ 2011-02-07 21:23 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207211844.GH63976@book.hvoigt.net>
On Mon, Feb 7, 2011 at 10:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Mon, Feb 07, 2011 at 10:07:10PM +0100, Erik Faye-Lund wrote:
>> On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> >
>> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
>> > directory is busy, we only want to retry deleting the directory if it
>> > is empty, so test specifically for that case and set ENOTEMPTY rather
>> > than EACCES.
>> >
>>
>> I'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with
>> errno=ENOTEMPTY if a directory isn't empty:
>
> I think Johannes was referring to the case when a directory is busy.
> E.g. a process is running that has its working directory inside that
> directory. In that case ENOTEMPTY was not returned even though the
> directory is not empty. Thats what I read from the patch.
>
I don't think that's the case either:
$ echo "int main() { while (1); }" | gcc -x c - -o foo/bin.exe
[kusma@HUE-PC:/git@work/xgetenv]
$ foo/bin.exe &
[2] 3188
[kusma@HUE-PC:/git@work/xgetenv]
$ ./a.exe
rmdir: Directory not empty
errno: 41 (expected 41)
^ permalink raw reply
* Re: [msysGit] Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Heiko Voigt @ 2011-02-07 21:18 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Junio C Hamano, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <AANLkTin6A-HVKvM9_5ggMezpM--tt1qUwSXF+CEaXg7J@mail.gmail.com>
On Mon, Feb 07, 2011 at 10:07:10PM +0100, Erik Faye-Lund wrote:
> On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
> > directory is busy, we only want to retry deleting the directory if it
> > is empty, so test specifically for that case and set ENOTEMPTY rather
> > than EACCES.
> >
>
> I'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with
> errno=ENOTEMPTY if a directory isn't empty:
I think Johannes was referring to the case when a directory is busy.
E.g. a process is running that has its working directory inside that
directory. In that case ENOTEMPTY was not returned even though the
directory is not empty. Thats what I read from the patch.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH v2] commit: fix memory-leak
From: Erik Faye-Lund @ 2011-02-07 21:12 UTC (permalink / raw)
To: git; +Cc: matthieu.moy, msysgit, blees
In-Reply-To: <1297110111-7620-1-git-send-email-kusmabite@gmail.com>
On Mon, Feb 7, 2011 at 9:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> The name, email and date strings are some times allocated on the
> heap, but not free'd. Fix this by making sure they are allways
> heap-allocated, so we can safely free the memory.
>
> At the same time, this fixes a problem with strict-POSIX getenv
> implementations. POSIX says "The return value from getenv() may
> point to static data which may be overwritten by subsequent calls
> to getenv()", so not duplicating the strings is a potential bug.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
> Fixed typo in commit message, as pointed out by Matthieu Moy.
>
> builtin/commit.c | 9 ++++++---
> git-compat-util.h | 1 +
> wrapper.c | 6 ++++++
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 03cff5a..e5a649e 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -465,9 +465,9 @@ static void determine_author_info(struct strbuf *author_ident)
> {
> char *name, *email, *date;
>
> - name = getenv("GIT_AUTHOR_NAME");
> - email = getenv("GIT_AUTHOR_EMAIL");
> - date = getenv("GIT_AUTHOR_DATE");
> + name = xgetenv("GIT_AUTHOR_NAME");
> + email = xgetenv("GIT_AUTHOR_EMAIL");
> + date = xgetenv("GIT_AUTHOR_DATE");
>
> if (use_message && !renew_authorship) {
> const char *a, *lb, *rb, *eol;
> @@ -507,6 +507,9 @@ static void determine_author_info(struct strbuf *author_ident)
> date = force_date;
> strbuf_addstr(author_ident, fmt_ident(name, email, date,
> IDENT_ERROR_ON_NO_NAME));
> + free(name);
> + free(email);
> + free(date);
Hmm, but I'm getting a crash here on Linux. Guess I need to debug a bit...
^ permalink raw reply
* Re: [1.8.0] git checkout refs/heads/foo checks out branch foo
From: Heiko Voigt @ 2011-02-07 21:11 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <alpine.DEB.1.10.1102062234010.3788@debian>
Hallo Martin,
On Mon, Feb 07, 2011 at 06:01:51AM -0500, Martin von Zweigbergk wrote:
> Proposal:
>
> 'git checkout refs/heads/foo' (or 'git checkout heads/foo' for that
> matter) does not check out the branch, but instead detaches HEAD at
> foo. This is quite counter-intuitive (at least to me) and the same
> functionality can be achieved by using e.g. foo~0. Change the behavior
> so that the branch is actually checked out. This also applies to
> e.g. 'git rebase master refs/heads/topic', which currently rebases a
> detached HEAD. There are probably other examples as well that I'm not
> aware of.
Just to clarify: You are not proposing that 'git checkout origin/master'
would also not checkout to a detached head, right? Because that is a
feature I am using frequently to test branches that have been pushed by
another developer to a remote server. If that would create a new local
branch that would be confusing.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Erik Faye-Lund @ 2011-02-07 21:07 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207205400.GF63976@book.hvoigt.net>
On Mon, Feb 7, 2011 at 9:54 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
> directory is busy, we only want to retry deleting the directory if it
> is empty, so test specifically for that case and set ENOTEMPTY rather
> than EACCES.
>
I'm sorry, I don't quite understand. rmdir on Windows/MinGW fails with
errno=ENOTEMPTY if a directory isn't empty:
---8<---
#include <errno.h>
#include <stdlib.h>
#include <stdio.h>
int main()
{
if (mkdir("foo") < 0 && errno != EEXIST) {
perror("mkdir");
exit(-1);
}
if (!fopen("foo/bar", "w")) {
perror("fopen");
exit(-1);
}
if (rmdir("foo") < 0)
perror("rmdir");
printf("errno: %d (expected %d)\n", errno, ENOTEMPTY);
}
---8<---
$ ./a.exe
rmdir: Directory not empty
errno: 41 (expected 41)
^ permalink raw reply
* Re: "git add -u" broken in git 1.7.4?
From: Jeff King @ 2011-02-07 21:02 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, Sebastian Pipping, Git ML
In-Reply-To: <vpqtygfa7g8.fsf@bauges.imag.fr>
On Mon, Feb 07, 2011 at 09:57:43PM +0100, Matthieu Moy wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I would say so; "add -p" was an ill-executed afterthought. The codepath
> > was originally meant to be used from "-i" as the top-level interface that
> > was a fully interactive way to prepare for the next commit, which is an
> > operation that is inherently full-tree.
>
> I agree that "git add -i" is a way to prepare the next commit, but you
> seem to imply that "git add -u" is not and then I have to disagree.
How about "git commit -a"? Shouldn't that be more or less equivalent to
"git add -u && git commit"? But it is full-tree.
So I think there is less "we do it one way, and this is the outlier" and
more "we are horribly inconsistent".
-Peff
^ permalink raw reply
* Re: [1.8.0] git checkout refs/heads/foo checks out branch foo
From: Jeff King @ 2011-02-07 20:59 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <alpine.DEB.1.10.1102062234010.3788@debian>
On Mon, Feb 07, 2011 at 06:01:51AM -0500, Martin von Zweigbergk wrote:
> 'git checkout refs/heads/foo' (or 'git checkout heads/foo' for that
> matter) does not check out the branch, but instead detaches HEAD at
> foo. This is quite counter-intuitive (at least to me) and the same
> functionality can be achieved by using e.g. foo~0. Change the behavior
> so that the branch is actually checked out. This also applies to
> e.g. 'git rebase master refs/heads/topic', which currently rebases a
> detached HEAD. There are probably other examples as well that I'm not
> aware of.
I have seen that behavior claimed as intentional, but I agree it is
unintuitive. In most other places referring to a ref by a short name or
a fully qualified name is equivalent (except with respect to
disambiguating short names, of course).
> Existing scripts may depend on the current behavior. It seems unlikely
> that many users depend on it. Most likely, they use foo~0 or foo^0
> instead.
As cool and clever as the foo^0 behavior is once you understand it, I
think it is a horribly confusing thing for non-experts. As part of this
proposal, should we perhaps offer "git checkout --detach" as the
easy-on-the-eyes way of intentionally detaching?
-Peff
^ permalink raw reply
* Re: "git add -u" broken in git 1.7.4?
From: Matthieu Moy @ 2011-02-07 20:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Sebastian Pipping, Git ML
In-Reply-To: <7vhbcguytf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I would say so; "add -p" was an ill-executed afterthought. The codepath
> was originally meant to be used from "-i" as the top-level interface that
> was a fully interactive way to prepare for the next commit, which is an
> operation that is inherently full-tree.
I agree that "git add -i" is a way to prepare the next commit, but you
seem to imply that "git add -u" is not and then I have to disagree.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [1.8.0] Provide proper remote ref namespaces
From: Jeff King @ 2011-02-07 20:56 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Johan Herland, git, Sverre Rabbelier,
Nguyen Thai Ngoc Duy
In-Reply-To: <alpine.LFD.2.00.1102071541460.14920@xanadu.home>
On Mon, Feb 07, 2011 at 03:51:55PM -0500, Nicolas Pitre wrote:
> > Will it
> > also apply to refs/heads/foo versus refs/remotes/*/tags/foo? In the
> > final case, that does matter to "git push" (should the destination be in
> > the head or tag namespace?).
>
> In such a case I'd say that head refs have precedence over tag refs, and
> when that occurs then warn the user. And I wouldn't make a special
> case for 'git push' here. Whether it is push or log, the head ref would
> take precedence, and the user warned about existence of a tag with the
> same name. Of course using "tag/foo" should be unambigous again.
I am fine with that, although it is the opposite of the current dwim_ref
behavior (which warns on ambiguity but prefers tags). However, Junio has
said that the current behavior was simply arbitrary.
> > So the actual names of the ref can matter,
> > and should probably be taken into account when deciding what is
> > ambiguous.
>
> What happens today when you have refs/heads/foo and refs/tags/foo?
For dwim_ref, it prefers the tag and issues a warning. For git-push, it
complains about the ambiguity and dies. For git checkout, we prefer the
head. For git-tag, we prefer the tag (though I think that only matters
for "git tag -d").
-Peff
^ permalink raw reply
* [PATCH v4 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Heiko Voigt @ 2011-02-07 20:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207204818.GA63976@book.hvoigt.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
directory is busy, we only want to retry deleting the directory if it
is empty, so test specifically for that case and set ENOTEMPTY rather
than EACCES.
Noticed by Greg Hazel.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
compat/mingw.c | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 68cc79f..368edf2 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -225,6 +225,30 @@ int mingw_unlink(const char *pathname)
return ret;
}
+static int is_dir_empty(const char *path)
+{
+ struct strbuf buf = STRBUF_INIT;
+ WIN32_FIND_DATAA findbuf;
+ HANDLE handle;
+
+ strbuf_addf(&buf, "%s\\*", path);
+ handle = FindFirstFileA(buf.buf, &findbuf);
+ if (handle == INVALID_HANDLE_VALUE) {
+ strbuf_release(&buf);
+ return GetLastError() == ERROR_NO_MORE_FILES;
+ }
+
+ while (!strcmp(findbuf.cFileName, ".") ||
+ !strcmp(findbuf.cFileName, ".."))
+ if (!FindNextFile(handle, &findbuf)) {
+ strbuf_release(&buf);
+ return GetLastError() == ERROR_NO_MORE_FILES;
+ }
+ FindClose(handle);
+ strbuf_release(&buf);
+ return 0;
+}
+
#undef rmdir
int mingw_rmdir(const char *pathname)
{
@@ -233,6 +257,10 @@ int mingw_rmdir(const char *pathname)
while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
if (!is_file_in_use_error(GetLastError()))
break;
+ if (!is_dir_empty(pathname)) {
+ errno = ENOTEMPTY;
+ break;
+ }
/*
* We assume that some other process had the source or
* destination file open at the wrong moment and retry.
--
1.7.4.34.gd2cb1
^ permalink raw reply related
* Re: [1.8.0] git checkout refs/heads/foo checks out branch foo
From: Junio C Hamano @ 2011-02-07 20:53 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <alpine.DEB.1.10.1102062234010.3788@debian>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> Proposal:
>
> 'git checkout refs/heads/foo' (or 'git checkout heads/foo' for that
> matter) does not check out the branch, but instead detaches HEAD at
> foo.
Yes, and it is deliberately so as it is a guaranteed-sure way for scripts
to work around potential ref ambiguity when talking about the branch
called 'foo' to spell it out with leading refs/heads/.
As long as you do not break scripts that guard against breakage you are
proposing by saying "refs/heads/foo^0", we are Ok with the proposal, I
think.
> Migration plan:
>
> Make 'git checkout refs/head/foo' emit a warning in the next 1.7.x
> explaining that its semantics will change in 1.8.0. Then change the
> behavior in 1.8.0 and remove the warning.
Reasonable.
Thanks.
^ permalink raw reply
* [PATCH v4 4/5] mingw: add fallback for rmdir in case directory is in use
From: Heiko Voigt @ 2011-02-07 20:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207204818.GA63976@book.hvoigt.net>
The same logic as for unlink and rename also applies to rmdir. For
example in case you have a shell open in a git controlled folder. This
will easily fail. So lets be nice for such cases as well.
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
compat/mingw.c | 25 +++++++++++++++++++++++++
compat/mingw.h | 3 +++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 1354b8e..68cc79f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -225,6 +225,31 @@ int mingw_unlink(const char *pathname)
return ret;
}
+#undef rmdir
+int mingw_rmdir(const char *pathname)
+{
+ int ret, tries = 0;
+
+ while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ if (!is_file_in_use_error(GetLastError()))
+ break;
+ /*
+ * We assume that some other process had the source or
+ * destination file open at the wrong moment and retry.
+ * In order to give the other process a higher chance to
+ * complete its operation, we give up our time slice now.
+ * If we have to retry again, we do sleep a bit.
+ */
+ Sleep(delay[tries]);
+ tries++;
+ }
+ while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+ ask_yes_no_if_possible("Deletion of directory '%s' failed. "
+ "Should I try again?", pathname))
+ ret = rmdir(pathname);
+ return ret;
+}
+
#undef open
int mingw_open (const char *filename, int oflags, ...)
{
diff --git a/compat/mingw.h b/compat/mingw.h
index 970d743..fe6ba34 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -169,6 +169,9 @@ int link(const char *oldpath, const char *newpath);
int mingw_unlink(const char *pathname);
#define unlink mingw_unlink
+int mingw_rmdir(const char *path);
+#define rmdir mingw_rmdir
+
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
--
1.7.4.34.gd2cb1
^ permalink raw reply related
* Re: [1.8.0] Provide proper remote ref namespaces
From: Nicolas Pitre @ 2011-02-07 20:51 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Johan Herland, git, Sverre Rabbelier,
Nguyen Thai Ngoc Duy
In-Reply-To: <20110207202551.GC13461@sigill.intra.peff.net>
On Mon, 7 Feb 2011, Jeff King wrote:
> I guess the problem is that I'm not clear on exactly what the new lookup
> / ambiguity rules are proposed to be. Clearly something will need to go
> in the dwim_ref level. And I think it's obvious that if "refs/tags/foo"
> and "refs/remotes/*/tags/foo" point to the same sha1, they will not be
> considered ambiguous.
Agreed.
> Will the same apply to refs/heads/foo versus refs/remotes/*/foo?
I think it should. If both branches are pointing to the same commit
then the short form "foo" should be unambigous and the operation proceed
(be that 'git push' or 'git log' that shouldn't matter).
> Will it
> also apply to refs/heads/foo versus refs/remotes/*/tags/foo? In the
> final case, that does matter to "git push" (should the destination be in
> the head or tag namespace?).
In such a case I'd say that head refs have precedence over tag refs, and
when that occurs then warn the user. And I wouldn't make a special
case for 'git push' here. Whether it is push or log, the head ref would
take precedence, and the user warned about existence of a tag with the
same name. Of course using "tag/foo" should be unambigous again.
> So the actual names of the ref can matter,
> and should probably be taken into account when deciding what is
> ambiguous.
What happens today when you have refs/heads/foo and refs/tags/foo?
Nicolas
^ permalink raw reply
* [PATCH v4 3/5] mingw: make failures to unlink or move raise a question
From: Heiko Voigt @ 2011-02-07 20:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207204818.GA63976@book.hvoigt.net>
On Windows in case a program is accessing a file unlink or
move operations may fail. To give the user a chance to correct
this we simply wait until the user asks us to retry or fail.
This is useful because of the following use case which seem
to happen rarely but when it does it is a mess:
After making some changes the user realizes that he was on the
incorrect branch. When trying to change the branch some file
is still in use by some other process and git stops in the
middle of changing branches. Now the user has lots of files
with changes mixed with his own. This is especially confusing
on repositories that contain lots of files.
Although the recent implementation of automatic retry makes
this scenario much more unlikely lets provide a fallback as
a last resort.
Thanks to Albert Dvornik for disabling the question if users can't see it.
If the stdout of the command is connected to a terminal but the stderr
has been redirected, the odds are good that the user can't see any
question we print out to stderr. This will result in a "mysterious
hang" while the app is waiting for user input.
It seems better to be conservative, and avoid asking for input
whenever the stderr is not a terminal, just like we do for stdin.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Albert Dvornik <dvornik+git@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
compat/mingw.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4a1c218..1354b8e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2,6 +2,7 @@
#include "win32.h"
#include <conio.h>
#include "../strbuf.h"
+#include "../run-command.h"
static const int delay[] = { 0, 1, 10, 20, 40 };
@@ -129,6 +130,74 @@ static inline int is_file_in_use_error(DWORD errcode)
return 0;
}
+static int read_yes_no_answer(void)
+{
+ char answer[1024];
+
+ if (fgets(answer, sizeof(answer), stdin)) {
+ size_t answer_len = strlen(answer);
+ int got_full_line = 0, c;
+
+ /* remove the newline */
+ if (answer_len >= 2 && answer[answer_len-2] == '\r') {
+ answer[answer_len-2] = '\0';
+ got_full_line = 1;
+ }
+ else if (answer_len >= 1 && answer[answer_len-1] == '\n') {
+ answer[answer_len-1] = '\0';
+ got_full_line = 1;
+ }
+ /* flush the buffer in case we did not get the full line */
+ if (!got_full_line)
+ while((c = getchar()) != EOF && c != '\n');
+ } else
+ /* we could not read, return the
+ * default answer which is no */
+ return 0;
+
+ if (tolower(answer[0]) == 'y' && !answer[1])
+ return 1;
+ if (!strncasecmp(answer, "yes", sizeof(answer)))
+ return 1;
+ if (tolower(answer[0]) == 'n' && !answer[1])
+ return 0;
+ if (!strncasecmp(answer, "no", sizeof(answer)))
+ return 0;
+
+ /* did not find an answer we understand */
+ return -1;
+}
+
+static int ask_yes_no_if_possible(const char *format, ...)
+{
+ char question[4096];
+ const char *retry_hook[] = { NULL, NULL, NULL };
+ va_list args;
+
+ va_start(args, format);
+ vsnprintf(question, sizeof(question), format, args);
+ va_end(args);
+
+ if ((retry_hook[0] = getenv("GIT_ASK_YESNO"))) {
+ retry_hook[1] = question;
+ return !run_command_v_opt(retry_hook, 0);
+ }
+
+ if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
+ return 0;
+
+ while (1) {
+ int answer;
+ fprintf(stderr, "%s (y/n) ", question);
+
+ if ((answer = read_yes_no_answer()) >= 0)
+ return answer;
+
+ fprintf(stderr, "Sorry, I did not understand your answer. "
+ "Please type 'y' or 'n'\n");
+ }
+}
+
#undef unlink
int mingw_unlink(const char *pathname)
{
@@ -149,6 +218,10 @@ int mingw_unlink(const char *pathname)
Sleep(delay[tries]);
tries++;
}
+ while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+ ask_yes_no_if_possible("Unlink of file '%s' failed. "
+ "Should I try again?", pathname))
+ ret = unlink(pathname);
return ret;
}
@@ -1326,6 +1399,11 @@ repeat:
tries++;
goto repeat;
}
+ if (gle == ERROR_ACCESS_DENIED &&
+ ask_yes_no_if_possible("Rename from '%s' to '%s' failed. "
+ "Should I try again?", pold, pnew))
+ goto repeat;
+
errno = EACCES;
return -1;
}
--
1.7.4.34.gd2cb1
^ permalink raw reply related
* [PATCH v4 2/5] mingw: work around irregular failures of unlink on windows
From: Heiko Voigt @ 2011-02-07 20:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207204818.GA63976@book.hvoigt.net>
If a file is opened by another process (e.g. indexing of an IDE) for
reading it is not allowed to be deleted. So in case unlink fails retry
after waiting for some time. This extends the workaround from 6ac6f878.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
compat/mingw.c | 31 +++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index a7e1c6b..4a1c218 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3,6 +3,8 @@
#include <conio.h>
#include "../strbuf.h"
+static const int delay[] = { 0, 1, 10, 20, 40 };
+
int err_win_to_posix(DWORD winerr)
{
int error = ENOSYS;
@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)
return error;
}
+static inline int is_file_in_use_error(DWORD errcode)
+{
+ switch(errcode) {
+ case ERROR_SHARING_VIOLATION:
+ case ERROR_ACCESS_DENIED:
+ return 1;
+ }
+
+ return 0;
+}
+
#undef unlink
int mingw_unlink(const char *pathname)
{
+ int ret, tries = 0;
+
/* read-only files cannot be removed */
chmod(pathname, 0666);
- return unlink(pathname);
+ while ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+ if (!is_file_in_use_error(GetLastError()))
+ break;
+ /*
+ * We assume that some other process had the source or
+ * destination file open at the wrong moment and retry.
+ * In order to give the other process a higher chance to
+ * complete its operation, we give up our time slice now.
+ * If we have to retry again, we do sleep a bit.
+ */
+ Sleep(delay[tries]);
+ tries++;
+ }
+ return ret;
}
#undef open
@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)
{
DWORD attrs, gle;
int tries = 0;
- static const int delay[] = { 0, 1, 10, 20, 40 };
/*
* Try native rename() first to get errno right.
--
1.7.4.34.gd2cb1
^ permalink raw reply related
* [PATCH v4 1/5] mingw: move unlink wrapper to mingw.c
From: Heiko Voigt @ 2011-02-07 20:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <20110207204818.GA63976@book.hvoigt.net>
The next patch implements a workaround in case unlink fails on Windows.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
compat/mingw.c | 8 ++++++++
compat/mingw.h | 11 +++--------
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index bee6054..a7e1c6b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -116,6 +116,14 @@ int err_win_to_posix(DWORD winerr)
return error;
}
+#undef unlink
+int mingw_unlink(const char *pathname)
+{
+ /* read-only files cannot be removed */
+ chmod(pathname, 0666);
+ return unlink(pathname);
+}
+
#undef open
int mingw_open (const char *filename, int oflags, ...)
{
diff --git a/compat/mingw.h b/compat/mingw.h
index cafc1eb..970d743 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -119,14 +119,6 @@ static inline int mingw_mkdir(const char *path, int mode)
}
#define mkdir mingw_mkdir
-static inline int mingw_unlink(const char *pathname)
-{
- /* read-only files cannot be removed */
- chmod(pathname, 0666);
- return unlink(pathname);
-}
-#define unlink mingw_unlink
-
#define WNOHANG 1
pid_t waitpid(pid_t pid, int *status, unsigned options);
@@ -174,6 +166,9 @@ int link(const char *oldpath, const char *newpath);
* replacements of existing functions
*/
+int mingw_unlink(const char *pathname);
+#define unlink mingw_unlink
+
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
--
1.7.4.34.gd2cb1
^ permalink raw reply related
* Re: [PATCH] cache-tree: do not cache empty trees
From: Junio C Hamano @ 2011-02-07 20:48 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, git, Ilari Liusvaara, Jakub Narebski,
Dmitry S. Kravtsov, Shawn Pearce
In-Reply-To: <20110207095713.GA19653@do>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Perhaps it's not a good approach after all. What I wanted was to make
> pre-1.8.0 tolerate empty trees created by 1.8.0. Perhaps it's better
> to just let pre-1.8.0 refuse to work with empty trees, forcing users
> to upgrade to 1.8.0?
I don't think we saw even something remotely resembles an agreement that
empty tree is a good thing to have yet. Why are you rushing things?
^ permalink raw reply
* [PATCH v4 0/5] make open/unlink failures user friendly on windows using retry/abort
From: Heiko Voigt @ 2011-02-07 20:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: kusmabite, Johannes Sixt, Pat Thoyts, msysgit, git,
Johannes Schindelin
In-Reply-To: <7vmxo6pxyi.fsf@alter.siamese.dyndns.org>
This is the fourth iteration of my patch series with hopefully all the
mentioned issues addressed.
On Wed, Dec 15, 2010 at 12:45:09PM -0800, Junio C Hamano wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
> > Perhaps instead we would be better of with something like an xunlink
> > (etc) in wrapper.c that deals with this on all platforms (and make
> > sure that unlink sets errno to EBUSY correctly if it doesn't already)?
>
> That is superficially similar to the way we let xread() silently handle
> short read to give us an easier to use API. Especially, the part to
> silently retry for a few times is similar to xread() recovering by
> repeating short reads.
>
> I do not think "ask the user one last time" part belongs to such a
> wrapper, though.
I think we should currently keep this Windows specific because this is the
only system where such problems occur in practise. If we find later on
that other systems are affected with the same problem we can still move
the functions to another more public place. But without proper testing
whether it helps on other setups as well putting this into a more
prominent place does not make sense, IMO.
Cheers Heiko
Heiko Voigt (4):
mingw: move unlink wrapper to mingw.c
mingw: work around irregular failures of unlink on windows
mingw: make failures to unlink or move raise a question
mingw: add fallback for rmdir in case directory is in use
Johannes Schindelin (1):
mingw_rmdir: set errno=ENOTEMPTY when appropriate
compat/mingw.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
compat/mingw.h | 14 ++---
2 files changed, 173 insertions(+), 9 deletions(-)
--
1.7.4.34.gd2cb1
^ permalink raw reply
* Re: [1.8.0] Provide proper remote ref namespaces
From: Nicolas Pitre @ 2011-02-07 20:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Marc Branchaud, Jeff King, Johan Herland, git, Sverre Rabbelier,
Nguyen Thai Ngoc Duy
In-Reply-To: <7v62svvdjo.fsf@alter.siamese.dyndns.org>
On Mon, 7 Feb 2011, Junio C Hamano wrote:
> As I said in my message, it feels awkward to use refs/private-tags for
> tags everybody uses for his or her own purpose, so by swapping the roles
> of namespaces around, we would be able to use refs/tags for private ones,
> and refs/remotes/origin/tags for the ones that came from upstream. But
> then if you fetch/pull from a third party (including the "interim
> maintainer"), it feels wasteful to get full set of tags that you have in
> the origin namespace anyway replicated in refs/remotes/interim/tags.
Why so? At least you get to know if that particular remote has a
particular tag that may also exist elsewhere. And if you decide to drop
one remote source with all its tags from your local repository, then the
remaining one(s) still have relevant tags available.
> And that is what bothers me---not the waste itself, but I have this
> nagging feeling that the wasteful duplication is an indication of
> something else designed wrong.
We do have "wasteful" duplicated references all over the place, such as
when two directories (tree objects) contain the same file content (refer
to the same blob object). But no one feels like this is wasted
duplication as those two directories end up using the same single blob
object even if in the working directory you get twice the content.
Here this is the same thing. You have multiple handles to the same tag
which are distributed across different remote repositoryes you are
tracking. The name on the tag may be found in many places, and even
under different names. But there is still only one actual tag object.
If you have 3 remotes sharing the same tag then you know that the tag
cannot be garbage collected until all three remotes have been removed
from your repository.
Simply storing snapshots of the tag state per remote repository is
simple and allow for smarter processing on top, which is in agreement
with the design philosophy for the rest of Git.
Nicolas
^ permalink raw reply
* Re: [1.8.0] Provide proper remote ref namespaces
From: Jeff King @ 2011-02-07 20:25 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Johan Herland, git, Sverre Rabbelier,
Nguyen Thai Ngoc Duy
In-Reply-To: <alpine.LFD.2.00.1102070936530.14920@xanadu.home>
On Mon, Feb 07, 2011 at 09:53:29AM -0500, Nicolas Pitre wrote:
> > But more importantly, don't we sometimes care where the ref came from?
>
> Not at the moment. Certainly not with the current flat namespace used
> for tags.
I'm not sure I was clear. I didn't mean "which remote the ref came from"
but rather "when choosing between two refs, don't we sometimes care
about the actual name of the ref?". But see below.
> > If I say "git push remote v1.7.4" we do some automagic on the
> > destination side of the refspec based on the fact that the source ref
> > was found in the refs/tags hierarchy. In the case we're talking about,
> > all of the ambiguous refs would presumably also be coming from
> > refs/remotes/*/tags/, so they would be functionally equivalent. But I
> > wanted to point it out because:
> >
> > 1. It is an additional equivalent requirement for two refs to not be
> > ambiguous. They must have the same sha1, _and_ they must have the
> > same "type".
>
> How can this matter? The same automagic on the destination ref may
> still take place. Semantically you want to push v1.7.4 so nothing has
> to change there, irrespective of the namespace the v1.7.4 tag comes
> from. This doesn't matter today, so why would this particular case need
> to change?
I guess the problem is that I'm not clear on exactly what the new lookup
/ ambiguity rules are proposed to be. Clearly something will need to go
in the dwim_ref level. And I think it's obvious that if "refs/tags/foo"
and "refs/remotes/*/tags/foo" point to the same sha1, they will not be
considered ambiguous.
Will the same apply to refs/heads/foo versus refs/remotes/*/foo? Will it
also apply to refs/heads/foo versus refs/remotes/*/tags/foo? In the
final case, that does matter to "git push" (should the destination be in
the head or tag namespace?). So the actual names of the ref can matter,
and should probably be taken into account when deciding what is
ambiguous.
Maybe that was obvious to everybody and it was an implicit part of the
proposal, but it wasn't clear to me.
-Peff
^ permalink raw reply
* [PATCH v2] commit: fix memory-leak
From: Erik Faye-Lund @ 2011-02-07 20:21 UTC (permalink / raw)
To: git; +Cc: matthieu.moy, msysgit, blees
In-Reply-To: <AANLkTikKZ+2qUMF1T5pP60cUd9Ya3n2mfhTkX6L32zmn@mail.gmail.com>
The name, email and date strings are some times allocated on the
heap, but not free'd. Fix this by making sure they are allways
heap-allocated, so we can safely free the memory.
At the same time, this fixes a problem with strict-POSIX getenv
implementations. POSIX says "The return value from getenv() may
point to static data which may be overwritten by subsequent calls
to getenv()", so not duplicating the strings is a potential bug.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
Fixed typo in commit message, as pointed out by Matthieu Moy.
builtin/commit.c | 9 ++++++---
git-compat-util.h | 1 +
wrapper.c | 6 ++++++
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 03cff5a..e5a649e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -465,9 +465,9 @@ static void determine_author_info(struct strbuf *author_ident)
{
char *name, *email, *date;
- name = getenv("GIT_AUTHOR_NAME");
- email = getenv("GIT_AUTHOR_EMAIL");
- date = getenv("GIT_AUTHOR_DATE");
+ name = xgetenv("GIT_AUTHOR_NAME");
+ email = xgetenv("GIT_AUTHOR_EMAIL");
+ date = xgetenv("GIT_AUTHOR_DATE");
if (use_message && !renew_authorship) {
const char *a, *lb, *rb, *eol;
@@ -507,6 +507,9 @@ static void determine_author_info(struct strbuf *author_ident)
date = force_date;
strbuf_addstr(author_ident, fmt_ident(name, email, date,
IDENT_ERROR_ON_NO_NAME));
+ free(name);
+ free(email);
+ free(date);
}
static int ends_rfc2822_footer(struct strbuf *sb)
diff --git a/git-compat-util.h b/git-compat-util.h
index d6d269f..12f111f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -409,6 +409,7 @@ typedef void (*try_to_free_t)(size_t);
extern try_to_free_t set_try_to_free_routine(try_to_free_t);
extern char *xstrdup(const char *str);
+extern char *xgetenv(const char *name);
extern void *xmalloc(size_t size);
extern void *xmallocz(size_t size);
extern void *xmemdupz(const void *data, size_t len);
diff --git a/wrapper.c b/wrapper.c
index 8d7dd31..e6173c4 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -30,6 +30,12 @@ char *xstrdup(const char *str)
return ret;
}
+char *xgetenv(const char *name)
+{
+ char *tmp = getenv(name);
+ return tmp ? xstrdup(tmp) : NULL;
+}
+
void *xmalloc(size_t size)
{
void *ret = malloc(size);
--
1.7.4.msysgit.0
^ permalink raw reply related
* Re: [1.8.0] Provide proper remote ref namespaces
From: Jeff King @ 2011-02-07 20:19 UTC (permalink / raw)
To: Johan Herland
Cc: git, Matthieu Moy, Dmitry Potapov, Junio C Hamano,
Sverre Rabbelier, Nguyen Thai Ngoc Duy, Nicolas Pitre
In-Reply-To: <201102070958.11551.johan@herland.net>
On Mon, Feb 07, 2011 at 09:58:11AM +0100, Johan Herland wrote:
> > No, that won't give you me/shawn/junio/v1.7.4, but it does mean we have
> > to gracefully handle the case of ambiguous duplicate tags (that happen
> > to point to the same thing).
>
> Whoa, we use the "ambiguous" term differently here. In this whole thread I
> have used "ambiguous" exclusively about when the same (shorthand) tag name
> point to _different_ things. As long as they point to the same thing, there
> is no ambiguity, IMHO.
Sorry, I should have been more clear. I meant "ambiguous by the current
code's definition", meaning "we would still need to use your new
ambiguity definition to resolve this situation".
IOW, I think we are on the same page.
> This is the same technique we use when talking about branch names: On this
> mailing list, nobody is confused when I refer to 'maint', 'master', 'next'
> and 'pu'. Still, in our own work repos (at least in mine), these branches
> are actually called "refs/remotes/origin/<name>" (commonly referred to by
> their shorthands "origin/<name>"). Here we are, juggling the same kind of
> namespaces that I propose for tags, and it seems to work well without
> causing much confusion.
Just playing devil's advocate for a moment: isn't this namespace
distinction one of the more confusing things in git for new users? That
is, I have seen new-ish git users say "OK, so I cloned from upstream.
How come I can't say "git log maint" now?" Or it used to be "how come I
can't "git checkout maint" now?" The latter is now handled by some very
specific magic in "git checkout", but in general ref lookup does not
automagically look in remotes namespaces, and it has caused some
confusion.
So here we are introducing more distinction between project-wide names
and per-remote names. I absolutely think it's the right thing to do from
a "keep it simple, orthogonal, and distributed" perspective. But we also
need to recognize we are making some common use cases more confusing. In
the case of remote-tracking branches, we ended up adding some porcelain
features to make common actions (like checking out a local branch from a
remote) more seamless. But there is still some confusion among new
users.
I'm sort of rambling as I'm not quite sure yet what this means for the
tags proposal, but a few questions I think are important to consider
are:
1. Where have we succeeded and where have we failed with making
separate-remotes / tracking branches seamless to the user (as
opposed to something like a system where
fetching from upstream fetches straight into your master branch
(which has its own problems, but would be conceptually very
simple)? Do those failures apply in this case, and if so how can we
do better?
2. Can we apply new ideas for handling separate-remote tags to the
branches case? Obviously one big proposal is searching in the
per-remote tag namespace for refs. Should we be doing the same with
heads?
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox