Re: [PATCH 1/3] t9350: point out that refs are not updated correctly

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Oct 25, 2012, 05:50 UTC
Message-ID: <CAMP44s3kBxzJbyoxPqWbRMWmpX9sNPGjdRy_KrTeRoVmGC-+Hg@mail.gmail.com>
In-Reply-To: <CAGdFq_gg3gPvCADje9ibz8xHgPOLF+=79EqksVzG2JeTOfHocw@mail.gmail.com>

On Thu, Oct 25, 2012 at 7:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:

Show 20 quoted lines

> On Wed, Oct 24, 2012 at 10:28 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> The testcase is imho correct and does not need changing.  So yes, I
>> don't want your help changing it.  I don't suspect you will be using
>> "git fast-export $(git rev-parse master)..master".  It is safe and
>> good to add additional testcases documenting the syntax that you do
>> use, as an independent topic.
>
> To re-iterate Dscho's point, the reason for this testcase is that if
> you do this:
> $ git checkout master
> $ git branch next
> $ git push hg://example.com master
> $ git push hg://example.com next
>
> With the current design, next will not be present on the remote. This
> is caused by the fact that git looks at "fast-export ^master next",
> sees that it's empty, and decides not to export anything. This patch
> series solves that, by having "fast-export ^master next" emit a "from
> :42\nreset next" (or something like that, assuming :42 is where master
> is currently at).

Only if the remote helper is using marks, and this particular patch is adding a test-case without any use of marks at all.

IOW; this test is testing something completely different, which happens to fix the original issue, but this is not the only way to fix, and in IMO certainly not the best.

As I showed in my script above:

$ git checkout master $ git branch next $ git push hg://example.com master $ git push hg://example.com next

This works just fine. Go ahead, apply my patch, and run it, the second branch gets updated.

It will fail this test, but that's because the test is not testing what it should: that *when using marks* the second branch exported is ignored.

This test does that:

--- cat > expected << EOF reset refs/heads/master from ##mark##

EOF

test_expect_failure 'refs are updated even if no commits need to be exported' '
        cp tmp-marks /tmp
        git fast-export --import-marks=tmp-marks \
                --export-marks=tmp-marks master | true &&
        git fast-export --import-marks=tmp-marks \
                --export-marks=tmp-marks master > actual &&
        mark=$(grep $(git rev-parse master) tmp-marks | cut -f 1 -d " ")
        sed -i -e "s/##mark##/$mark/" expected &&
        test_cmp expected actual
'
---

-- 
Felipe Contreras