From: Jeff King <peff@peff.net>
To: Petter Urkedal <urkedal@nbi.dk>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Reserve a slot for argv[0] in default_arg.
Date: Sun, 4 Oct 2009 14:27:46 -0400 [thread overview]
Message-ID: <20091004182746.GA22995@coredump.intra.peff.net> (raw)
In-Reply-To: <20091004141355.GA15783@eideticdew.org>
On Sun, Oct 04, 2009 at 04:13:55PM +0200, Petter Urkedal wrote:
> I was wondering myself. I tried to switch off optimisation, but that
> had no effect. I'm suspecting PIE, but it could be some other
> configuration implied by the Gentoo "hardened" use-flag.
Nope, it's just a plain old git bug...
> I can reproduce it on my machine with
>
> mkdir test-repo; cd test-repo
> /path/to/git init
> /path/to/git config showbranch.default --topo-order
> /path/to/git show-branch
Ah, thanks, for some reason I wasn't able to produce it before, but I
can easily replicate it here. I think it's a regression from converting
show-branch to use parse_options, which happened in May, but I didn't
actually bisect it. I'm not sure showbranch.default has worked at all
since then (which I guess goes to show how many people are actually
using it).
So your fix is definitely right, and the test case below (which can be
squashed in) fails reliably without it.
t3202 is maybe a bit of weird place to put it, but we don't seem to test
show-branch anywhere else. It could probably use a "check that
show-branch works at all" set of tests, but I am not volunteering to
write such a thing. I have always found its output to be one step above
line noise.
I also looked at putting it in t1200-tutorial.sh near the show-branch
call, but that script is an utter mess. Most of the tests don't actually
check the exit status of commands, and there is a random "test_done"
halfway through the script which skips all of the later tests (including
the show-branch test!). Removing that to enable the later tests reveals
that they are broken, with such obviously non-working crap as
git merge -s "Merge upstream changes." master
which is clearly bogus. I wonder if we should just remove that script
altogether; at best it just seems redundant with other tests, and it is
full of obvious errors.
> Comment's are treated as whitespace, but I'll adjust it for readability.
> Maybe worse: I missed the 8-column indentation. So, here is the patch
> again (attached, I hope Git can extract it).
Thanks, that looks better (I actually didn't even notice the indent
problem the first time, but yes, it should be 8 columns).
Squashable test case is below.
-Peff
---
diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
index 7fe4a6e..0a5d5e6 100755
--- a/t/t3202-show-branch-octopus.sh
+++ b/t/t3202-show-branch-octopus.sh
@@ -56,4 +56,12 @@ test_expect_success 'show-branch with more than 8 branches' '
'
+test_expect_success 'show-branch with showbranch.default' '
+ for i in $numbers; do
+ git config --add showbranch.default branch$i
+ done &&
+ git show-branch >out &&
+ test_cmp expect out
+'
+
test_done
next prev parent reply other threads:[~2009-10-04 18:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-03 13:29 [PATCH] Reserve a slot for argv[0] in default_arg Petter Urkedal
2009-10-04 13:33 ` Jeff King
2009-10-04 14:13 ` Petter Urkedal
2009-10-04 18:27 ` Jeff King [this message]
2009-10-04 20:02 ` Stephen Boyd
2009-10-04 22:20 ` Junio C Hamano
2009-10-05 6:36 ` Petter Urkedal
2009-10-05 18:45 ` Jeff King
2009-10-05 6:44 ` Petter Urkedal
2009-10-04 14:51 ` Petter Urkedal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091004182746.GA22995@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=urkedal@nbi.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).