Re: [PATCH v3 1/4] t3431: add rebase --fork-point tests
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 5, 2019, 18:51 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1904052047240.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <34af9bf5-4031-63ed-c34b-9f2c7ff76c51@kdbg.org>
Hi Hannes & Denton,
On Fri, 5 Apr 2019, Johannes Sixt wrote:
Show 14 quoted lines
> Am 05.04.19 um 19:25 schrieb Denton Liu:
> > On Fri, Apr 05, 2019 at 04:55:37PM +0200, Johannes Schindelin wrote:
> >> On Mon, 1 Apr 2019, Denton Liu wrote:
> >>> +test_rebase() {
> >>> + expected="$1" &&
> >>> + shift &&
> >>> + test_expect_success "git rebase $@" "
> >>> + git checkout master &&
> >>> + git reset --hard E &&
> >>> + git checkout side &&
> >>> + git reset --hard G &&
> >>> + git rebase $@ &&
> >>
> >> I think we need this patch, to make the macOS build happy:Actually, my patch did not even fix the build, I looked at the wrong (succeeding) build, sorry for the noise.
Show 15 quoted lines
> > Thanks for digging into this, both.
> >
> > Out of curiosity, t3432 is written similarly:
> >
> > test_rebase_same_head() {
> > status="$1" &&
> > shift &&
> > test_expect_$status "git rebase $@ with $changes is no-op" "
> > oldhead=\$(git rev-parse HEAD) &&
> > test_when_finished 'git reset --hard \$oldhead' &&
> > git rebase $@ &&
> > newhead=\$(git rev-parse HEAD) &&
> > test_cmp_rev \$oldhead \$newhead
> > "
> > }That is curious, indeed!
Show 9 quoted lines
> Using $@ in these expansions is wrong. You do not want to forward an > argument list, but you want to construct a command line. $* is correct > here. Then you can remove the single-quotes at the invocation, like so: > > test_rebase_same_head success > test_rebase_same_head success --onto B B > > Function test_rebase() could be done in the same way, but the first > argument, expected, still needs quotes at the call site, of course.
That's a good idea, let me run with it.
Ciao, Dscho