Subject: Re: [PATCH 1/8] git-rebase.sh: Fix --merge --abort failures when path contains whitespace

From: Johannes Sixt <[email-masked]>
Date: Wed, 09 Apr 2008 08:55:31 +0200

Bryan Donlan schrieb:
> -dotest=$GIT_DIR/.dotest-merge
> +dotest="$GIT_DIR/.dotest-merge"

This is not strictly necessary: The RHS expression of an assignment does
not undergo IFS splitting; but better safe than sorry. (But note that
'export foo=$bar', which is not POSIX, is *not* an assignment, and
different shells that support this construct treat it differently.)

> +### Test that we handle strange characters properly
> +work_dir="$(pwd)/test \" ' \$ \\ dir"

In effect, you modify only this test to stress-test strange characters,
but other tests in the test suite still run in a "sane" environment. IOW,
I don't think you should go to this extreme for this one test only. The
better approach would be to rename 'trash' in test-lib.sh to this strange
name so that all tests suffer from a challenging environment.

> -		git reset --hard pre-rebase
> -		test_must_fail git rebase'"$type"' master &&
> -		test -d '$dotest' &&
> +		git reset --hard pre-rebase &&
> +		test_must_fail git rebase$type master &&
> +		test -d \"\$dotest\" &&

I could imagine that the missing && after the git reset is deliberate. Mike?

> +		git reset --hard pre-rebase &&

Ditto.

Apart from that, this looks good.

-- Hannes