Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

9 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:22

Brian Campbell [off-list ref] writes:
+current_branch()
+{
+	echo "$(git symbolic-ref HEAD | sed -e 's#^refs/heads/##' -e 's#^refs/top-bases/##')"
+}
Two micronits.

 - what happens when you are on a detached HEAD?

 - You will be utterly confused by a local branch whose name is
   "refs/top-bases/foo"

To fix these, you might want to do something like:

	if head_=$(git symbolic-ref HEAD)
        then
                case "$head_" in
                refs/heads/*)
                        echo "${head_#refs/heads/}"
                        ;;
                refs/top-bases/*)
                        echo "${head_#refs/top-bases/}"
                        ;;
                *)
                        echo "$head_"
                        ;;
                esac
	else
        	whatever you want to do on a detached HEAD
	fi

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Uwe Kleine-König <hidden>
Date: 2016-06-15 22:46:22

Hello Brian, hello Junio,

On Wed, Mar 11, 2009 at 11:55:49PM -0700, Junio C Hamano wrote:
Brian Campbell [off-list ref] writes:
quoted
+current_branch()
+{
+	echo "$(git symbolic-ref HEAD | sed -e 's#^refs/heads/##' -e 's#^refs/top-bases/##')"
+}
Two micronits.

 - what happens when you are on a detached HEAD?
The original code had this problem, too, so I would not take this as a
stopper for the patch.  There are some other locations that suffer from
the same problem.  That's already on my todo list.  So I don't care much
here.
 
 - You will be utterly confused by a local branch whose name is
   "refs/top-bases/foo"
You mean a branch that has the full name refs/heads/refs/top-bases/foo?
Well OK, valid concern.
To fix these, you might want to do something like:

	if head_=$(git symbolic-ref HEAD)
        then
                case "$head_" in
                refs/heads/*)
                        echo "${head_#refs/heads/}"
                        ;;
                refs/top-bases/*)
                        echo "${head_#refs/top-bases/}"
                        ;;
                *)
                        echo "$head_"
                        ;;
                esac
	else
        	whatever you want to do on a detached HEAD
	fi
Thanks Junio and Brian.

Brian, do you update the series?

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Brian Campbell <hidden>
Date: 2016-06-15 22:46:23

On Mar 12, 2009, at 3:45 AM, Uwe Kleine-König wrote:
quoted
- You will be utterly confused by a local branch whose name is
  "refs/top-bases/foo"
You mean a branch that has the full name refs/heads/refs/top-bases/ 
foo?
Well OK, valid concern.
Yes, you're right, this is a problem.
quoted
To fix these, you might want to do something like:

	if head_=$(git symbolic-ref HEAD)
       then
               case "$head_" in
               refs/heads/*)
                       echo "${head_#refs/heads/}"
                       ;;
               refs/top-bases/*)
                       echo "${head_#refs/top-bases/}"
                       ;;
               *)
                       echo "$head_"
                       ;;
               esac
	else
       	whatever you want to do on a detached HEAD
	fi
Thanks Junio and Brian.

Brian, do you update the series?
Sure, I'll send an updated patch.

I'm thinking that for the detached HEAD case, this function should die  
with a message about not being on a valid branch, and then the call  
site in tg-summary that doesn't care about being on a valid branch  
should ignore the error and leave curname empty. Does that sound about  
right? I'm fairly new to doing Bourne shell scripting, so I don't yet  
have a good sense of how these things should be structured.

Also, has anyone considered writing a test suite for TopGit? I  
actually got fairly deep in to a series of 10 or so patches before I  
hit these problems, since tg-create worked fine as long as I only  
supplied one dependency, and I didn't notice the second issue until I  
tried to do a tg-update after modifying one of my base patches.

-- Brian

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Uwe Kleine-König <hidden>
Date: 2016-06-15 22:46:23

Hello Brian, hello Junio,

On Thu, Mar 12, 2009 at 11:00:00AM -0400, Brian Campbell wrote:
On Mar 12, 2009, at 3:45 AM, Uwe Kleine-König wrote:
quoted
quoted
- You will be utterly confused by a local branch whose name is
  "refs/top-bases/foo"
You mean a branch that has the full name refs/heads/refs/top-bases/ 
foo?
Well OK, valid concern.
Yes, you're right, this is a problem.
quoted
quoted
To fix these, you might want to do something like:

	if head_=$(git symbolic-ref HEAD)
Shouldn't git symbolic-ref -q HEAD be used here?
quoted
quoted
       then
               case "$head_" in
               refs/heads/*)
                       echo "${head_#refs/heads/}"
                       ;;
               refs/top-bases/*)
                       echo "${head_#refs/top-bases/}"
                       ;;
               *)
                       echo "$head_"
                       ;;
               esac
	else
       	whatever you want to do on a detached HEAD
How do I  distinguish between a detached HEAD and another error?  I have
the feeling that git symbolic-ref -q HEAD should exit(0) with a detached
HEAD.
quoted
Thanks Junio and Brian.

Brian, do you update the series?
Sure, I'll send an updated patch.

I'm thinking that for the detached HEAD case, this function should die  
with a message about not being on a valid branch, and then the call site 
in tg-summary that doesn't care about being on a valid branch should 
ignore the error and leave curname empty. Does that sound about right? 
mmh, I would return "" and let the caller handle that.
Also, has anyone considered writing a test suite for TopGit?
Yes, but I didn't found the time for that until now.  If you'd volunteer
that would be very welcome.

IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
Peiner Strasse 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686              | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: martin f krafft <hidden>
Date: 2016-06-15 22:46:23

also sprach Uwe Kleine-König [off-list ref] [2009.03.12.1620 +0100]:
IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.
Maybe we could even start to think about integrating TopGit back
into git.git?

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"perhaps debian is concerned more about technical excellence rather
 than ease of use by breaking software. in the former we may excel.
 in the latter we have to concede the field to microsoft. guess
 where i want to go today?"
                                                 -- manoj srivastava
 
spamtraps: madduck.bogus@madduck.net

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Brian Campbell <hidden>
Date: 2016-06-15 22:46:23

On Mar 12, 2009, at 11:20 AM, Uwe Kleine-König wrote:
quoted
quoted
quoted
To fix these, you might want to do something like:

	if head_=$(git symbolic-ref HEAD)
Shouldn't git symbolic-ref -q HEAD be used here?
Yes, most likely.
quoted
quoted
quoted
      then
              case "$head_" in
              refs/heads/*)
                      echo "${head_#refs/heads/}"
                      ;;
              refs/top-bases/*)
                      echo "${head_#refs/top-bases/}"
                      ;;
              *)
                      echo "$head_"
                      ;;
              esac
	else
      	whatever you want to do on a detached HEAD
How do I  distinguish between a detached HEAD and another error?  I  
have
the feeling that git symbolic-ref -q HEAD should exit(0) with a  
detached
HEAD.
If you pass -q, it exits with status 1 on a detached head, 128 on  
other errors, so we can use that to distinguish.
quoted
quoted
Thanks Junio and Brian.

Brian, do you update the series?
Sure, I'll send an updated patch.

I'm thinking that for the detached HEAD case, this function should  
die
with a message about not being on a valid branch, and then the call  
site
in tg-summary that doesn't care about being on a valid branch should
ignore the error and leave curname empty. Does that sound about  
right?
mmh, I would return "" and let the caller handle that.
Fair enough.
quoted
Also, has anyone considered writing a test suite for TopGit?
Yes, but I didn't found the time for that until now.  If you'd  
volunteer
that would be very welcome.
I would, but I'm not sure I'll be continuing to use TopGit for more  
than the one patch series I'm using it for now; I was trying it out,  
but it feels a little more heavy-weight than what I want. StGIT or  
just rewriting a patch series with git rebase -i works better for my  
uses; I'm not maintaining a lot of long-lived topic branches upon  
which I need full history.
IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.
Hmm. How would the tests find your git working tree? I'd be willing to  
start the process off at least by writing test cases for the  
functionality I'm changing here if I had a good idea of how to start.  
Would it be sufficient to make something like "GIT_CHECKOUT=~/src/git  
make check" work?

-- Brian

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Uwe Kleine-König <hidden>
Date: 2016-06-15 22:46:23

Hello Brian,

On Thu, Mar 12, 2009 at 12:41:45PM -0400, Brian Campbell wrote:
quoted
IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.
Hmm. How would the tests find your git working tree? I'd be willing to  
start the process off at least by writing test cases for the  
functionality I'm changing here if I had a good idea of how to start.  
Would it be sufficient to make something like "GIT_CHECKOUT=~/src/git  
make check" work?
Yes, this would be a good start.  I would call it GIT_SRC, but that's up
to you.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:23

martin f krafft [off-list ref] writes:
also sprach Uwe Kleine-König [off-list ref] [2009.03.12.1620 +0100]:
quoted
IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.
Maybe we could even start to think about integrating TopGit back
into git.git?
Heh, it would need massive style fixes before that happens. I am fairly
picky on shell script styles.

Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions

From: Uwe Kleine-König <hidden>
Date: 2016-06-15 22:46:23

Hello,

On Fri, Mar 13, 2009 at 10:54:41PM -0700, Junio C Hamano wrote:
martin f krafft [off-list ref] writes:
quoted
also sprach Uwe Kleine-König [off-list ref] [2009.03.12.1620 +0100]:
quoted
IMHO we should reuse as much as possible from git.git.  For me even
requiring a git.git checkout to use its files would be OK.  I consider
that even better then duplicating the relevant files.
Maybe we could even start to think about integrating TopGit back
into git.git?
Heh, it would need massive style fixes before that happens. I am fairly
picky on shell script styles.
me, too.  That's one of my todo list items, independently from martin's
suggestion.  (More to the bottom of that list, though.)

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help