threads / patch / 17936

patchAdd bare repository indicator for __git_ps1

Subject: [PATCH] Add bare repository indicator for __git_ps1

## tl;dr

18 messages between Feb 21, 2009 and Feb 25, 2009. Diffs are folded; open one to read it.

replies: 17people: 6as markdown or json

Marius Storm-Olsen· Feb 21, 2009, 14:48 UTC · lore

Prefixes the branch name with "BARE:" if you're in a bare repository.

Signed-off-by: Marius Storm-Olsen <git@storm-olsen.com>
---
 Ok, had some free cycles, so here's fixed up version.
 Based on next this time
 contrib/completion/git-completion.bash |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +8 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ec587d2..e585d40 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -135,11 +135,17 @@ __git_ps1 ()
 			fi
 		fi
 
+		local c
+
+		if [ "true" = "$(git config --bool core.bare 2>/dev/null)" ]; then
+			c="BARE:"
+		fi
+
 		if [ -n "$b" ]; then
 			if [ -n "${1-}" ]; then
-				printf "$1" "${b##refs/heads/}$w$i$r"
+				printf "$1" "$c${b##refs/heads/}$w$i$r"
 			else
-				printf " (%s)" "${b##refs/heads/}$w$i$r"
+				printf " (%s)" "$c${b##refs/heads/}$w$i$r"
 			fi
 		fi
 	fi

-- 
1.6.2.rc1.20.g8c5b
Marius Storm-Olsen· Feb 21, 2009, 14:53 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen said the following on 21.02.2009 15:48:
Show 7 quoted lines
> Prefixes the branch name with "BARE:" if you're in a
> bare repository.
> 
> Signed-off-by: Marius Storm-Olsen <git@storm-olsen.com>
> ---
>  Ok, had some free cycles, so here's fixed up version.
>  Based on next this time
*grmbl* This is v2 of the patch, of course.

-- .marius

Junio C Hamano· Feb 21, 2009, 19:29 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen <git@storm-olsen.com> writes:
> Prefixes the branch name with "BARE:" if you're in a
> bare repository.

The updated code may correctly detect when you are in such a situation, but I have to wonder why anybody would even want to be reminded that he is in a bare repository to begin with.

For doing any usual work of growing history, you would work inside a repository with an work tree. The only occasion you would *go* to a bare repository would be to tweak, futz with and fix one that is used as a distribution point, isn't it? You usually update such a repository by pushing into it, so your being there would be a result of very conscious act of chdir'ing into it yourself, and you wouldn't be spending too much time in there anyway.

There may be a different workflow where you would stay in a bare repository for an extended period of time and you would benefit from such a reminder like this patch adds, but I do not think of one.

Care to enlighten?
Marius Storm-Olsen· Feb 21, 2009, 19:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Junio C Hamano said the following on 21.02.2009 20:29:
Show 7 quoted lines
> Marius Storm-Olsen <git@storm-olsen.com> writes:
>> Prefixes the branch name with "BARE:" if you're in a bare
>> repository.
> 
> The updated code may correctly detect when you are in such a
> situation, but I have to wonder why anybody would even want to be
> reminded that he is in a bare repository to begin with.

Yeah I just noticed that, when I got some few cycles to spare. So I pushed out a v3 of that patch *sigh*

Show 14 quoted lines
> For doing any usual work of growing history, you would work inside
> a repository with an work tree.  The only occasion you would *go*
> to a bare repository would be to tweak, futz with and fix one that
> is used as a distribution point, isn't it?  You usually update such
> a repository by pushing into it, so your being there would be a
> result of very conscious act of chdir'ing into it yourself, and you
> wouldn't be spending too much time in there anyway.
> 
> There may be a different workflow where you would stay in a bare 
> repository for an extended period of time and you would benefit
> from such a reminder like this patch adds, but I do not think of
> one.
> 
> Care to enlighten?

Right, I have quite a few repos on my machine which are just bare, as I use them gather branches and push out again. (http://repo.or.cz/w/git/platforms.git is one of them) However, it's probably just me, since I could just as easily put them in a proper directory structure to indicate their bareness.

Anyways, I just thought it would fairly "low cost" to add, and nice to have.

Consider it, as Linus coined the term, a throw-away patch. I can easily put it in my .bashrc instead. :)

-- .marius

Junio C Hamano· Feb 22, 2009, 16:49 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen <marius@trolltech.com> writes:
Show 20 quoted lines
>> For doing any usual work of growing history, you would work inside
>> a repository with an work tree.  The only occasion you would *go*
>> to a bare repository would be to tweak, futz with and fix one that
>> is used as a distribution point, isn't it?  You usually update such
>> a repository by pushing into it, so your being there would be a
>> result of very conscious act of chdir'ing into it yourself, and you
>> wouldn't be spending too much time in there anyway.
>>
>> There may be a different workflow where you would stay in a bare
>> repository for an extended period of time and you would benefit
>> from such a reminder like this patch adds, but I do not think of
>> one.
>>
>> Care to enlighten?
>
> Right, I have quite a few repos on my machine which are just bare, as I
> use them gather branches and push out
> again. (http://repo.or.cz/w/git/platforms.git is one of them) However,
> it's probably just me, since I could just as easily put them in a proper
> directory structure to indicate their bareness.

Ah, so "gather branches and push out again" would look something like this?

    $ cd /pub/some/where/platforms.git
    $ git fetch platform1 ;# perhaps with master:one/master mapping
    $ git fetch platform2 ;# perhaps with master:two/master
    $ git push public

Then it is very understandable that you would spend time inside a bare repository. I do not understand the need for GIT_DIR! thing even less, but since we have that there already, I do not see a reason not to add this to the queue.

Marius Storm-Olsen· Feb 23, 2009, 07:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Junio C Hamano said the following on 22.02.2009 17:49:
Show 25 quoted lines
> Marius Storm-Olsen <marius@trolltech.com> writes:
>>> There may be a different workflow where you would stay in a
>>> bare repository for an extended period of time and you would
>>> benefit from such a reminder like this patch adds, but I do not
>>> think of one.
>>> 
>>> Care to enlighten?
>> Right, I have quite a few repos on my machine which are just
>> bare, as I use them gather branches and push out again.
>> (http://repo.or.cz/w/git/platforms.git is one of them) However, 
>> it's probably just me, since I could just as easily put them in a
>> proper directory structure to indicate their bareness.
> 
> Ah, so "gather branches and push out again" would look something
> like this?
> 
>     $ cd /pub/some/where/platforms.git
>     $ git fetch platform1 ;# perhaps with master:one/master mapping
>     $ git fetch platform2 ;# perhaps with master:two/master
>     $ git push public
> 
> Then it is very understandable that you would spend time inside a
> bare repository.  I do not understand the need for GIT_DIR! thing
> even less, but since we have that there already, I do not see a
> reason not to add this to the queue.
Indeed that's somewhat how I work.
Also, given the new GIT_DIR! "feature", I cannot simply keep my 
"BARE:" in my own .bash_rc anymore, since then I'd just get
     (BARE:GIT_DIR!)
which is less than useful. So, given that the overhead and impact to 
the current logic is minimal, I would appreciate the patch being queued.
Thanks!
-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'
Shawn O. Pearce· Feb 23, 2009, 15:42 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen <marius@trolltech.com> wrote:
Show 18 quoted lines
> Junio C Hamano said the following on 21.02.2009 20:29:
>> Marius Storm-Olsen <git@storm-olsen.com> writes:
>>> Prefixes the branch name with "BARE:" if you're in a bare
>>> repository.
>
>> There may be a different workflow where you would stay in a bare  
>> repository for an extended period of time and you would benefit
>> from such a reminder like this patch adds, but I do not think of
>> one.
>
> Right, I have quite a few repos on my machine which are just bare, as I 
> use them gather branches and push out again.  
> (http://repo.or.cz/w/git/platforms.git is one of them) However, it's  
> probably just me, since I could just as easily put them in a proper  
> directory structure to indicate their bareness.
>
> Anyways, I just thought it would fairly "low cost" to add, and nice to  
> have.

Its not that low of a cost, its an extra fork+exec per prompt when in a .git/ or a bare repository. Neither is very common when compared to a workdir, Junio's right about that. But its YAFE. ;)

> Consider it, as Linus coined the term, a throw-away patch. I can easily 
> put it in my .bashrc instead. :)

Like Junio, I'm not very compelled to include this patch. I just don't see enough to make including it worthwhile.

-- 
Shawn.
Marius Storm-Olsen· Feb 23, 2009, 16:03 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Shawn O. Pearce said the following on 23.02.2009 16:42:
Show 13 quoted lines
> Marius Storm-Olsen <marius@trolltech.com> wrote:
>> Anyways, I just thought it would fairly "low cost" to add, and
>> nice to have.
> 
> Its not that low of a cost, its an extra fork+exec per prompt when
> in a .git/ or a bare repository.  Neither is very common when
> compared to a workdir, Junio's right about that.  But its YAFE.  ;)
> 
>> Consider it, as Linus coined the term, a throw-away patch. I can
>> easily put it in my .bashrc instead. :)
> 
> Like Junio, I'm not very compelled to include this patch.  I just 
> don't see enough to make including it worthwhile.

If so, then I'd like to argue to remove setting the fake "GIT_DIR!" branch in the ps, since it hinders me from constructing this, IMO useful prompt, "(BARE:master)" in my own .bashrc.

     ~/source/some_repo (GIT_DIR!)$
simply isn't useful to me, and neither is
     ~/source/some_repo (BARE:GIT_DIR!)$
of course. Now, if we remove setting the fake branch
     ~/source/some_repo (BARE:some/funky/branch)$
is doable for me in my own .bashrc, and by your argument, it would 
also make it more light weight, since you'd remove one extra fork+exec 
for *every single prompt* (and not just one extra when inside GIT_DIR).

^shrug^ at this point you and Junio can discuss what to do, as Junio already said

   | "I do not understand the need for GIT_DIR! thing even
   |  less, but since we have that there already, I do not
   |  see a reason not to add this to the queue."

And I have to agree with him. At this point, __git_ps1() is actually removing useful information from the prompt; at least it does for me.

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'
Shawn O. Pearce· Feb 23, 2009, 16:16 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen <marius@trolltech.com> wrote:
Show 10 quoted lines
>
> ^shrug^ at this point you and Junio can discuss what to do, as Junio  
> already said
>
>   | "I do not understand the need for GIT_DIR! thing even
>   |  less, but since we have that there already, I do not
>   |  see a reason not to add this to the queue."
>
> And I have to agree with him. At this point, __git_ps1() is actually  
> removing useful information from the prompt; at least it does for me.
*sigh*
OK.  I guess we include it then.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
-- 
Shawn.
Marius Storm-Olsen· Feb 23, 2009, 18:55 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Shawn O. Pearce said the following on 23.02.2009 17:16:
Show 12 quoted lines
> Marius Storm-Olsen <marius@trolltech.com> wrote:
>> ^shrug^ at this point you and Junio can discuss what to do, as Junio  
>> already said
>>
>>   | "I do not understand the need for GIT_DIR! thing even
>>   |  less, but since we have that there already, I do not
>>   |  see a reason not to add this to the queue."
>>
>> And I have to agree with him. At this point, __git_ps1() is actually  
>> removing useful information from the prompt; at least it does for me.
> 
> *sigh*

Ops, I realize that it sounded like I was setting you two up against each other, which was not my intention! What I meant to say was, I've stated my case as clear as i can now, so you two can make a decision. I know Junio will listen to you, and I'd be fine if you said no, based on all the info I gave you. (Though I really didn't like the "GIT_DIR!"-branch, but oh well)

> OK.  I guess we include it then.
> 
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Thanks, and again, sorry if you felt I put you up against Junio!

-- .marius

Junio C Hamano· Feb 24, 2009, 01:35 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 17 quoted lines
> Marius Storm-Olsen <marius@trolltech.com> wrote:
>>
>> ^shrug^ at this point you and Junio can discuss what to do, as Junio  
>> already said
>>
>>   | "I do not understand the need for GIT_DIR! thing even
>>   |  less, but since we have that there already, I do not
>>   |  see a reason not to add this to the queue."
>>
>> And I have to agree with him. At this point, __git_ps1() is actually  
>> removing useful information from the prompt; at least it does for me.
>
> *sigh*
>
> OK.  I guess we include it then.
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>

Reverting GIT_DIR! ugliness is certainly a possibility. As long as people who chdir into there are the only ones who suffer from the ugliness I do not particularly care that much, though ;-)

Will apply anyway.
Ted Pavlic· Feb 24, 2009, 14:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Show 5 quoted lines
> Reverting GIT_DIR! ugliness is certainly a possibility.  As long as people
> who chdir into there are the only ones who suffer from the ugliness I do
> not particularly care that much, though ;-)
>
> Will apply anyway.

I only added the "GIT_DIR!" on the advice of Junio, who suggested a "danger" flag. I asked for suggestions on replacement text.

Keep in mind that "BARE:master" doesn't make much sense. If you're in a git dir, you don't have a working directory. "BARE" alone makes sense. Personally, I think it makes more sense to submit a patch that changes "GIT_DIR!" to "BARE" in the bare case (and doesn't print any branch).

--Ted
-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.
Marius Storm-Olsen· Feb 24, 2009, 14:46 UTC · re: Ted Pavlic · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Ted Pavlic said the following on 24.02.2009 15:25:
Show 8 quoted lines
> I only added the "GIT_DIR!" on the advice of Junio, who suggested a
> "danger" flag. I asked for suggestions on replacement text.
> 
> Keep in mind that "BARE:master" doesn't make much sense. If you're
> in a git dir, you don't have a working directory. "BARE" alone
> makes sense. Personally, I think it makes more sense to submit a
> patch that changes "GIT_DIR!" to "BARE" in the bare case (and
> doesn't print any branch).
It reflects what HEAD points to in the bare repository.
-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'
Ted Pavlic· Feb 24, 2009, 15:39 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

>> Keep in mind that "BARE:master" doesn't make much sense. If you're
>
> It reflects what HEAD points to in the bare repository.

Obviously, but that seems disingenuous when you're inside the git dir. "HEAD" is supposed to reflect the name of the currently checked-out branch, and so it is tied to a working directory. I'm not sure why it's useful to show $GIT_DIR/HEAD in PS1 while inside .git as it invites operations that probably should not be done while within the bare repo.

--Ted
-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.
Junio C Hamano· Feb 24, 2009, 17:01 UTC · re: Ted Pavlic · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Ted Pavlic <ted@tedpavlic.com> writes:
Show 10 quoted lines
>>> Keep in mind that "BARE:master" doesn't make much sense. If you're
>>
>> It reflects what HEAD points to in the bare repository.
>
> Obviously, but that seems disingenuous when you're inside the git
> dir. "HEAD" is supposed to reflect the name of the currently
> checked-out branch, and so it is tied to a working directory. I'm not
> sure why it's useful to show $GIT_DIR/HEAD in PS1 while inside .git as
> it invites operations that probably should not be done while within
> the bare repo.

It still indicates the branch in interest. That's the one you get a checkout for when you clone from the repository.

Marius Storm-Olsen· Feb 24, 2009, 19:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Junio C Hamano said the following on 24.02.2009 18:01:
Show 13 quoted lines
> Ted Pavlic <ted@tedpavlic.com> writes:
>>>> Keep in mind that "BARE:master" doesn't make much sense. If
>>>> you're
>>> It reflects what HEAD points to in the bare repository.
>> Obviously, but that seems disingenuous when you're inside the git
>>  dir. "HEAD" is supposed to reflect the name of the currently 
>> checked-out branch, and so it is tied to a working directory. I'm
>> not sure why it's useful to show $GIT_DIR/HEAD in PS1 while
>> inside .git as it invites operations that probably should not be
>> done while within the bare repo.
> 
> It still indicates the branch in interest.  That's the one you get
> a checkout for when you clone from the repository.
Junio, unfortunately you applied the incorrect version.

It was v3 (Message-Id: <1235244057-16912-1-git-send-email-git@storm-olsen.com>) which was the correct one, since it's the one that avoids the "GIT_DIR!" in a bare repo. :-/

-- .marius

Junio C Hamano· Feb 25, 2009, 06:08 UTC · re: Marius Storm-Olsen · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Marius Storm-Olsen <marius@storm-olsen.com> writes:
Show 6 quoted lines
> Junio, unfortunately you applied the incorrect version.
>
> It was v3 (Message-Id:
> <1235244057-16912-1-git-send-email-git@storm-olsen.com>) which was the
> correct one, since it's the one that avoids the "GIT_DIR!" in a bare
> repo. :-/

Sorry, I only was looking at the thread that had Shawn's Ack. Is this interdiff as a fix-up Ok?

-- >8 --
Subject: [PATCH] Fixup: Add bare repository indicator for __git_ps1
Signed-off-by: Marius Storm-Olsen <git@storm-olsen.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)
Show changes to contrib/completion/git-completion.bash +6 −7
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a61d852..dd393cd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -117,9 +117,14 @@ __git_ps1 ()
 
 		local w
 		local i
+		local c
 
 		if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
-			b="GIT_DIR!"
+			if [ "true" = "$(git config --bool core.bare 2>/dev/null)" ]; then
+				c="BARE:"
+			else
+				b="GIT_DIR!"
+			fi
 		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
 			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
 				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
@@ -135,12 +140,6 @@ __git_ps1 ()
 			fi
 		fi
 
-		local c
-
-		if [ "true" = "$(git config --bool core.bare 2>/dev/null)" ]; then
-			c="BARE:"
-		fi
-
 		if [ -n "$b" ]; then
 			if [ -n "${1-}" ]; then
 				printf "$1" "$c${b##refs/heads/}$w$i$r"

-- 
1.6.2.rc1.113.ga620b
Marius Storm-Olsen· Feb 25, 2009, 06:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add bare repository indicator for __git_ps1

Junio C Hamano said the following on 25.02.2009 07:08:
Show 10 quoted lines
> Marius Storm-Olsen <marius@storm-olsen.com> writes:
>> Junio, unfortunately you applied the incorrect version.
>>
>> It was v3 (Message-Id:
>> <1235244057-16912-1-git-send-email-git@storm-olsen.com>) which was the
>> correct one, since it's the one that avoids the "GIT_DIR!" in a bare
>> repo. :-/
> 
> Sorry, I only was looking at the thread that had Shawn's Ack.
> Is this interdiff as a fix-up Ok?
Yup, looks sane to me. Thanks.

-- .marius

← back to recent threads