git-shortlog hangs on bare repo without --bare option

git-shortlog hangs on bare repo without --bare option

[Garry Dolley]

I didn't see this happen with git 1.5.x, but if you do git-shortlog on a bare 
repo, without specifying --bare, the command will seemingly hang indefinitely.

Here's my output:

[garry@garry-thinkpad] ~/dev (master) $ cd ebay4r/                       <----- Repo with working tree (non-bare)
[garry@garry-thinkpad] ~/dev/ebay4r (master) $ git shortlog -e -s
   130  Garry Dolley <gdolley@arpnetworks.com>
[garry@garry-thinkpad] ~/dev/ebay4r (master) $ cd ..
[garry@garry-thinkpad] ~/dev (master) $ git clone --mirror ebay4r/ ebay4r-bare
Initialized empty Git repository in /home/garry/dev/ebay4r-bare/
[garry@garry-thinkpad] ~/dev (master) $ cd ebay4r-bare/
[garry@garry-thinkpad] ~/dev/ebay4r-bare (master) $ git shortlog -e -s
<hang>                                                                   <----- *** It hangs here ***

[garry@garry-thinkpad] ~/dev/ebay4r-bare (master) $ git --bare shortlog -e -s
   130  Garry Dolley <gdolley@arpnetworks.com>
[garry@garry-thinkpad] ~/dev/ebay4r-bare (master) $ 


If newlines break this output in your reader, you can also view it 
here: http://pastie.org/261134

I'm using git 1.6.0.1

Is this a bug?

-- 
Garry Dolley
ARP Networks, Inc.
818-206-0181
Los Angeles County REACT, Unit 336
WQGK336

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

Garry Dolley <gdolley@arpnetworks.com> writes:

> [garry@garry-thinkpad] ~/dev/ebay4r-bare (master) $ git shortlog -e -s
> <hang>                                                                   <----- *** It hangs here ***

Tryp typing ^D here.  I think it is operating in the filter mode.

Re: git-shortlog hangs on bare repo without --bare option

[Garry Dolley]

On Wed, Aug 27, 2008 at 01:32:31PM -0700, Junio C Hamano wrote:
> Garry Dolley <gdolley@arpnetworks.com> writes:
> 
> > [garry@garry-thinkpad] ~/dev/ebay4r-bare (master) $ git shortlog -e -s
> > <hang>                                                                   <----- *** It hangs here ***
> 
> Tryp typing ^D here.  I think it is operating in the filter mode.

Yeah, ^D just gets me back to the prompt, no output.  It does indeed
seem to be waiting for something.

-- 
Garry Dolley
http://scie.nti.st

Re: git-shortlog hangs on bare repo without --bare option

[René Scharfe]

Garry Dolley schrieb:
> I didn't see this happen with git 1.5.x, but if you do git-shortlog on a bare 
> repo, without specifying --bare, the command will seemingly hang indefinitely.

FWIW, I tried git 1.5.0 and it hangs, too.  Which exact version did work
for you?  Could you, based on it, bisect the commit that introduced this
behaviour?

As a workaround, you can use "git log | git shortlog".

René

Re: git-shortlog hangs on bare repo without --bare option

[Garry Dolley]

On Wed, Aug 27, 2008 at 11:40:52PM +0200, Ren?? Scharfe wrote:
> Garry Dolley schrieb:
> > I didn't see this happen with git 1.5.x, but if you do git-shortlog on a bare 
> > repo, without specifying --bare, the command will seemingly hang indefinitely.
> 
> FWIW, I tried git 1.5.0 and it hangs, too.  Which exact version did work
> for you?  Could you, based on it, bisect the commit that introduced this
> behaviour?

I just tried it with 1.5.6.2, and it hung.  Unfortunately, I had
several versions of git inside my ~/src as I upgraded over the
months, and just last week I decided to blow away the old ones, so I
don't know what version it was that I saw this work. :(

Either way, it's too big a deal, I just wanted to point out what I
saw.

If it helps, this is the last few lines of strace before it hangs:

stat(".git", 0x7fff6e44dd90)            = -1 ENOENT (No such file or directory)
open(".git/config", O_RDONLY)           = -1 ENOENT (No such file or directory)
pipe([3, 4])                            = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f7a6642d790) = 12674
dup2(3, 0)                              = 0
close(3)                                = 0
close(4)                                = 0
select(1, [0], NULL, [0], NULL

So looks like it is waiting to be fed some input?  It's hungry :)

> As a workaround, you can use "git log | git shortlog".

Also, 'git --bare shortlog' works too.

-- 
Garry Dolley
http://scie.nti.st
ARP Networks, Inc.
818-206-0181
Los Angeles County REACT, Unit 336
WQGK336

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Garry Dolley schrieb:
>> I didn't see this happen with git 1.5.x, but if you do git-shortlog on a bare 
>> repo, without specifying --bare, the command will seemingly hang indefinitely.
>
> FWIW, I tried git 1.5.0 and it hangs, too.  Which exact version did work
> for you?  Could you, based on it, bisect the commit that introduced this
> behaviour?
>
> As a workaround, you can use "git log | git shortlog".

It is not a workaround.  Shortlog can work as a filter to "git log", but
if you give revs to work on, e.g. "git shortlog -s -n v1.5.6.5.." or "git
shortlog HEAD", it can generate the log and feed to itself.

Somehow it (perhaps incorrectly, I did not look at the command line input)
judging that your command line is telling it not to do the log generation
itself but instead you will feed the log from its standard input, which
happens to be your terminal.  So another workaround would be to type the
series of log messages ;-)

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

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

> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Garry Dolley schrieb:
>>> I didn't see this happen with git 1.5.x, but if you do git-shortlog on a bare 
>>> repo, without specifying --bare, the command will seemingly hang indefinitely.
>>
>> FWIW, I tried git 1.5.0 and it hangs, too.  Which exact version did work
>> for you?  Could you, based on it, bisect the commit that introduced this
>> behaviour?
>>
>> As a workaround, you can use "git log | git shortlog".
>
> It is not a workaround.  Shortlog can work as a filter to "git log", but
> if you give revs to work on, e.g. "git shortlog -s -n v1.5.6.5.." or "git
> shortlog HEAD", it can generate the log and feed to itself.
>
> Somehow it (perhaps incorrectly, I did not look at the command line input)
> judging that your command line is telling it not to do the log generation
> itself but instead you will feed the log from its standard input, which
> happens to be your terminal.  So another workaround would be to type the
> series of log messages ;-)

And I think this is related to the complexity that snuck in when worktree
feature was added to the setup sequence.

Untested, but I think this would help.

 setup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git c/setup.c w/setup.c
index 6cf9094..296f712 100644
--- c/setup.c
+++ w/setup.c
@@ -461,7 +461,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			inside_git_dir = 1;
 			if (!work_tree_env)
 				inside_work_tree = 0;
-			setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+			set_git_dir(".");
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

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

> And I think this is related to the complexity that snuck in when worktree
> feature was added to the setup sequence.
>
> Untested, but I think this would help.

While it might help, I see there are more serious issue here.

If you start "git-shortlog" (or anything that has USE_PAGER but not
RUN_SETUP in git.c) inside your project/.git directory (or a bare one, for
that matter), the call sequence would become like this:

git.c::main()
 -> handle_internal_command()
   -> run_command()
     -> commit_pager_choice()
       -> setup_pager() (this is in pager.c)
         -> git_config(git_default_config, NULL)
           -> git_path("config")
             -> get_git_dir()
               -> setup_git_env()
                 This sets git_dir to ".git"!!!!

And that was part of the reason why "git-shortlog" started from a bare
repository did not even notice that the default "HEAD" that is given
internally by the program is not a valid ref (because it tries to read
from .git/HEAD without the patch I sent earlier).

This may be repeating what Jeff said earlier in another thread, but I
think we should rethink the start-up sequence carefully.  Ideally (I am
thinking aloud)...

 * Have a single function "git_setup()" that notices --bare and --git-dir
   from the command line and GIT_DIR environment, and does the discovery
   of git_dir (if not told with the command line or environment
   explicitly); if we do not find it, do *NOT* barf.  Just record the
   facts it finds somewhere (e.g. the location of git_dir, absense of
   git_dir, if the repository is explicitly bare, etc.).

 * As the next step, still in this single function, if we have git_dir,
   find out where the work_tree is, paying attention to --work-tree from
   the command line and GIT_WORK_TREE environment if exists.  If we do not
   have work tree, do *NOT* barf.  Again, just record the facts it finds.
   If we do not have --work-tree from the command line and if we are not
   told that it is --bare from the command line, then we may need to open
   the config to see where core.worktree points at.

 * Before returning from that single setup function, i think it is Ok to
   make it mimick the current git_setup_directory_gently(), cd up to the
   normal work-tree top if needed, and make it return prefix.

 * After all of the above is done, then we can start futzing with other
   things such as pager.  At that point we know we can read from the
   correct configuratoin.

 * Each individual command may decide to refuse to work in the absense of
   git_dir and/or work_tree, by calling require_work_tree(), etc.

Hmm?

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

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

> And I think this is related to the complexity that snuck in when worktree
> feature was added to the setup sequence.
>
> Untested, but I think this would help.

While this might help, I see there are more serious issues here.

If you start "git-shortlog" (or anything that has USE_PAGER but not
RUN_SETUP in git.c) inside your project/.git directory (or a bare one, for
that matter), the call sequence would become like this:

git.c::main()
 -> handle_internal_command()
   -> run_command()
     -> commit_pager_choice()
       -> setup_pager() (this is in pager.c)
         -> git_config(git_default_config, NULL)
           -> git_path("config")
             -> get_git_dir()
               -> setup_git_env()
                 This sets git_dir to ".git"!!!!

And that was part of the reason why "git-shortlog" started from a bare
repository did not even notice that the default "HEAD" that is given
internally by the program is not a valid ref (because it tries to read
from .git/HEAD without the patch I sent earlier).  Oh, of course that
git_config() won't read from the right config file either.

This may be repeating what Jeff said earlier in another thread, but I
think we should rethink the start-up sequence carefully.  Ideally (I am
thinking aloud)...

 * Have a single function "git_setup()" that notices --bare and --git-dir
   from the command line and GIT_DIR environment, and does the discovery
   of git_dir (if not told with the command line or environment
   explicitly); if we do not find it, do *NOT* barf.  Just record the
   facts it finds somewhere (e.g. the location of git_dir, absense of
   git_dir, if the repository is explicitly bare, etc.).

 * As the next step, still in this single function, if we have git_dir,
   find out where the work_tree is, paying attention to --work-tree from
   the command line and GIT_WORK_TREE environment if exists.  If we do not
   have work tree, do *NOT* barf.  Again, just record the facts it finds.
   If we do not have --work-tree from the command line and if we are not
   told that it is --bare from the command line, then we may need to open
   the config to see where core.worktree points at.

 * Before returning from that single setup function, i think it is Ok to
   make it mimick the current git_setup_directory_gently(), cd up to the
   normal work-tree top if needed, and make it return prefix.

 * After all of the above is done, then we can start futzing with other
   things such as pager.  At that point we know we can read from the
   correct configuratoin.

 * Each individual command may decide to refuse to work in the absense of
   git_dir and/or work_tree, by calling require_work_tree(), etc.

Hmm?

Re: git-shortlog hangs on bare repo without --bare option

[Jeff King]

On Wed, Aug 27, 2008 at 04:16:32PM -0700, Junio C Hamano wrote:

> While this might help, I see there are more serious issues here.
> 
> If you start "git-shortlog" (or anything that has USE_PAGER but not
> RUN_SETUP in git.c) inside your project/.git directory (or a bare one, for
> that matter), the call sequence would become like this:
> 
> git.c::main()
>  -> handle_internal_command()
>    -> run_command()
>      -> commit_pager_choice()
>        -> setup_pager() (this is in pager.c)
>          -> git_config(git_default_config, NULL)
>            -> git_path("config")
>              -> get_git_dir()
>                -> setup_git_env()
>                  This sets git_dir to ".git"!!!!
> [...]
> This may be repeating what Jeff said earlier in another thread, but I
> think we should rethink the start-up sequence carefully.  Ideally (I am
> thinking aloud)...

Yep. This is the exact same problem I have been complaining about for a
while.

> [some implementation ideas]

Yes, this is roughly what I have been thinking of, as well. IOW,
unconditionally find everything right at the beginning, but let us
decide whether to barf later, when we know what command we are running.

A few complexities I found were:

  - some things really want no setup, like init and clone. I don't
    recall the exact issues, but trying obvious things caused breakage
    there.

  - the setup needs to not just be part of git.c, but factored out so
    non-builtin C programs can use it. I don't recall if there are
    issues related to git-sh-setup that need to be dealt with.

I think my approach was something like "let's just unconditionally call
setup_git_directory() in git.c, and wrap it with a static variable to
make sure we don't run it twice." but I ran into a host of issues. Sorry
I can't remember specifics.

-Peff

Re: git-shortlog hangs on bare repo without --bare option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> A few complexities I found were:
>
>   - some things really want no setup, like init and clone. I don't
>     recall the exact issues, but trying obvious things caused breakage
>     there.

An obvious thing is to start these commands in a subdirectory of an
existing git worktree.

>   - the setup needs to not just be part of git.c, but factored out so
>     non-builtin C programs can use it.

Obviously.

Re: git-shortlog hangs on bare repo without --bare option

[Nguyen Thai Ngoc Duy]

On 8/28/08, Junio C Hamano <gitster@pobox.com> wrote:
>  This may be repeating what Jeff said earlier in another thread, but I
>  think we should rethink the start-up sequence carefully.  Ideally (I am
>  thinking aloud)...
>
>   * Have a single function "git_setup()" that notices --bare and --git-dir
>    from the command line and GIT_DIR environment, and does the discovery
>    of git_dir (if not told with the command line or environment
>    explicitly); if we do not find it, do *NOT* barf.  Just record the
>    facts it finds somewhere (e.g. the location of git_dir, absense of
>    git_dir, if the repository is explicitly bare, etc.).
>
>   * As the next step, still in this single function, if we have git_dir,
>    find out where the work_tree is, paying attention to --work-tree from
>    the command line and GIT_WORK_TREE environment if exists.  If we do not
>    have work tree, do *NOT* barf.  Again, just record the facts it finds.
>    If we do not have --work-tree from the command line and if we are not
>    told that it is --bare from the command line, then we may need to open
>    the config to see where core.worktree points at.

Part of worktree issues comes from the fact that worktree setup is
spread over many functions, starting at setup_git_dir*() and ending at
setup_work_tree().
So setting it up in a single function and turning setup_work_tree()
into "check and die()" logic sounds great.
-- 
Duy