This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5801

files: respect ui.relative-paths
ClosedPublic

Authored by martinvonz on Feb 2 2019, 2:12 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG15f63ac122ea: files: respect ui.relative-paths
rHGce0a4ed25ebc: files: respect ui.relative-paths
Summary

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Feb 2 2019, 2:12 AM
martinvonz updated this revision to Diff 13733.Feb 3 2019, 12:46 PM
martinvonz updated this revision to Diff 13812.Feb 5 2019, 12:48 PM
Closed by commit rHGce0a4ed25ebc: files: respect ui.relative-paths (authored by martinvonz). · Explain WhyFeb 5 2019, 5:45 PM
This revision was automatically updated to reflect the committed changes.
mharbison72 added a subscriber: mharbison72.Feb 16 2019, 10:45 PM

For some reason, this breaks test-contrib-perf.t:

https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/1274/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio

I also tried setting True and False in ui.relative-paths, to no effect. Any ideas?

martinvonz added a comment.Feb 17 2019, 12:29 PM
In D5801#87273, @mharbison72 wrote:

For some reason, this breaks test-contrib-perf.t:
https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/1274/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
I also tried setting True and False in ui.relative-paths, to no effect. Any ideas?

I noticed another bug and sent D5978. Maybe your test failure is because you're using the eol extension? I don't know what else would cause the \r in contrib.perf. I have no idea how that's related to this patch, though.

mharbison72 added a comment.Feb 17 2019, 3:15 PM
In D5801#87296, @martinvonz wrote:

I noticed another bug and sent D5978. Maybe your test failure is because you're using the eol extension? I don't know what else would cause the \r in contrib.perf. I have no idea how that's related to this patch, though.

The \r is how output normally is on Windows. The test harness accounts for this when matching lines, but displays the actual output (with \r) when there are differences. The eol extension isn’t configured on this machine, but the custom HGRCPATH content from the test harness would override that anyway.

martinvonz added a comment.Feb 17 2019, 5:21 PM
In D5801#87300, @mharbison72 wrote:
In D5801#87296, @martinvonz wrote:

I noticed another bug and sent D5978. Maybe your test failure is because you're using the eol extension? I don't know what else would cause the \r in contrib.perf. I have no idea how that's related to this patch, though.

The \r is how output normally is on Windows. The test harness accounts for this when matching lines, but displays the actual output (with \r) when there are differences. The eol extension isn’t configured on this machine, but the custom HGRCPATH content from the test harness would override that anyway.

Ah, so the only difference is the extra "import newer module separately in try clause for early Mercurial" in contrib/perf.py then. This patch didn't change that file. Can you check again that it was this patch that caused that and that it's not just flaky?

In D5801#87300, @mharbison72 wrote:
In D5801#87296, @martinvonz wrote:

I noticed another bug and sent D5978. Maybe your test failure is because you're using the eol extension? I don't know what else would cause the \r in contrib.perf. I have no idea how that's related to this patch, though.

The \r is how output normally is on Windows. The test harness accounts for this when matching lines, but displays the actual output (with \r) when there are differences. The eol extension isn’t configured on this machine, but the custom HGRCPATH content from the test harness would override that anyway.

mharbison72 added a comment.Feb 17 2019, 5:52 PM
In D5801#87302, @martinvonz wrote:
In D5801#87300, @mharbison72 wrote:
In D5801#87296, @martinvonz wrote:

I noticed another bug and sent D5978. Maybe your test failure is because you're using the eol extension? I don't know what else would cause the \r in contrib.perf. I have no idea how that's related to this patch, though.

The \r is how output normally is on Windows. The test harness accounts for this when matching lines, but displays the actual output (with \r) when there are differences. The eol extension isn’t configured on this machine, but the custom HGRCPATH content from the test harness would override that anyway.

Ah, so the only difference is the extra "import newer module separately in try clause for early Mercurial" in contrib/perf.py then. This patch didn't change that file. Can you check again that it was this patch that caused that and that it's not just flaky?

Correct. The extra noise makes it hard to see the actual problems sometimes, and it makes trivial things like fixing (glob) endings more of a nuisance. But I don't see a way to handle that.

This one is definitely the problem, because the failing *.t invokes hg files to generate the whitelist. It turns out, we can either set ui.slash=False or just not use os.sep in check-perf-code.py. I'm leaning toward the latter.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (3 lines)
Mtests/test-locate.t (18 lines)

Diff 13837

mercurial/cmdutil.py

bad.extend(f for f in rejected if f in match.files()) bad.extend(f for f in rejected if f in match.files())
forgot.extend(f for f in forget if f not in rejected) forgot.extend(f for f in forget if f not in rejected)
return bad, forgot return bad, forgot
def files(ui, ctx, m, fm, fmt, subrepos): def files(ui, ctx, m, fm, fmt, subrepos):
ret = 1 ret = 1
needsfctx = ui.verbose or {'size', 'flags'} & fm.datahint() needsfctx = ui.verbose or {'size', 'flags'} & fm.datahint()
uipathfn = scmutil.getuipathfn(ctx.repo(), legacyrelativevalue=True)
for f in ctx.matches(m): for f in ctx.matches(m):
fm.startitem() fm.startitem()
fm.context(ctx=ctx) fm.context(ctx=ctx)
if needsfctx: if needsfctx:
fc = ctx[f] fc = ctx[f]
fm.write('size flags', '% 10d % 1s ', fc.size(), fc.flags()) fm.write('size flags', '% 10d % 1s ', fc.size(), fc.flags())
fm.data(path=f) fm.data(path=f)
fm.plain(fmt % m.rel(f)) fm.plain(fmt % uipathfn(f))
ret = 0 ret = 0
for subpath in sorted(ctx.substate): for subpath in sorted(ctx.substate):
submatch = matchmod.subdirmatcher(subpath, m) submatch = matchmod.subdirmatcher(subpath, m)
if (subrepos or m.exact(subpath) or any(submatch.files())): if (subrepos or m.exact(subpath) or any(submatch.files())):
sub = ctx.sub(subpath) sub = ctx.sub(subpath)
try: try:
recurse = m.exact(subpath) or subrepos recurse = m.exact(subpath) or subrepos

tests/test-locate.t

../t/e.h ../t/e.h
../t/x ../t/x
$ hg files path:. $ hg files path:.
../b ../b
../dir.h/foo ../dir.h/foo
../t.h ../t.h
../t/e.h ../t/e.h
../t/x ../t/x
$ hg files --config ui.relative-paths=yes
../b
../dir.h/foo
../t.h
../t/e.h
../t/x
$ hg files --config ui.relative-paths=no
b
dir.h/foo
t.h
t/e.h
t/x
$ hg files --config ui.relative-paths=legacy
../b
../dir.h/foo
../t.h
../t/e.h
../t/x
$ hg locate b $ hg locate b
../b ../b
../t/b ../t/b
$ hg locate '*.h' $ hg locate '*.h'
../t.h ../t.h
../t/e.h ../t/e.h
$ hg locate path:t/x $ hg locate path:t/x