| Submitter | Matt Harbison |
|---|---|
| Date | Dec. 10, 2017, 8:47 p.m. |
| Message ID | <ebc7a05d9389989fc2e8.1512938865@Envy> |
| Download | mbox | patch |
| Permalink | /patch/26204/ |
| State | Accepted |
| Headers | show |
Comments
> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote: > > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1512882971 18000 > # Sun Dec 10 00:16:11 2017 -0500 > # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 > # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 > run-tests: accept '\' vs '/' path differences without '(glob)’ series queued, many thanks […] > It's probably unreasonable to submit a single patch that wipes out all of the > (glob) instances that were only used to hide path differences, given the churn > from other contributors. Since their presence isn't harming the tests, these > can be removed through attrition. You could do it if you wanted with a skip-blame annotation as was done in d92dc725223b. Then we can pass the right filter to `hg annotate` and not see the noop changes. Not a requirement though, don’t feel obligated to do so.
On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote: > >> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> >> wrote: >> >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1512882971 18000 >> # Sun Dec 10 00:16:11 2017 -0500 >> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >> run-tests: accept '\' vs '/' path differences without '(glob)’ > > series queued, many thanks > > […] > >> It's probably unreasonable to submit a single patch that wipes out all >> of the >> (glob) instances that were only used to hide path differences, given >> the churn >> from other contributors. Since their presence isn't harming the tests, >> these >> can be removed through attrition. > > You could do it if you wanted with a skip-blame annotation as was done > in d92dc725223b. Then we can pass the right filter to `hg annotate` and > not see the noop changes. Not a requirement though, don’t feel obligated > to do so. I hadn't thought of blame- I was more worried about import conflicts. Does skip-blame somehow help the import tools? Probably a rough approximation: $ grep '(glob)' *.t | grep -v '[*?]' | wc -l 1125 Without the negative filter, it's 3216. (If I knew the equivalent regex, it should be trivial to pipe it through sed, and see what blows up.)
> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> wrote: > > On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote: > >> >>> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>> >>> # HG changeset patch >>> # User Matt Harbison <matt_harbison@yahoo.com> >>> # Date 1512882971 18000 >>> # Sun Dec 10 00:16:11 2017 -0500 >>> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >>> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >>> run-tests: accept '\' vs '/' path differences without '(glob)’ >> >> series queued, many thanks >> >> […] >> >>> It's probably unreasonable to submit a single patch that wipes out all of the >>> (glob) instances that were only used to hide path differences, given the churn >>> from other contributors. Since their presence isn't harming the tests, these >>> can be removed through attrition. >> >> You could do it if you wanted with a skip-blame annotation as was done in d92dc725223b. Then we can pass the right filter to `hg annotate` and not see the noop changes. Not a requirement though, don’t feel obligated to do so. > > I hadn't thought of blame- I was more worried about import conflicts. Does skip-blame somehow help the import tools? Sadly no, but I’ve often worked around issues like that by importing at the parent revision listed in the patch header and then using rebase so I get a proper merge tool. And we’d likely only get patches with such conflicts for a week or two... > Probably a rough approximation: > > $ grep '(glob)' *.t | grep -v '[*?]' | wc -l > 1125 > > Without the negative filter, it's 3216. > > (If I knew the equivalent regex, it should be trivial to pipe it through sed, and see what blows up.) I’d probably write a tool in Python to do this replacement, because I’m lazy and it’d be less work for my brain.
On Sun, 10 Dec 2017 18:28:03 -0500, Augie Fackler <raf@durin42.com> wrote: > >> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> >> wrote: >> >> On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> >> wrote: >> >>> >>>> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> >>>> wrote: >>>> >>>> # HG changeset patch >>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>> # Date 1512882971 18000 >>>> # Sun Dec 10 00:16:11 2017 -0500 >>>> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >>>> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >>>> run-tests: accept '\' vs '/' path differences without '(glob)’ >>> >>> series queued, many thanks >>> >>> […] >>> >>>> It's probably unreasonable to submit a single patch that wipes out >>>> all of the >>>> (glob) instances that were only used to hide path differences, given >>>> the churn >>>> from other contributors. Since their presence isn't harming the >>>> tests, these >>>> can be removed through attrition. >>> >>> You could do it if you wanted with a skip-blame annotation as was done >>> in d92dc725223b. Then we can pass the right filter to `hg annotate` >>> and not see the noop changes. Not a requirement though, don’t feel >>> obligated to do so. OK, done. But 3 of 3 isn't showing up in the mail archive, and at ~290K for that patch, I would have expected a digest email too. So something choked. I'll try to check tomorrow around 8:30 EST and resend if needed, before too much more activity occurs. Random thought: should these # skip-blame markers also imply a default skip in bisect? It's probably too late to change the name now...
> On Dec 11, 2017, at 00:24, Matt Harbison <mharbison72@gmail.com> wrote: > > On Sun, 10 Dec 2017 18:28:03 -0500, Augie Fackler <raf@durin42.com> wrote: > >> >>> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>> >>> On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote: >>> >>>> >>>>> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>>>> >>>>> # HG changeset patch >>>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>>> # Date 1512882971 18000 >>>>> # Sun Dec 10 00:16:11 2017 -0500 >>>>> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >>>>> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >>>>> run-tests: accept '\' vs '/' path differences without '(glob)’ >>>> >>>> series queued, many thanks >>>> >>>> […] >>>> >>>>> It's probably unreasonable to submit a single patch that wipes out all of the >>>>> (glob) instances that were only used to hide path differences, given the churn >>>>> from other contributors. Since their presence isn't harming the tests, these >>>>> can be removed through attrition. >>>> >>>> You could do it if you wanted with a skip-blame annotation as was done in d92dc725223b. Then we can pass the right filter to `hg annotate` and not see the noop changes. Not a requirement though, don’t feel obligated to do so. > > OK, done. But 3 of 3 isn't showing up in the mail archive, and at ~290K for that patch, I would have expected a digest email too. So something choked. I'll try to check tomorrow around 8:30 EST and resend if needed, before too much more activity occurs. It probably got stuck in moderation. I'll mod it through. > > Random thought: should these # skip-blame markers also imply a default skip in bisect? It's probably too late to change the name now... probably not? It's not a default skip in blame, you have to specify the flag anyway...
Is it this patch that broke test-run-tests.t? See https://buildbot.mercurial-scm.org/builders/hg%20tests/builds/1357/steps/run-tests.py%20%28python%202.7.10%29/logs/stdio. I haven't even tried to verify, but Matt and I were both on the blame list for the broken build and it didn't seem related to anything I did. On Mon, Dec 11, 2017 at 5:48 AM, Augie Fackler <raf@durin42.com> wrote: > >> On Dec 11, 2017, at 00:24, Matt Harbison <mharbison72@gmail.com> wrote: >> >> On Sun, 10 Dec 2017 18:28:03 -0500, Augie Fackler <raf@durin42.com> wrote: >> >>> >>>> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>>> >>>> On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote: >>>> >>>>> >>>>>> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>>>>> >>>>>> # HG changeset patch >>>>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>>>> # Date 1512882971 18000 >>>>>> # Sun Dec 10 00:16:11 2017 -0500 >>>>>> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >>>>>> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >>>>>> run-tests: accept '\' vs '/' path differences without '(glob)’ >>>>> >>>>> series queued, many thanks >>>>> >>>>> […] >>>>> >>>>>> It's probably unreasonable to submit a single patch that wipes out all of the >>>>>> (glob) instances that were only used to hide path differences, given the churn >>>>>> from other contributors. Since their presence isn't harming the tests, these >>>>>> can be removed through attrition. >>>>> >>>>> You could do it if you wanted with a skip-blame annotation as was done in d92dc725223b. Then we can pass the right filter to `hg annotate` and not see the noop changes. Not a requirement though, don’t feel obligated to do so. >> >> OK, done. But 3 of 3 isn't showing up in the mail archive, and at ~290K for that patch, I would have expected a digest email too. So something choked. I'll try to check tomorrow around 8:30 EST and resend if needed, before too much more activity occurs. > > It probably got stuck in moderation. I'll mod it through. > >> >> Random thought: should these # skip-blame markers also imply a default skip in bisect? It's probably too late to change the name now... > > probably not? It's not a default skip in blame, you have to specify the flag anyway... > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> On Dec 13, 2017, at 12:40 AM, Martin von Zweigbergk <martinvonz@google.com> wrote: > > Is it this patch that broke test-run-tests.t? See > https://buildbot.mercurial-scm.org/builders/hg%20tests/builds/1357/steps/run-tests.py%20%28python%202.7.10%29/logs/stdio. > I haven't even tried to verify, but Matt and I were both on the blame > list for the broken build and it didn't seem related to anything I > did. My guess is that it’s this one: https://www.mercurial-scm.org/repo/hg-all/rev/fd61bad1a338 I noticed the .replace() wasn’t doing anything useful when I tried adding the other patterns to common-patterns.py last night. Somehow, it not only passed on Windows and Linux for me, it provided the correct output when I eliminated the existing output. Maybe try dropping that tuple in run-tests.py, since I don’t think it’s needed any more. Sorry about that. >> On Mon, Dec 11, 2017 at 5:48 AM, Augie Fackler <raf@durin42.com> wrote: >> >>> On Dec 11, 2017, at 00:24, Matt Harbison <mharbison72@gmail.com> wrote: >>> >>> On Sun, 10 Dec 2017 18:28:03 -0500, Augie Fackler <raf@durin42.com> wrote: >>> >>>> >>>>> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>>>> >>>>> On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote: >>>>> >>>>>> >>>>>>> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote: >>>>>>> >>>>>>> # HG changeset patch >>>>>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>>>>> # Date 1512882971 18000 >>>>>>> # Sun Dec 10 00:16:11 2017 -0500 >>>>>>> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76 >>>>>>> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40 >>>>>>> run-tests: accept '\' vs '/' path differences without '(glob)’ >>>>>> >>>>>> series queued, many thanks >>>>>> >>>>>> […] >>>>>> >>>>>>> It's probably unreasonable to submit a single patch that wipes out all of the >>>>>>> (glob) instances that were only used to hide path differences, given the churn >>>>>>> from other contributors. Since their presence isn't harming the tests, these >>>>>>> can be removed through attrition. >>>>>> >>>>>> You could do it if you wanted with a skip-blame annotation as was done in d92dc725223b. Then we can pass the right filter to `hg annotate` and not see the noop changes. Not a requirement though, don’t feel obligated to do so. >>> >>> OK, done. But 3 of 3 isn't showing up in the mail archive, and at ~290K for that patch, I would have expected a digest email too. So something choked. I'll try to check tomorrow around 8:30 EST and resend if needed, before too much more activity occurs. >> >> It probably got stuck in moderation. I'll mod it through. >> >>> >>> Random thought: should these # skip-blame markers also imply a default skip in bisect? It's probably too late to change the name now... >> >> probably not? It's not a default skip in blame, you have to specify the flag anyway... >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
I think the problem is in this line from the test (that you changed):
$ echo " saved backup bundle to \$TESTTMP\foo.hg" >> test-failure.t
The '\' there probably just needs to be escaped. It seems like at
least dash and tcsh otherwise interpret it (but bash treats it
literally). Proof:
$ dash -c 'echo " saved backup bundle to \$TESTTMP\foo.hg"'
saved backup bundle to $TESTTMP
oo.hg
I'll just change the '\f' to '\\f' in flight and we'll see if tests
pass when they run again.
On Tue, Dec 12, 2017 at 10:06 PM, Matt Harbison <mharbison72@gmail.com> wrote:
>
> On Dec 13, 2017, at 12:40 AM, Martin von Zweigbergk <martinvonz@google.com>
> wrote:
>
> Is it this patch that broke test-run-tests.t? See
> https://buildbot.mercurial-scm.org/builders/hg%20tests/builds/1357/steps/run-tests.py%20%28python%202.7.10%29/logs/stdio.
> I haven't even tried to verify, but Matt and I were both on the blame
> list for the broken build and it didn't seem related to anything I
> did.
>
>
> My guess is that it’s this one:
>
> https://www.mercurial-scm.org/repo/hg-all/rev/fd61bad1a338
>
>
> I noticed the .replace() wasn’t doing anything useful when I tried adding
> the other patterns to common-patterns.py last night. Somehow, it not only
> passed on Windows and Linux for me, it provided the correct output when I
> eliminated the existing output.
>
>
> Maybe try dropping that tuple in run-tests.py, since I don’t think it’s
> needed any more. Sorry about that.
>
>
>
> On Mon, Dec 11, 2017 at 5:48 AM, Augie Fackler <raf@durin42.com> wrote:
>
>
> On Dec 11, 2017, at 00:24, Matt Harbison <mharbison72@gmail.com> wrote:
>
>
> On Sun, 10 Dec 2017 18:28:03 -0500, Augie Fackler <raf@durin42.com> wrote:
>
>
>
> On Dec 10, 2017, at 5:39 PM, Matt Harbison <mharbison72@gmail.com> wrote:
>
>
> On Sun, 10 Dec 2017 16:48:26 -0500, Augie Fackler <raf@durin42.com> wrote:
>
>
>
> On Dec 10, 2017, at 3:47 PM, Matt Harbison <mharbison72@gmail.com> wrote:
>
>
> # HG changeset patch
>
> # User Matt Harbison <matt_harbison@yahoo.com>
>
> # Date 1512882971 18000
>
> # Sun Dec 10 00:16:11 2017 -0500
>
> # Node ID ebc7a05d9389989fc2e81d30fa1c10dd01be9a76
>
> # Parent e1119050f15e2169ac766e6a69eeab70729e9e40
>
> run-tests: accept '\' vs '/' path differences without '(glob)’
>
>
> series queued, many thanks
>
>
> […]
>
>
> It's probably unreasonable to submit a single patch that wipes out all of
> the
>
> (glob) instances that were only used to hide path differences, given the
> churn
>
> from other contributors. Since their presence isn't harming the tests,
> these
>
> can be removed through attrition.
>
>
> You could do it if you wanted with a skip-blame annotation as was done in
> d92dc725223b. Then we can pass the right filter to `hg annotate` and not see
> the noop changes. Not a requirement though, don’t feel obligated to do so.
>
>
> OK, done. But 3 of 3 isn't showing up in the mail archive, and at ~290K for
> that patch, I would have expected a digest email too. So something choked.
> I'll try to check tomorrow around 8:30 EST and resend if needed, before too
> much more activity occurs.
>
>
> It probably got stuck in moderation. I'll mod it through.
>
>
>
> Random thought: should these # skip-blame markers also imply a default skip
> in bisect? It's probably too late to change the name now...
>
>
> probably not? It's not a default skip in blame, you have to specify the flag
> anyway...
>
> _______________________________________________
>
> Mercurial-devel mailing list
>
> Mercurial-devel@mercurial-scm.org
>
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/tests/run-tests.py b/tests/run-tests.py --- a/tests/run-tests.py +++ b/tests/run-tests.py @@ -1451,10 +1451,7 @@ r = self.linematch(el, lout) if isinstance(r, str): - if r == '+glob': - lout = el[:-1] + ' (glob)\n' - r = '' # Warn only this line. - elif r == '-glob': + if r == '-glob': lout = ''.join(el.rsplit(' (glob)', 1)) r = '' # Warn only this line. elif r == "retry": @@ -1613,7 +1610,7 @@ if os.altsep: _l = l.replace(b'\\', b'/') if el == _l or os.name == 'nt' and el[:-1] + b'\r\n' == _l: - return b'+glob' + return True return retry @staticmethod diff --git a/tests/test-run-tests.py b/tests/test-run-tests.py --- a/tests/test-run-tests.py +++ b/tests/test-run-tests.py @@ -67,9 +67,9 @@ missing glob >>> lm(b'/g/c/d/fg\n', b'\\g\\c\\d/fg\n') - 'special: +glob' + True >>> lm(b'/g/c/d/fg\n', b'\\g\\c\\d\\fg\r\n') - 'special: +glob' + True restore os.altsep >>> os.altsep = _osaltsep