| Submitter | phabricator |
|---|---|
| Date | Oct. 13, 2017, 7:45 p.m. |
| Message ID | <differential-rev-PHID-DREV-orhszjlcleaechdby4n3-req@phab.mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/24849/ |
| State | Superseded |
| Headers | show |
Comments
durin42 requested changes to this revision. durin42 added a comment. This revision now requires changes to proceed. (Feel free to push back with a TODO documenting how to do a fix or something.) INLINE COMMENTS > filemerge.py:604 > return None > - > + from . import context > a = _workingpath(repo, fcd) Well that's a bummer. Could we move context.*filectx* into mercurial.filecontext and then avoid the need for this function-level import? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1057 To: phillco, #hg-reviewers, durin42 Cc: durin42, mercurial-devel
phillco added inline comments. INLINE COMMENTS > durin42 wrote in filemerge.py:604 > Well that's a bummer. Could we move context.*filectx* into mercurial.filecontext and then avoid the need for this function-level import? I don't think so, because the filectx classes reference the ctx classes in various places. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1057 To: phillco, #hg-reviewers, durin42 Cc: durin42, mercurial-devel
durin42 added inline comments. INLINE COMMENTS > phillco wrote in filemerge.py:604 > I don't think so, because the filectx classes reference the ctx classes in various places. 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1057 To: phillco, #hg-reviewers, durin42 Cc: durin42, mercurial-devel
phillco added inline comments. INLINE COMMENTS > durin42 wrote in filemerge.py:604 > 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. Sure, will do. I think most likely by breaking the context -> fileset dependency. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1057 To: phillco, #hg-reviewers, durin42 Cc: durin42, mercurial-devel
phillco marked 4 inline comments as done. phillco added inline comments. INLINE COMMENTS > phillco wrote in filemerge.py:604 > Sure, will do. I think most likely by breaking the context -> fileset dependency. @durin42 done REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1057 To: phillco, #hg-reviewers, durin42 Cc: durin42, mercurial-devel
Patch
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py --- a/mercurial/filemerge.py +++ b/mercurial/filemerge.py @@ -7,7 +7,6 @@ from __future__ import absolute_import -import filecmp import os import re import tempfile @@ -226,9 +225,9 @@ return '\n' return None # unknown -def _matcheol(file, origfile): +def _matcheol(file, back): "Convert EOL markers in a file to match origfile" - tostyle = _eoltype(util.readfile(origfile)) + tostyle = _eoltype(back.data()) # No repo.wread filters? if tostyle: data = util.readfile(file) style = _eoltype(data) @@ -505,7 +504,9 @@ args = _toolstr(ui, tool, "args", '$local $base $other') if "$output" in args: - out, a = a, back # read input from backup, write to original + # read input from backup, write to original + out = a + a = repo.wvfs.join(back.path()) replace = {'local': a, 'base': b, 'other': c, 'output': out} args = util.interpolate(r'\$', replace, args, lambda s: util.shellquote(util.localpath(s))) @@ -588,24 +589,24 @@ def _restorebackup(fcd, back): # TODO: Add a workingfilectx.write(otherfilectx) path so we can use # util.copy here instead. - fcd.write(util.readfile(back), fcd.flags()) + fcd.write(back.data(), fcd.flags()) -def _makebackup(repo, ui, fcd, premerge): - """Makes a backup of the local `fcd` file prior to merging. +def _makebackup(repo, ui, wctx, fcd, premerge): + """Makes and returns a filectx-like object for ``fcd``'s backup file. In addition to preserving the user's pre-existing modifications to `fcd` (if any), the backup is used to undo certain premerges, confirm whether a merge changed anything, and determine what line endings the new file should have. """ if fcd.isabsent(): return None - + from . import context a = _workingpath(repo, fcd) back = scmutil.origpath(ui, repo, a) if premerge: util.copyfile(a, back) - return back + return context.arbitraryfilectx(back, repo=repo) def _maketempfiles(repo, fco, fca): """Writes out `fco` and `fca` as temporary files, so an external merge @@ -691,7 +692,7 @@ ui.warn(onfailure % fd) return True, 1, False - back = _makebackup(repo, ui, fcd, premerge) + back = _makebackup(repo, ui, wctx, fcd, premerge) files = (None, None, None, back) r = 1 try: @@ -719,7 +720,7 @@ return True, r, deleted finally: if not r and back is not None: - util.unlink(back) + back.remove() def _check(repo, r, ui, tool, fcd, files): fd = fcd.path() @@ -741,7 +742,7 @@ if not r and not checked and (_toolbool(ui, tool, "checkchanged") or 'changed' in _toollist(ui, tool, "check")): - if back is not None and filecmp.cmp(_workingpath(repo, fcd), back): + if back is not None and not fcd.cmp(back): if ui.promptchoice(_(" output file %s appears unchanged\n" "was merge successful (yn)?" "$$ &Yes $$ &No") % fd, 1):