( )⚙ D1057 filemerge: use arbitraryfilectx for backups

This is an archive of the discontinued Mercurial Phabricator instance.

filemerge: use arbitraryfilectx for backups
ClosedPublic

Authored by phillco on Oct 13 2017, 3:45 PM.

Details

Summary

With in-memory merge, backup files might be overlayworkingfilectxs stored
in memory. But they could also be real files if the user's backup directory is
outside the working dir.

Rather than have two code paths everywhere, let's use arbitraryfilectx so they
can be consistent.

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 13 2017, 3:45 PM
durin42 requested changes to this revision.Oct 14 2017, 12:53 AM
durin42 added a subscriber: durin42.

(Feel free to push back with a TODO documenting how to do a fix or something.)

mercurial/filemerge.py
604–606

Well that's a bummer. Could we move context.*filectx* into mercurial.filecontext and then avoid the need for this function-level import?

This revision now requires changes to proceed.Oct 14 2017, 12:53 AM
phillco added inline comments.Oct 15 2017, 11:25 PM
mercurial/filemerge.py
604–606

I don't think so, because the filectx classes reference the ctx classes in various places.

phillco retitled this revision from use arbitraryfilectx for backups to filemerge: use arbitraryfilectx for backups.Oct 15 2017, 11:36 PM
durin42 added inline comments.Oct 16 2017, 3:03 PM
mercurial/filemerge.py
604–606

Ick. Please add a TODO here to figure out a way to break the cycle (it can be done! I'm sure of it, even if I don't yet know how) and then we can move forward with this.

phillco added inline comments.Oct 16 2017, 4:07 PM
mercurial/filemerge.py
604–606

Sure, will do. I think most likely by breaking the context -> fileset dependency.

phillco updated this revision to Diff 2843.Oct 16 2017, 4:11 PM
phillco marked 4 inline comments as done.Oct 16 2017, 4:11 PM
phillco added inline comments.
mercurial/filemerge.py
604–606
durin42 accepted this revision.Oct 16 2017, 9:18 PM
This revision is now accepted and ready to land.Oct 16 2017, 9:18 PM
phillco marked an inline comment as done.Oct 16 2017, 9:46 PM
This revision was automatically updated to reflect the committed changes.