On Sun, Mar 20, 2016 at 8:11 PM, Jose Ivan B. Vilarouca Filho
<joseivan@xxxxxxxxxxxxx> wrote:
> Hello, Eric.
>
> Thanks for suggestions. I've added a test in commit replacing git fetch origin by a fake FETCH_HEAD content.
Thanks for the re-roll. To be "git am"-friendly, you should either
place a "-- >8 --" scissor line after the above commentary, or use
"git send-email" to send the patch, and place the commentary just
below the "---" line following your sign-off.
More below...
> merge: don't dereference NULL pointer
>
> A segmentaion fault is raised when trying to merge FETCH_HEAD
> formed only by "not-for-merge" refs.
>
> Ex:
> git init .
> git remote add origin ...
> git fetch origin
> git merge FETCH_HEAD
>
> Signed-off-by: Jose Ivan B. Vilarouca Filho <joseivan@xxxxxxxxxxxxx>
> ---
> diff --git a/test-merge-fetchhead.sh b/test-merge-fetchhead.sh
As you're new around here, I probably should have been more specific
in my previous review and explained that you'd want your new test to
be incorporated into the existing test suite so that it actually gets
exercised regularly. A good place for the new test might be at the
bottom of t/t7600-merge.sh.
Writing a new test is pretty simple, especially when you already have
a recipe for reproduction. Take a look at existing tests in that file
to get a feel for how it should be done. Rather than all the "|| exit
1"'s, chain your test statements together with &&. And, because you'll
be incorporating the new test into an existing script, you can drop a
lot of the boilerplate below and just focus on the recipe.
Some style comments below (most of which won't matter after you drop
the boilerplate but are generally good to know)...
> @@ -0,0 +1,23 @@
> +#!/bin/bash
> +GIT=$(pwd)/git
> +REPO_DIR=./test-fetch-head-repo
> +
> +if [ ! -x "${GIT}" ]; then
Use 'test' rather than '['. Drop the semicolon. Place 'then' on its own line.
> + echo "Missing git binary"
> + exit 1
> +fi
> +
> +${GIT} init ${REPO_DIR} || rm -rf ${REPO_DIR} || exit 1
> +pushd ${REPO_DIR} || rm -rf ${REPO_DIR} || exit 1
In test scripts you need to take extra care when switching directories
since failure of a command following 'pushd' will prevent the
subsequent 'popd' from executing, and then tests later in the script
will be invoked in the wrong directory. The typical way of dealing
with this is to use a subhsell since 'cd' within a subshell doesn't
affect the parent, and the parent continues at its own working
directory when the subshell exits. For example:
some-command &&
(
cd somewhere &&
command-which-might-fail &&
another-possible-failure
)
> +#Let's fake a FETCH_HEAD only formed by not-for-merge (git fetch origin)
> +echo -ne "f954fc9919c9ec33179e11aa1af562384677e174\tnot-for-merge\tbranch 'master' of https://github.com/git/git.git"; > .git/FETCH_HEAD
Non-portable 'echo' options. Use 'printf' instead. And, you'll need to
take extra care with the single-quotes since the test body itself will
be within single-quotes.
> +${GIT} merge FETCH_HEAD
> +GRET=$?
> +popd
> +if [ ${GRET} -eq 139 ]; then
So, both before and after this patch, this git-merge fails, and you
want the test to detect that it's failing in a controlled way (via
die()) rather than the previous uncontrolled way (via segfault). You
could use test_expect_code() for this (from t/test-lib-functions.sh),
or you could capture stderr and verify that it has the expected
failure message from die(). The latter is probably preferable, and may
look something like this:
test_expect_success 'my test title' '
...setup stuff... &&
test_must_fail git merge FETCH_HEAD 2>err &&
test_i18ngrep "not something we can merge" err
'
> + rm -rf ${REPO_DIR}
> + exit 1
> +fi
> +
> +rm -rf ${REPO_DIR}
> +exit 0
You typically don't need to cleanup after your test, so this
boilerplate can go away.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html