( )⚙ D1122 arbitraryfilecontext: skip the cmp fast path if any side is a symlink

This is an archive of the discontinued Mercurial Phabricator instance.

arbitraryfilecontext: skip the cmp fast path if any side is a symlink
ClosedPublic

Authored by phillco on Oct 16 2017, 3:01 PM.

Details

Summary

filecmp follows symlinks by default, which a filectx.cmp() call should not
be doing as it should only compare the requested entry. After this patch, only
the contexts' data are compared, which is the correct contract.

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

phillco created this revision.Oct 16 2017, 3:01 PM
phillco edited the summary of this revision. (Show Details)Oct 16 2017, 3:36 PM
phillco updated this revision to Diff 2838.
phillco updated this revision to Diff 2839.Oct 16 2017, 3:37 PM
phillco updated this revision to Diff 2840.
phillco edited the summary of this revision. (Show Details)Oct 16 2017, 3:41 PM
phillco updated this revision to Diff 2841.
durin42 accepted this revision.Oct 16 2017, 9:42 PM
This revision is now accepted and ready to land.Oct 16 2017, 9:42 PM
This revision was automatically updated to reflect the committed changes.

Hm, check-code is failing here.

I'm dropping this from hg-committed for now. Please re-submit with the check-code issues fixed.

@hg-reviewers: If I'm doing this wrong by dropping the patch, please let me know so I can correct my behavior in the future.

tests/test-arbitraryfilectx.t
53

why is there a space after the e here?

No, that sounds fair, Phil should resend (as a new differential ID) when the issue is sorted.

Whoops, sorry, I forgot to run tests on the most recent version. Will fix & resend.

phillco updated this revision to Diff 2941.Oct 17 2017, 3:40 PM

I've sent D1165, which is the corrected version.