Git development
 help / color / mirror / Atom feed
* Re: [RFC PATCH v2 09/16] Move WebDAV HTTP push under remote-curl
From: Mike Hommey @ 2009-10-13  4:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Daniel Barkalow, Tay Ray Chuan
In-Reply-To: <1255400715-10508-10-git-send-email-spearce@spearce.org>

On Mon, Oct 12, 2009 at 07:25:08PM -0700, Shawn O. Pearce wrote:
> The remote helper interface now supports the push capability,
> which can be used to ask the implementation to push one or more
> specs to the remote repository.  For remote-curl we implement this
> by calling the existing WebDAV based git-http-push executable.
> 
> Internally the helper interface uses the push_refs transport hook
> so that the complexity of the refspec parsing and matching can be
> reused between remote implementations.  When possible however the
> helper protocol uses source ref name rather than the source SHA-1,
> thereby allowing the helper to access this name if it is useful.

It's been a while I haven't followed changes in the remote code but this
looks nice, though I haven't checked thoroughly. I guess the next step
would be to kill http-push as an external program.

Mike

^ permalink raw reply

* Re: [PATCH/RFC 3/4] git check-ref-format --print
From: Jonathan Nieder @ 2009-10-13  4:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, git, Shawn O. Pearce
In-Reply-To: <7vococ6v73.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> I do not disagree with a desire to help fixing the unicode insanity on
> that platform, but I suspect that check-ref-format is a wrong place to
> tackle the issue.  You would need a similar filter for outputs from the
> likes of ls-files and "diff --name-only", iow, anything that deal with
> pathnames, no?
> 
> It would have be something like "check-ref-format --print | iconv ..."
> pipeline (conceptually, if not forcing the pipeline to the end users, that
> is).

GNU iconv does not write the various Unicode normalization forms, so it
would have to be something like "check-ref-format --print | charlint ..."
instead.  Regardless of the filesystem, it seems reasonable to consider é
(U+00e9) and é (U+0065 + U+0301) the same character when it appears in a
ref name, and one way to achieve this would be to pick one normalization
form and stick to it.  This does not seem so different from stripping out
empty path components.

As a side effect, that would deal with OS X’s strange handling of unicode
filenames for .git/refs/*.  Now that I think about it, if fighting OS X
were the only problem that needed to be solved, I don’t think I’d like
this solution so much.  The analogous solution to the also unsolved issue
of case insensitive filesystems is to force all ref names to lowercase.
Do we want to do that?  (The case insensitivity issue might not be as bad,
since the relevant filesystems will at least _preserve_ the case of
filenames in .git/refs.  Should we copy them and smudge new ref names to
match known ones that differ only in case?  Just thinking about the
problem makes me cringe.)

Coping with the unicode filename craziness in the working tree is a
separate issue, though probably a more important one.  I think Linus set
up a framework for solving it in
<http://thread.gmane.org/gmane.comp.version-control.git/77827>.

Regards,
Jonathan

^ permalink raw reply

* Re: Git: "No you can't handle my root!" (?)
From: Jeff King @ 2009-10-13  5:01 UTC (permalink / raw)
  To: Daniele Segato; +Cc: sylvain, git
In-Reply-To: <1255407433.15646.12.camel@localhost>

On Tue, Oct 13, 2009 at 06:17:13AM +0200, Daniele Segato wrote:

> > This seems to work:
> > 
> >   $ cd ~
> >   $ git init
> >   $ echo '*' >.gitignore
> >   $ echo '!.*' >.gitignore
> > 
> > > Or better: provide a list of directory under $HOME I want to track 
> > 
> > Same thing, but make your ! pattern more specific.
> 
> thanks again!

You're welcome, though while reading the quoted text I noticed a typo in
my instructions. The second echo should obviously be _appending_ to
.gitignore:

  $ echo '!.*' >>.gitignore

Hopefully that was obvious, but I thought I would point it out for the
record.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-13  6:36 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin
  Cc: Euguess, Mikael Magnusson, Matthieu Moy, Jeff King, Jay Soffian,
	git
In-Reply-To: <7vr5t89qiw.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > Your idea is also a backwards incompatible change, so we can just as
> > well implement the original suggestion and force scripts (or us) to
> > use some other means when they want to detach.  Say, why not just
> > invent an option along the lines of
> >
> >   git checkout {-d|--detach} $ref
> >
> > to make it explicit.
> 
> Or can't you go the other way, say
> 
> 	git checkout -t $remote_tracking
> 
> to create a local branch forking from the named remote tracking branch?

Sure, but we already have that and we still failed to fix the users,
so FWIW, I think Dscho's right and we should try fixing the UI next.

[I've also seen several users shoot themselves with detached HEADs to
the point where I explain the concept before even mentioning
checkout.]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
From: Johannes Sixt @ 2009-10-13  6:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git
In-Reply-To: <7vr5t8coex.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> I even think that the support for an explicit branch name in the reset
> subcommand should eventually be deprecated, perhaps unless it matches what
> is stored in BISECT_START.

Goodness, NO!

> The documentation, does not even talk about what the optional <branch>
> argument is good for, even though it lists the optional <branch> in the
> synopsis section.

If I had know about this feature (yes, FEATURE), I would have used it like
this in the past:

  $ git branch tmp
  $ git bisect reset tmp
  $ git branch -d tmp

With the patch proposed by Anders this shortens to:

  $ git bisect reset HEAD

> Having said all that, four years and two months are looooooong time in git
> timescale, and I am discounting, without any evidence to judge either way,
> the possibility that people may have learned during that time to (ab)use
> this as a (very useful?) shortcut to finish the current bisection and
> switch to some entirely different branch.

In all the bisect runs that I have done in my live, the ONLY way I wanted
'bisect reset' to act was to NOT change the commit it currently was on.
The fact that it switched back to the commit or branch that the bisect was
started on, was always a major inconvenience.

So, I have no problem if you want to deprecate the branch parameter, if at
the same time bisect reset no longer switches to some other commit. ;)

> I offhand do not see a good rationale for such a shortcut to finish bisect
> and switch to another branch (IOW, I understand "it is shorter to type",
> but I do not see "it is often done and very useful"), but I am open to be
> enlightened by a workflow where such a shortcut is useful.

In my workflow, after I've found the bad commit, I always want bisect to
stay at the commit that it found. I don't want it to warp me somewhere
else; I want to make the decision where to go next myself.

-- Hannes

^ permalink raw reply

* Re: [RFC PATCH v2 00/16] Return of smart HTTP
From: Junio C Hamano @ 2009-10-13  6:42 UTC (permalink / raw)
  To: eduard stefan; +Cc: Shawn O. Pearce, git
In-Reply-To: <4AD3F7C5.2060203@gmail.com>

eduard stefan <eduard.stefan@gmail.com> writes:

> I have apllied you patches on top of Git 1.6.5 release,
> and they solved my http cloning crashes on windows
> (msysGit environment).

Unfortunately that is not a very good news.

As there is no chance Shawn's new series will be applied to 'maint' for
updating 1.6.5.X series, somebody needs to address the breakage without
introducing the whole new HTTP code.  Is the breakage you are seeing
limited only to msysgit environment, or does the same breakage appear on
other platforms?

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
From: Johannes Sixt @ 2009-10-13  6:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git
In-Reply-To: <7vaazw6uyi.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> I would understand it, if not agreeing that I also am often in that
> situation myself", if somebody does not even care which commit he was on
> before starting the bisection, but knows (or is willing to decide at that
> point) which branch (or even a specific commit, while still being
> detached) he wants to switch to.  And it would make sense to avoid an
> extra checkout that snaps back to the pre-bisection commit before
> switching to the new state he has chosen.

The situation that I'm faced quite frequently is that after I find a
regression, I cannot tell which released version did not have the
breakage. Hence, the first thing I have to do is to find a good commit.
Therefore, I jump around in ancient history until I find a good commit.
Then I start bisect. I certainly do NOT want to be warped back to this
ancient commit by 'bisect reset'.

-- Hannes

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
From: Junio C Hamano @ 2009-10-13  7:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Anders Kaseorg, git
In-Reply-To: <4AD420BC.5060506@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> In my workflow, after I've found the bad commit, I always want bisect to
> stay at the commit that it found. I don't want it to warp me somewhere
> else; I want to make the decision where to go next myself.

Are you sure about what you are saying?

Half of the time, the commit you test in your "git bisect" section would
be a "good" one, and immediately after you tell it "bisect good", it tells
you that some _other_ commit you marked "bad" is the first bad commit.  In
such a case, you won't be on the commit that the bisect has found.

So I _do_ agree that you would always want to stare at the commit that is
the first bad one, leaving the bisection session at the detached HEAD
state bisection session ends at is _totally_ different from what you want.

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a branch
From: Anders Kaseorg @ 2009-10-13  7:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaazw6uyi.fsf@alter.siamese.dyndns.org>

On Mon, 12 Oct 2009, Junio C Hamano wrote:
> But I do not know how it hurts to still have bisect states around, in
> order to find where you want to go next.  Could you elaborate?

Mostly little irritations.  Extra bisect/* refs show up in gitk.  If you 
use __git_ps1 in your prompt (from git-completion.bash), it adds 
|BISECTING to your prompt.

Also, I just noticed that if you start a new bisection without ever 
cleaning up the old one, the next ‘git bisect reset’ will bring you back 
to HEAD before the old bisection instead of HEAD before the new one, which 
is not what you would expect if you forgot that the old bisection ever 
happened.

> I am inclined to ask you to come up with a paragraph in the 
> documentation to discuss how the optional <branch> (now it will be 
> <commit>) parameter to the reset subcommand is meant to be used and 
> re-submit the original patch, perhaps with an updated commit log 
> message.

I note that the ‘git checkout’ documentation mentions <branch> and not 
<commit>, perhaps to emphasize that HEAD will become attached to the 
branch if you specify a branch name.  Do you think it makes sense for 
these to be documented differently?

Anders

^ permalink raw reply

* Re: [PATCH 2/3] git config: clarify bool types
From: Michael J Gruber @ 2009-10-13  7:09 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Michael J Gruber, Junio C Hamano, git
In-Reply-To: <94a0d4530910121014k666207b9ub38fcecd47641ace@mail.gmail.com>

Felipe Contreras venit, vidit, dixit 12.10.2009 19:14:
> On Mon, Oct 12, 2009 at 3:30 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Felipe Contreras venit, vidit, dixit 12.10.2009 12:03:
>>> On Mon, Oct 12, 2009 at 8:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>>
>>>>> The value is what it is, the --bool and --bool-or-int options don't
>>>>> specify the value type, just how it is interpreted. For example: a value
>>>>> of '1' can be interpreted as 'true'.
>>>>
>>>> It is not really about "interpreting", but about showing, isn't it?
>>>
>>> Unless you are setting it, instead of reading it.
>>>
>>
>> I'd still suggest fixing the typo ("interpreted") and spelling out
>> "boolean".
> 
> Oops! You mean s/intepreted/interpreted/?

Yep :)

> 
> If we spell 'boolean' we might as well spell 'integer'; I think bool
> and int are fine.
> 

"int" is at least a standard type name in C, whereas "bool" is not; but,
yes, feel free to spell out "integer", or use "--int or --bool" as it
is, which is a back reference to the corresponding entries for "--int"
and "--bool", where things should be spelled out.

Michael

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-13  7:16 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Johannes Schindelin, Euguess, Mikael Magnusson, Matthieu Moy,
	Jeff King, Jay Soffian, git
In-Reply-To: <200910130836.57011.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

>> Or can't you go the other way, say
>> 
>> 	git checkout -t $remote_tracking
>> 
>> to create a local branch forking from the named remote tracking branch?
>
> Sure, but we already have that and we still failed to fix the users,
> so FWIW, I think Dscho's right and we should try fixing the UI next.

What it means is that -t was a broken attempt to help the users at the UI
level, and I can surely see that.

So we need the set of new rules, say, for 1.7.0 release.  A strawman?

Assume that these are the only refs that exist:

    refs/remotes/origin/{master,next,nitfol}
    refs/remotes/xyzzy/{frotz,nitfol}
    refs/heads/master
    refs/tags/v1.0.0

#0. These will stay as is:

 $ git checkout mine               ;# switches to the branch
 $ git checkout $any_committish^0  ;# detaches

#1. These used to detach, but will create a local branch

 $ git checkout origin/next        ;# as if with -t
 $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

#2. These are allowed only when unambiguous and there is no local branch yet.

 $ git checkout next               ;# ok
 $ git checkout frotz              ;# ok (origin is not special)
 $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)

#3. These used to detach, but what should we do?

 $ git checkout v1.0.0             ;# detach, or refuse???
 $ git checkout origin/master      ;# detach, or refuse???

I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
started refusing to detach in case #3, as people who want to detach can
always suffix ^0 or ~0 to make it a general committish.

Did I cover all cases?

^ permalink raw reply

* Re: [PATCH 1/2] user-manual: add global config section
From: Michael J Gruber @ 2009-10-13  7:19 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Michael J Gruber, git, Junio C Hamano, J. Bruce Fields,
	Jonathan Nieder
In-Reply-To: <94a0d4530910121009r52d45522jf1c27dd102db4ad9@mail.gmail.com>

Felipe Contreras venit, vidit, dixit 12.10.2009 19:09:
> On Mon, Oct 12, 2009 at 3:25 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Felipe Contreras venit, vidit, dixit 11.10.2009 22:43:
>>> So that users get to know how to configure git from the get-to with good
>>> practical example (color.ui = auto) that most people would probably like
>>> anyway.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>>  Documentation/user-manual.txt |   27 +++++++++++++++++++++++++++
>>>  1 files changed, 27 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
>>> index 67ebffa..ff2563a 100644
>>> --- a/Documentation/user-manual.txt
>>> +++ b/Documentation/user-manual.txt
>>> @@ -40,6 +40,33 @@ without any explanation.
>>>  Finally, see <<todo>> for ways that you can help make this manual more
>>>  complete.
>>>
>>> +[[getting-started]]
>>> +Getting started
>>> +=============
>>> +
>>> +Git's configuration is distributed among different locations--this manual will
>>> +only to deal with 'global' (for the user) and 'repository' variables, where
>>> +'repository' variables take precedence over 'global' ones.
>>
>> Well, you do talk about "system" below, and that's about it. Also, the
>> configuration is not really distributed among different locations. Most
>> newbies interested in a *D*VCS will misunderstand this (as git having
>> distributed configuration).
>>
>> Alternative:
>>
>> Git's default configuration can be changed on a system wide, global (per
>> user) and local (per repository) level, in the order of increasing
>> precedence.
> 
> When I read that it's not clear if the local level discards the global
> level completely or it's aggregated. If we specify that it's only the
> variables that take precedence it might be clearer:
> 
> Git's configuration is composed of variables that are stored in
> multiple locations: 'system' (all users), 'global' (for the user), and
> 'repository' -- in decreasing order of precedence.

Yep, although established lingo is "options" (not "variables"), and it's
really increasing order, not decreasing.

> 
>>> +
>>> +You would probably want to start setting up something useful:
>>> +------------------------------------------------
>>> +$ git config --global color.ui auto
>>> +------------------------------------------------
>>> +
>>> +This will make prettier the output of certain commands such as `git diff`, but
>>> +that's not important; what is important here is that `color.ui` has been
>>> +stored in the 'global' configuration.
>>
>> This will make certain commands such as `git diff` use colors in the
>> output. What is important here is that the value `auto` for the option
>> `color.ui` has been stored in the 'global' configuration. Use `--system`
>> for the system wide configuration; specifying neither `--system` nor
>> `--global` makes `git config` access the local configuration.
> 
> I think we should only mention (once) the system wide configuration,
> but not cover it. That's for system administrators, not users.
> 
>>> +
>>> +View and manually modify the configuration by opening `~/.gitconfig`:
>>
>> View and manually modify the global configuration by opening
>> `~/.gitconfig` in your editor or using `git config --global --edit`:
> 
> I have separate patches for 'git config --edit', but Junio suggested
> to hold them back because --edit is a relatively new option.
> 
>>> +------------------------------------------------
>>> +[color]
>>> +        ui = auto
>>> +------------------------------------------------
>>> +
>>> +Other locations are `/etc/gitconfig` (system), and `.git/config` (repository).
>>
>> I don't even think we should talk about locations here, "git config -e"
>> should be the first user's way to do it.
> 
> I disagree. Most useful configurations (color.ui, user.email) should
> be global. The complete newbie might think: cool, now I have my git
> properly configured (with 'git config -e'), and then when cloning a
> new repo (s)he would think: ok, git just forgot what I told him. When
> that happens (s)he would have to re-learn and re-configure git.
> 
> When users think about configuration, it's usually a 'global'
> configuration, so that's what we should teach from the beginning and
> make sure they understand the difference between 'global' and
> 'repository' configurations.

Sure. What I meant are the file locations, the actual paths. First
timers should use "git config -e" and "git config --global -e" if they
really want to edit their local and global config files. Better yet,
they should use "git config" and "git config --global" in their set and
get modes, because they make sure that there's no total garbage in the
config. The locations of the files are an implementation detail.

> 
>>> +
>>> +More git configurations will be covered in the rest of the manual, if you want
>>> +to learn more look at linkgit:git-config[1] for details.
>>
>> "Configurations" is ambiguous, it can be easily (mis)understood as
>> "types of configuration" (global, local etc.). Also, the above doesn't
>> really cover even one option. How about:
>>
>> This manual covers many configuration options (such as `color.ui.`). For
>> more details on the `git config` command as well as all configuration
>> options see linkgit:git-config[1].
> 
> Looks better, except s/configuration options/configuration variables/
> 

Uhm, no, for the reason mentioned above. While the man page of git
config is not completely consistent either, we're really talking about
configuration options. An "option" can be set to a "value", and the
thing you pass in order to do that can be called a "variable". For the
most part this is how git-config[1] uses this terminology.

Michael

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
From: Junio C Hamano @ 2009-10-13  7:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Anders Kaseorg, git
In-Reply-To: <4AD42203.6030802@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> The situation that I'm faced quite frequently is that after I find a
> regression, I cannot tell which released version did not have the
> breakage. Hence, the first thing I have to do is to find a good commit.
> Therefore, I jump around in ancient history until I find a good commit.
> Then I start bisect. I certainly do NOT want to be warped back to this
> ancient commit by 'bisect reset'.

Unlike your other message, now, I can see _this_ one making sense very
much.

It is a very good explanation as to why BISECT_START (whose sole purpose
is to go back there) is not a very useful concept.  What you wrote deserve
to go to the "bisect reset" documentation to explain what the optional
<branch> argument (perhaps we would make it a <commit> with Anders's
patch) is good for and how it is intended to be used.

^ permalink raw reply

* Re: [RFC PATCH v2 12/16] Smart fetch and push over HTTP: server side
From: Johannes Sixt @ 2009-10-13  7:30 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-13-git-send-email-spearce@spearce.org>

Shawn O. Pearce schrieb:
> diff --git a/http-backend.c b/http-backend.c
> index 39cfd25..adb3256 100644
> --- a/http-backend.c
> +++ b/http-backend.c

#include "run-command.h" is missing here because you added it already in
patch 10/16 unnecessarily.

> +	if (start_command(&cld))
> +		die_errno("Cannot start %s", argv[0]);
> ...
> +	if (finish_command(&cld))
> +		die("%s terminated with error", argv[0]);

start_command and finish_command already write an error message for you
that includes argv[0] and errno. You can just exit(1) here.

-- Hannes

^ permalink raw reply

* Re: [RFC PATCH v2 01/16] pkt-line: Add strbuf based functions
From: Johannes Sixt @ 2009-10-13  7:29 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1255400715-10508-2-git-send-email-spearce@spearce.org>

Shawn O. Pearce schrieb:
> -int packet_read_line(int fd, char *buffer, unsigned size)
> +static int packet_length(unsigned *ret_len, const char *linelen)
>  {
>  	int n;
> -	unsigned len;
> -	char linelen[4];
> -
> -	safe_read(fd, linelen, 4);
> +	unsigned len = 0;
>  
> -	len = 0;
>  	for (n = 0; n < 4; n++) {
>  		unsigned char c = linelen[n];
>  		len <<= 4;
> @@ -96,8 +116,20 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  			len += c - 'A' + 10;
>  			continue;
>  		}
> -		die("protocol error: bad line length character");
> +		return -1;
>  	}
> +	*ret_len = len;
> +	return 0;
> +}

len can be signed: Valid lengths fit into a signed int. Then you can
'return len;' on success and 'return -1;' on failure and don't need return
the result by reference. packet_read_line() ultimately converts it to int
anyway:

> +int packet_read_line(int fd, char *buffer, unsigned size)
> +{
> +	unsigned len;
> +	char linelen[4];
> +
> +	safe_read(fd, linelen, 4);
> +	if (packet_length(&len, linelen))
> +		die("protocol error: bad line length character");
>  	if (!len)
>  		return 0;
>  	len -= 4;
> @@ -107,3 +139,28 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  	buffer[len] = 0;
>  	return len;
>  }

-- Hannes

^ permalink raw reply

* [PATCH] gitk: Prevent garbage text at the end of the tag description
From: Björn Gustavsson @ 2009-10-13  7:34 UTC (permalink / raw)
  To: Paul Mackerras, git

When first clicking on a commit with a huge diff (in 1000 files
or more), and then clicking on a tag, garbage text could appear
after the tag description.

The problem is that commits are shown incrementally using a
run queue (implemented by the runq() and filerun() procedures).
When the user requests a tag to be shown, there may still be
jobs in the run queue updating the previous commit.

Solve this problem by implementing a new procedure
empty_run_queue() that will empty the run queue and cancel all
outstanding file events. Call it from showtag().

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---

This is more an annoyance than a real bug, but I was quite
surprised the first time I saw it happen and at first searched
for a bug in my own scripts that had created the tags.

This patch is a suggested correction.

Here is an example of a repository where the problem can be
reproduced quite easily:

git://github.com/mfoemmel/erlang-otp.git

 gitk-git/gitk |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a0214b7..ee0d0c3 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -34,12 +34,16 @@ proc run args {
 }
 
 proc filerun {fd script} {
+    global pending_file_events
+
+    set pending_file_events($fd) $fd
     fileevent $fd readable [list filereadable $fd $script]
 }
 
 proc filereadable {fd script} {
-    global runq currunq
+    global runq currunq pending_file_events
 
+    unset pending_file_events($fd)
     fileevent $fd readable {}
     if {$runq eq {} && ![info exists currunq]} {
 	after idle dorunq
@@ -60,7 +64,7 @@ proc nukefile {fd} {
 }
 
 proc dorunq {} {
-    global isonrunq runq currunq
+    global isonrunq runq currunq pending_file_events
 
     set tstart [clock clicks -milliseconds]
     set t0 $tstart
@@ -79,6 +83,7 @@ proc dorunq {} {
 		# file readers return 2 if they could do more straight away
 		lappend runq [list $fd $script]
 	    } else {
+		set pending_file_events($fd) $fd
 		fileevent $fd readable [list filereadable $fd $script]
 	    }
 	} elseif {$fd eq {}} {
@@ -92,6 +97,29 @@ proc dorunq {} {
     }
 }
 
+# Empty the run queue, cancel all outstanding file events,
+# and close all files associated with file events.
+proc empty_run_queue {} {
+    global isonrunq runq pending_file_events
+
+    foreach entry $runq {
+	set fd [lindex $entry 0]
+	set script [lindex $entry 1]
+	if {$fd eq {}} {
+	    unset isonrunq($script)
+	} else {
+	    catch {close $fd}
+	}
+    }
+    set runq {}
+
+    foreach fd [array names pending_file_events] {
+	fileevent $fd readable {}
+	unset pending_file_events($fd)
+	catch {close $fd}
+    }
+}
+
 proc reg_instance {fd} {
     global commfd leftover loginstance
 
@@ -10261,6 +10289,8 @@ proc listrefs {id} {
 proc showtag {tag isnew} {
     global ctext tagcontents tagids linknum tagobjid
 
+    empty_run_queue
+
     if {$isnew} {
 	addtohistory [list showtag $tag 0]
     }
-- 
1.6.5.2.gd6127

^ permalink raw reply related

* Bad URL passed to RA lay
From: m.skoric @ 2009-10-13  7:35 UTC (permalink / raw)
  To: git

Hi List,

i have a problem with git-svn clone / fetch. I get following error while doing one of previous command -> "Bad URL passed to RA lay"
This happens because a branch doesn't exists in svn anymore and git wants to retrieve data from it. Here is the complete error message

Initializing parent: Abo-Uebernahme (Bug #994)@341
Found possible branch point: "quoted"..trunk => "quoted"...Abo-Uebernahme (Bug #994), 203
Found branch parent: (Abo-Uebernahme (Bug #994)@341) bb831869748c98bf97d105c5894ae65331c95c08
Bad URL passed to RA layer: Malformed URL for repository at /usr/bin/git-svn line 4311

git version 1.6.3.3

Aynone else has this Problem?
Can anyone help me?

Thanks in advance

Majk
______________________________________________________
GRATIS für alle WEB.DE-Nutzer: Die maxdome Movie-FLAT!
Jetzt freischalten unter http://movieflat.web.de

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a   branch
From: Johannes Sixt @ 2009-10-13  7:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Anders Kaseorg, git
In-Reply-To: <7v3a5n3hgn.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Half of the time, the commit you test in your "git bisect" section would
> be a "good" one, and immediately after you tell it "bisect good", it tells
> you that some _other_ commit you marked "bad" is the first bad commit.  In
> such a case, you won't be on the commit that the bisect has found.

Oh, yes, very true; but it is very close. But the commit that git bisect
reset warps me to is perhaps 1000 steps in history away. I certainly do
not want to go there, ever, because I want to go back near the bad commit
right away. (Think of fewer files changed means less build time.) If git
bisect reset would check out the bad commit, this would be *very* convenient.

-- Hannes

^ permalink raw reply

* Re: Questions about the new
From: Sergio Callegari @ 2009-10-13  7:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7v8wfge2zu.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Sergio Callegari <sergio.callegari@gmail.com> writes:
>
>   
>> If I want to replace some commit X by some commit X' I merely need to
>> modify the
>> parent information of all the commits that are child of X so that they
>> pretend
>> to be child of X', or am I missing something?
>>     
>
> You need to find all the commits that are child of X in the first place.
> What should happen if your colleague has such a commit in his repository
> (which you haven't fetched from yet), you enumerated all children of X
> known to you in your graft file and then you fetch from him?  You need to
> enumerate all children of X again to keep the graft file up to date.
>   
Ok, that is enlightening. When trying to sort out the differences, 
advantages and disadvantages of
operating on arcs (grafts like) or on nodes (replacements like) I was 
thinking local, rather than
distributed. Now the advantage of working on nodes is much clearer to me.

Thanks as usual for the very clear explanation.

Sergio

^ permalink raw reply

* Re: [PATCH] bisect reset: Allow resetting to any commit, not just a  branch
From: Junio C Hamano @ 2009-10-13  8:33 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Anders Kaseorg, git
In-Reply-To: <4AD43002.5080003@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
>> Half of the time, the commit you test in your "git bisect" section would
>> be a "good" one, and immediately after you tell it "bisect good", it tells
>> you that some _other_ commit you marked "bad" is the first bad commit.  In
>> such a case, you won't be on the commit that the bisect has found.
>
> Oh, yes, very true; but it is very close. But the commit that git bisect
> reset warps me to is perhaps 1000 steps in history away. I certainly do
> not want to go there, ever, because I want to go back near the bad commit
> right away. (Think of fewer files changed means less build time.) If git
> bisect reset would check out the bad commit, this would be *very* convenient.

I agree that "git bisect reset-and-detach-at-the-first-bad-one" would make
a lot of sense.

In my workflow, after I chased a bug in frotz, I often do

    $ git name-rev $the_bad_one_that_was_found
    
to learn what was the first tagged release that has the bug, and create a
topic from there:

    $ git checkout -b jc/maint-X.Y.Z-fix-frotz $the_bad_one_that_was_found

so that the fix I'd build on the commit can be merged initially to 'pu',
then 'next', then 'maint-X.Y.Z' and upwards to 'master', but all of that
is done after "git bisect reset" to switch back to the 'master' branch.
It is cumbersome to have to type (actually, I use the cut buffer in screen)
the commit object name of the first bad one twice.

It certainly sounds attractive if we can do:

    $ git bisect reset-and-detach-at-the-first-bad-one
    $ git name-rev HEAD
    $ git checkout -b jc/maint-X.Y.Z-fix-frotz
    $ hack hack hack
    $ git commit

But at that point we are not talking about switching to arbitrary commit
anymore.

^ permalink raw reply

* Re: [RFC PATCH v2 16/16] Smart HTTP fetch: gzip requests
From: Junio C Hamano @ 2009-10-13  8:38 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Daniel Barkalow
In-Reply-To: <1255400715-10508-17-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> diff --git a/remote-curl.c b/remote-curl.c
> index 31d1d34..d53215d 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -261,11 +261,12 @@ static size_t rpc_in(const void *ptr, size_t eltsize,
>  	return size;
>  }
>  
> -static int post_rpc(struct rpc_state *state)
> +static int post_rpc(struct rpc_state *state, int use_gzip)
>  {
>  	struct active_request_slot *slot;
>  	struct slot_results results;
>  	struct curl_slist *headers = NULL;
> +	unsigned char *gzip_body = NULL;
>  	int err = 0, large_request = 0;
>  
>  	/* Try to load the entire request, if we can fit it into the
> @@ -311,6 +313,48 @@ static int post_rpc(struct rpc_state *state)
> ...
> +		curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);

cc1: warnings being treated as errors
In function 'post_rpc',
    inlined from 'one_shot_rpc_service' at remote-curl.c:433:
remote-curl.c:350: error: call to '_curl_easy_setopt_err_postfields' declared with attribute warning: curl_easy_setopt expects a void* or char* argument for this option
make: *** [remote-curl.o] Error 1

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-13  8:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Rast, Johannes Schindelin, Euguess, Mikael Magnusson,
	Matthieu Moy, Jeff King, Jay Soffian, git
In-Reply-To: <7vljjf226t.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> So we need the set of new rules, say, for 1.7.0 release.  A strawman?
>
> Assume that these are the only refs that exist:
>
>     refs/remotes/origin/{master,next,nitfol}
>     refs/remotes/xyzzy/{frotz,nitfol}
>     refs/heads/master

Sorry, I had this as refs/heads/{master,mine} in my initial draft but
removed the 'mine' branch by mistake; the first item in #0 does not make
sense without it.

>     refs/tags/v1.0.0
>
> #0. These will stay as is:
>
>  $ git checkout mine               ;# switches to the branch
>  $ git checkout $any_committish^0  ;# detaches
>
> #1. These used to detach, but will create a local branch
>
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
>
> #2. These are allowed only when unambiguous and there is no local branch yet.
>
>  $ git checkout next               ;# ok
>  $ git checkout frotz              ;# ok (origin is not special)
>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
>
> #3. These used to detach, but what should we do?
>
>  $ git checkout v1.0.0             ;# detach, or refuse???
>  $ git checkout origin/master      ;# detach, or refuse???
>
> I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
> started refusing to detach in case #3, as people who want to detach can
> always suffix ^0 or ~0 to make it a general committish.
>
> Did I cover all cases?

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-13  8:51 UTC (permalink / raw)
  To: Junio C Hamano, Johannes Schindelin
  Cc: Euguess, Mikael Magnusson, Matthieu Moy, Jeff King, Jay Soffian,
	git
In-Reply-To: <7vljjf226t.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> 
> What it means is that -t was a broken attempt to help the users at the UI
> level, and I can surely see that.
> 
> So we need the set of new rules, say, for 1.7.0 release.  A strawman?

I feel somewhat uneasy commenting on this because I have a history of
writing just-barely-workable UIs.  That being said:

> Assume that these are the only refs that exist:
> 
>     refs/remotes/origin/{master,next,nitfol}
>     refs/remotes/xyzzy/{frotz,nitfol}
>     refs/heads/master
>     refs/tags/v1.0.0
> 
> #0. These will stay as is:
> 
>  $ git checkout mine               ;# switches to the branch
>  $ git checkout $any_committish^0  ;# detaches
> 
> #1. These used to detach, but will create a local branch
> 
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

Agreed, though I'm still in favour of a cleaner syntax for explicit
detaching.  (Cleaner in the sense that ^0 is documented as having a
completely different purpose and only works by accident.)

> #2. These are allowed only when unambiguous and there is no local branch yet.
> 
>  $ git checkout next               ;# ok
>  $ git checkout frotz              ;# ok (origin is not special)
>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)

I'm weakly leaning towards refusing all three, as the user should be
required to explicitly say a remote branch should be involved.

(Weakly because there's also a certain DWIM advantage to 'git checkout
sometopic'...)

> #3. These used to detach, but what should we do?
> 
>  $ git checkout v1.0.0             ;# detach, or refuse???

Refuse, on the grounds that the main goal here is not detaching unless
specifically told to.  (Having a branch called v1.0.0 is worse, as it
would just cause a lot of confusion and/or a refusal at the next
checkout.)

>  $ git checkout origin/master      ;# detach, or refuse???

This seems to be the trickiest of them.  Maybe check out 'master', to
make the process repeatable.  Imagine, in your setting,

  git checkout origin/next           ;# creates 'next' as with -t
  git checkout -                     ;# back
  git checkout origin/next           ;# should go to 'next' again

Then again, that would trade the confusion of detaching for the
confusion of not checking out the exact commit that the user
specified.  Worse, 'next' could conceivably be tracking (as per
branch.next.merge) some entirely different branch, making the "Your
branch is behind..." message misleading.

> I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
> started refusing to detach in case #3, as people who want to detach can
> always suffix ^0 or ~0 to make it a general committish.
> 
> Did I cover all cases?

Some that come to mind:

#3a. Other refs apart from tags that currently detach:

  git fetch origin master            ;# or even sillier, 'git fetch . master'
  git checkout FETCH_HEAD            ;# used to detach; refuse?

#3b. Full specifiers that currently detach:

  git checkout refs/heads/master     ;# could eventually attach
  git checkout heads/master          ;# same

#0a. Should probably detach if the previous checkout was detached:

  git checkout -                     ;# detach if previous was detached?
  git checkout @{-1}                 ;# same

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-13  9:24 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Johannes Schindelin, Euguess, Mikael Magnusson, Matthieu Moy,
	Jeff King, Jay Soffian, git
In-Reply-To: <200910131051.47117.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

>> #0. These will stay as is:
>> 
>>  $ git checkout mine               ;# switches to the branch
>>  $ git checkout $any_committish^0  ;# detaches
>> 
>> #1. These used to detach, but will create a local branch
>> 
>>  $ git checkout origin/next        ;# as if with -t
>>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
>
> Agreed, though I'm still in favour of a cleaner syntax for explicit
> detaching.  (Cleaner in the sense that ^0 is documented as having a
> completely different purpose and only works by accident.)

Oh, ^0 was just one way to make sure a committish is not a refname.  If
you have an abbreviated hexadecimal commit object name, that would also
detach, which should fall into category #0.  Sorry for the omission.

>> #2. These are allowed only when unambiguous and there is no local branch yet.
>> 
>>  $ git checkout next               ;# ok
>>  $ git checkout frotz              ;# ok (origin is not special)
>>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
>
> I'm weakly leaning towards refusing all three, as the user should be
> required to explicitly say a remote branch should be involved.
>
> (Weakly because there's also a certain DWIM advantage to 'git checkout
> sometopic'...)

I thought this was the primary point of what Dscho has been advocating.

>> #3. These used to detach, but what should we do?
>> 
>>  $ git checkout v1.0.0             ;# detach, or refuse???
>
> Refuse, on the grounds that the main goal here is not detaching unless
> specifically told to.  (Having a branch called v1.0.0 is worse, as it
> would just cause a lot of confusion and/or a refusal at the next
> checkout.)
>
>>  $ git checkout origin/master      ;# detach, or refuse???
>
> This seems to be the trickiest of them.  Maybe check out 'master', to
> make the process repeatable.  Imagine, in your setting,
>
>   git checkout origin/next           ;# creates 'next' as with -t
>   git checkout -                     ;# back
>   git checkout origin/next           ;# should go to 'next' again
>
> Then again, that would trade the confusion of detaching for the
> confusion of not checking out the exact commit that the user
> specified.  Worse, 'next' could conceivably be tracking (as per
> branch.next.merge) some entirely different branch, making the "Your
> branch is behind..." message misleading.

As I said already in the thread, I think that is a misguided attempt to
half-hide the fact that there are origin/next (tracking branch) and next
(a fork of it), that are two separate entities.  It is misguided because
the user needs to understand and take advantage of the distinction to do
anything; in other words, it is not even an unnecessary complexity.

So I am very doubtful about the benefit of checking out 'master' when
the user explicitly tells us to check out 'origin/master', only because
the former forked from the latter.

> Some that come to mind:
>
> #3a. Other refs apart from tags that currently detach:
>
>   git fetch origin master            ;# or even sillier, 'git fetch . master'
>   git checkout FETCH_HEAD            ;# used to detach; refuse?

> #3b. Full specifiers that currently detach:
>
>   git checkout refs/heads/master     ;# could eventually attach
>   git checkout heads/master          ;# same

I'd throw both of these into category #3.

Anything that is valid "ref" (i.e. what dwim_ref() groks) that is not
a remote tracking branch (which creates a corresponding local branch)
can refuse to avoid unintended detachment by newbies.

> #0a. Should probably detach if the previous checkout was detached:
>
>   git checkout -                     ;# detach if previous was detached?
>   git checkout @{-1}                 ;# same

Perhaps.

So to recap, "git checkout $token" would:

 * If dwim_ref() groks $token, and

   - if it resolves to refs/heads/*, that is checking out a local branch;

   - if it resolves to refs/remotes/*, and if there is no corresponding
     local branch, create one forked from there, as if -t was given;

   - everything else we used to detach, but we refuse in 1.7.0, to make it
     harder for newbies to detach.

 * If check_ref_format() is happy with $token, get_sha1() does not grok
   $token, and there is only one ref of the form refs/remotes/$o/$token
   then we pretend as if -t $o/$token was given and create a local branch
   $token forked from it.

 * Otherwise, we always detach.

Note that "checkout -" and "checkout @{-4}" are part of dwim_ref() family.

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Sixt @ 2009-10-13  9:32 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Rast, Johannes Schindelin, Euguess, Mikael Magnusson,
	Matthieu Moy, Jeff King, Jay Soffian, git
In-Reply-To: <7vljjf226t.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> #1. These used to detach, but will create a local branch
> 
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

If I did 'git checkout origin/next' last week, I will already have a
branch next. What should happen if I do it again today?

I think that it should DWIM: If last week's next fast-fowards to this
week's origin/next (*and* next is the branch that tracks origin/next),
then the fast foward should happen. Otherwise 'git checkout origin/next'
should fail.

This way, if I built on last week's next, I will be notified; but if I
only want to browse history, then I won't be impeded by the existence of next.

-- Hannes

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox