| Submitter | Alexander Fomin |
|---|---|
| Date | March 21, 2017, 5:08 p.m. |
| Message ID | <9a11a79f6bcdd1134484.1490116120@devvm2125.lla2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/19534/ |
| State | Changes Requested |
| Headers | show |
Comments
Overall this series looks good to me, except for this last patch. See inline comments. For now, I'd take the rest of this series if we're okay with the BC break, and just drop this patch while we figure out the mq stuff. On 3/21/17 5:08 PM, Alexander Fomin wrote: > # HG changeset patch > # User Alexander Fomin<afomin@fb.com> > # Date 1490113938 25200 > # Tue Mar 21 09:32:18 2017 -0700 > # Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a > # Parent e9044ade1523e847877f4eee1d4e06734e2aa4cd > tests: add a test case verifying that mq respects --no-git option > > This patch adds a test case to verify that --no-git option still works in mq > after making it explicitly request binary diff even in Git mode (issue5510). > > diff --git a/tests/test-mq.t b/tests/test-mq.t > --- a/tests/test-mq.t > +++ b/tests/test-mq.t > @@ -1162,6 +1162,21 @@ check binary patches can be popped and p > 8ba2a2f3e77b55d03051ff9c24ad65e7 bucephalus > > > +check binary patches respect --no-git > + > + $ cat > writebin.py <<EOF > + > import sys > + > path = sys.argv[1] > + > open(path, 'wb').write('BIN\x42RY') Hex 42 is the character 'B', isn't it? So this isn't binary at all. Also, binary detection just searches for \x00 I think. So that would be more appropriate to use here. > + > EOF > + $ python writebin.py answer Rather than creating a little python program, I think you could just use printf here: $ printf 'BIN\x00RY' > answer > + > + $ python "$TESTDIR/md5sum.py" answer > + ce0b4fda508e3d9f9ece98f8e823b6f7 answer What is the reason for the md5sum here? Did you want to check round-tripping? (but I don't see that here) > + $ hg add answer > + $ hg qnew -f --no-git addanswer What does --no-git do before this patch series? I don't see any differences in patch files with or without --no-git today, so I'm not sure it's actually respected today. > + $ grep diff .hg/patches/addanswer > + diff -r [a-f0-9]* -r [a-f0-9]* answer (re) > > strip again > > > >
FYI, there's a discussion going on about the "correct" behavior on the issue tracker: https://bz.mercurial-scm.org/show_bug.cgi?id=5510 On 3/21/17 8:30 PM, Ryan McElroy wrote: > Overall this series looks good to me, except for this last patch. See > inline comments. For now, I'd take the rest of this series if we're > okay with the BC break, and just drop this patch while we figure out > the mq stuff. > > On 3/21/17 5:08 PM, Alexander Fomin wrote: >> # HG changeset patch >> # User Alexander Fomin<afomin@fb.com> >> # Date 1490113938 25200 >> # Tue Mar 21 09:32:18 2017 -0700 >> # Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a >> # Parent e9044ade1523e847877f4eee1d4e06734e2aa4cd >> tests: add a test case verifying that mq respects --no-git option >> >> This patch adds a test case to verify that --no-git option still works in mq >> after making it explicitly request binary diff even in Git mode (issue5510). >> >> diff --git a/tests/test-mq.t b/tests/test-mq.t >> --- a/tests/test-mq.t >> +++ b/tests/test-mq.t >> @@ -1162,6 +1162,21 @@ check binary patches can be popped and p >> 8ba2a2f3e77b55d03051ff9c24ad65e7 bucephalus >> >> >> +check binary patches respect --no-git >> + >> + $ cat > writebin.py <<EOF >> + > import sys >> + > path = sys.argv[1] >> + > open(path, 'wb').write('BIN\x42RY') > > Hex 42 is the character 'B', isn't it? So this isn't binary at all. > > Also, binary detection just searches for \x00 I think. So that would > be more appropriate to use here. > >> + > EOF >> + $ python writebin.py answer > > Rather than creating a little python program, I think you could just > use printf here: > > $ printf 'BIN\x00RY' > answer > >> + >> + $ python "$TESTDIR/md5sum.py" answer >> + ce0b4fda508e3d9f9ece98f8e823b6f7 answer > > What is the reason for the md5sum here? Did you want to check > round-tripping? (but I don't see that here) > >> + $ hg add answer >> + $ hg qnew -f --no-git addanswer > > What does --no-git do before this patch series? I don't see any > differences in patch files with or without --no-git today, so I'm not > sure it's actually respected today. > >> + $ grep diff .hg/patches/addanswer >> + diff -r [a-f0-9]* -r [a-f0-9]* answer (re) >> >> strip again >> >> >> >> > > > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=2gvSPeBA6Q2eteGH5pR0M0qoiyQsjRbkUa0BkXzQ2C4&s=eYuPnseRo71oYyYXfcJgzwAbECl69gbMV2QU6VUmAVw&e=
Patch
diff --git a/tests/test-mq.t b/tests/test-mq.t --- a/tests/test-mq.t +++ b/tests/test-mq.t @@ -1162,6 +1162,21 @@ check binary patches can be popped and p 8ba2a2f3e77b55d03051ff9c24ad65e7 bucephalus +check binary patches respect --no-git + + $ cat > writebin.py <<EOF + > import sys + > path = sys.argv[1] + > open(path, 'wb').write('BIN\x42RY') + > EOF + $ python writebin.py answer + + $ python "$TESTDIR/md5sum.py" answer + ce0b4fda508e3d9f9ece98f8e823b6f7 answer + $ hg add answer + $ hg qnew -f --no-git addanswer + $ grep diff .hg/patches/addanswer + diff -r [a-f0-9]* -r [a-f0-9]* answer (re) strip again