# HG changeset patch # User Mads Kiilerich # Date 2014-08-26 20:03:32 # Node ID 650b5b6e75ed7f357ae486eb55056cdfa91bb9aa # Parent 926bc0d3b595caf37c5d70833a347eb43285de2f convert: use None value for missing files instead of overloading IOError The internal API used IOError to indicate that a file should be marked as removed. There is some correlation between IOError (especially with ENOENT) and files that should be removed, but using IOErrors to represent file removal internally required some hacks. Instead, use the value None to indicate that the file not is present. Before, spurious IO errors could cause commits that silently removed files. They will now be reported like all other IO errors so the root cause can be fixed. diff --git a/hgext/convert/bzr.py b/hgext/convert/bzr.py --- a/hgext/convert/bzr.py +++ b/hgext/convert/bzr.py @@ -122,8 +122,7 @@ class bzr_source(converter_source): kind = revtree.kind(fileid) if kind not in supportedkinds: # the file is not available anymore - was deleted - raise IOError(_('%s is not available in %s anymore') % - (name, rev)) + return None, None mode = self._modecache[(name, rev)] if kind == 'symlink': target = revtree.get_symlink_target(fileid) diff --git a/hgext/convert/common.py b/hgext/convert/common.py --- a/hgext/convert/common.py +++ b/hgext/convert/common.py @@ -88,8 +88,8 @@ class converter_source(object): def getfile(self, name, rev): """Return a pair (data, mode) where data is the file content as a string and mode one of '', 'x' or 'l'. rev is the - identifier returned by a previous call to getchanges(). Raise - IOError to indicate that name was deleted in rev. + identifier returned by a previous call to getchanges(). + Data is None if file is missing/deleted in rev. """ raise NotImplementedError diff --git a/hgext/convert/cvs.py b/hgext/convert/cvs.py --- a/hgext/convert/cvs.py +++ b/hgext/convert/cvs.py @@ -220,7 +220,7 @@ class convert_cvs(converter_source): self._parse() if rev.endswith("(DEAD)"): - raise IOError + return None, None args = ("-N -P -kk -r %s --" % rev).split() args.append(self.cvsrepo + '/' + name) diff --git a/hgext/convert/darcs.py b/hgext/convert/darcs.py --- a/hgext/convert/darcs.py +++ b/hgext/convert/darcs.py @@ -8,7 +8,7 @@ from common import NoRepo, checktool, commandline, commit, converter_source from mercurial.i18n import _ from mercurial import util -import os, shutil, tempfile, re +import os, shutil, tempfile, re, errno # The naming drift of ElementTree is fun! @@ -192,8 +192,13 @@ class darcs_source(converter_source, com if rev != self.lastrev: raise util.Abort(_('internal calling inconsistency')) path = os.path.join(self.tmppath, name) - data = util.readfile(path) - mode = os.lstat(path).st_mode + try: + data = util.readfile(path) + mode = os.lstat(path).st_mode + except IOError, inst: + if inst.errno == errno.ENOENT: + return None, None + raise mode = (mode & 0111) and 'x' or '' return data, mode diff --git a/hgext/convert/git.py b/hgext/convert/git.py --- a/hgext/convert/git.py +++ b/hgext/convert/git.py @@ -135,7 +135,7 @@ class convert_git(converter_source): def getfile(self, name, rev): if rev == hex(nullid): - raise IOError + return None, None if name == '.hgsub': data = '\n'.join([m.hgsub() for m in self.submoditer()]) mode = '' diff --git a/hgext/convert/gnuarch.py b/hgext/convert/gnuarch.py --- a/hgext/convert/gnuarch.py +++ b/hgext/convert/gnuarch.py @@ -137,9 +137,8 @@ class gnuarch_source(converter_source, c if rev != self.lastrev: raise util.Abort(_('internal calling inconsistency')) - # Raise IOError if necessary (i.e. deleted files). if not os.path.lexists(os.path.join(self.tmppath, name)): - raise IOError + return None, None return self._getfile(name, rev) diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py --- a/hgext/convert/hg.py +++ b/hgext/convert/hg.py @@ -134,6 +134,8 @@ class mercurial_sink(converter_sink): def getfilectx(repo, memctx, f): v = files[f] data, mode = source.getfile(f, v) + if data is None: + return None if f == '.hgtags': data = self._rewritetags(source, revmap, data) return context.memfilectx(self.repo, f, data, 'l' in mode, @@ -351,8 +353,8 @@ class mercurial_source(converter_source) try: fctx = self.changectx(rev)[name] return fctx.data(), fctx.flags() - except error.LookupError, err: - raise IOError(err) + except error.LookupError: + return None, None def getchanges(self, rev): ctx = self.changectx(rev) diff --git a/hgext/convert/monotone.py b/hgext/convert/monotone.py --- a/hgext/convert/monotone.py +++ b/hgext/convert/monotone.py @@ -282,11 +282,11 @@ class monotone_source(converter_source, def getfile(self, name, rev): if not self.mtnisfile(name, rev): - raise IOError # file was deleted or renamed + return None, None try: data = self.mtnrun("get_file_of", name, r=rev) except Exception: - raise IOError # file was deleted or renamed + return None, None self.mtnloadmanifest(rev) node, attr = self.files.get(name, (None, "")) return data, attr diff --git a/hgext/convert/p4.py b/hgext/convert/p4.py --- a/hgext/convert/p4.py +++ b/hgext/convert/p4.py @@ -183,7 +183,7 @@ class p4_source(converter_source): contents += data if mode is None: - raise IOError(0, "bad stat") + return None, None if keywords: contents = keywords.sub("$\\1$", contents) diff --git a/hgext/convert/subversion.py b/hgext/convert/subversion.py --- a/hgext/convert/subversion.py +++ b/hgext/convert/subversion.py @@ -933,7 +933,7 @@ class svn_source(converter_source): def getfile(self, file, rev): # TODO: ra.get_file transmits the whole file instead of diffs. if file in self.removed: - raise IOError + return None, None mode = '' try: new_module, revnum = revsplit(rev)[1:] @@ -954,7 +954,7 @@ class svn_source(converter_source): notfound = (svn.core.SVN_ERR_FS_NOT_FOUND, svn.core.SVN_ERR_RA_DAV_PATH_NOT_FOUND) if e.apr_err in notfound: # File not found - raise IOError + return None, None raise if mode == 'l': link_prefix = "link " @@ -1236,9 +1236,8 @@ class svn_sink(converter_sink, commandli # Apply changes to working copy for f, v in files: - try: - data, mode = source.getfile(f, v) - except IOError: + data, mode = source.getfile(f, v) + if data is None: self.delete.append(f) else: self.putfile(f, mode, data) diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -283,7 +283,7 @@ def collapse(repo, first, last, commitop isexec='x' in flags, copied=copied.get(path)) return mctx - raise IOError() + return None if commitopts.get('message'): message = commitopts['message'] diff --git a/hgext/largefiles/lfcommands.py b/hgext/largefiles/lfcommands.py --- a/hgext/largefiles/lfcommands.py +++ b/hgext/largefiles/lfcommands.py @@ -146,7 +146,7 @@ def _addchangeset(ui, rsrc, rdst, ctx, r try: fctx = ctx.filectx(lfutil.standin(f)) except error.LookupError: - raise IOError + return None renamed = fctx.renamed() if renamed: renamed = lfutil.splitstandin(renamed[0]) @@ -248,7 +248,7 @@ def _lfconvert_addchangeset(rsrc, rdst, try: fctx = ctx.filectx(srcfname) except error.LookupError: - raise IOError + return None renamed = fctx.renamed() if renamed: # standin is always a largefile because largefile-ness @@ -298,7 +298,7 @@ def _getnormalcontext(repo, ctx, f, revm try: fctx = ctx.filectx(f) except error.LookupError: - raise IOError + return None renamed = fctx.renamed() if renamed: renamed = renamed[0] diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2130,7 +2130,7 @@ def amend(ui, repo, commitfunc, old, ext copied=copied.get(path)) return mctx except KeyError: - raise IOError + return None else: ui.note(_('copying changeset %s to %s\n') % (old, base)) @@ -2139,7 +2139,7 @@ def amend(ui, repo, commitfunc, old, ext try: return old.filectx(path) except KeyError: - raise IOError + return None user = opts.get('user') or old.user() date = opts.get('date') or old.date() diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -347,7 +347,10 @@ class basectx(object): def makememctx(repo, parents, text, user, date, branch, files, store, editor=None): def getfilectx(repo, memctx, path): - data, (islink, isexec), copied = store.getfile(path) + data, mode, copied = store.getfile(path) + if data is None: + return None + islink, isexec = mode return memfilectx(repo, path, data, islink=islink, isexec=isexec, copied=copied, memctx=memctx) extra = {} @@ -1626,7 +1629,9 @@ class memctx(committablectx): self._repo.savecommitmessage(self._text) def filectx(self, path, filelog=None): - """get a file context from the working directory""" + """get a file context from the working directory + + Returns None if file doesn't exist and should be removed.""" return self._filectxfn(self._repo, self, path) def commit(self): diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -1394,9 +1394,12 @@ class localrepository(object): self.ui.note(f + "\n") try: fctx = ctx[f] - new[f] = self._filecommit(fctx, m1, m2, linkrev, trp, - changed) - m1.set(f, fctx.flags()) + if fctx is None: + removed.append(f) + else: + new[f] = self._filecommit(fctx, m1, m2, linkrev, + trp, changed) + m1.set(f, fctx.flags()) except OSError, inst: self.ui.warn(_("trouble committing %s!\n") % f) raise @@ -1404,9 +1407,7 @@ class localrepository(object): errcode = getattr(inst, 'errno', errno.ENOENT) if error or errcode and errcode != errno.ENOENT: self.ui.warn(_("trouble committing %s!\n") % f) - raise - else: - removed.append(f) + raise # update manifest m1.update(new) diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -382,7 +382,7 @@ class abstractbackend(object): def getfile(self, fname): """Return target file data and flags as a (data, (islink, - isexec)) tuple. + isexec)) tuple. Data is None if file is missing/deleted. """ raise NotImplementedError @@ -426,7 +426,12 @@ class fsbackend(abstractbackend): except OSError, e: if e.errno != errno.ENOENT: raise - return (self.opener.read(fname), (False, isexec)) + try: + return (self.opener.read(fname), (False, isexec)) + except IOError, e: + if e.errno != errno.ENOENT: + raise + return None, None def setfile(self, fname, data, mode, copysource): islink, isexec = mode @@ -528,7 +533,7 @@ class filestore(object): if fname in self.data: return self.data[fname] if not self.opener or fname not in self.files: - raise IOError + return None, None, None fn, mode, copied = self.files[fname] return self.opener.read(fn), mode, copied @@ -554,7 +559,7 @@ class repobackend(abstractbackend): try: fctx = self.ctx[fname] except error.LookupError: - raise IOError + return None, None flags = fctx.flags() return fctx.data(), ('l' in flags, 'x' in flags) @@ -597,13 +602,12 @@ class patchfile(object): self.copysource = gp.oldpath self.create = gp.op in ('ADD', 'COPY', 'RENAME') self.remove = gp.op == 'DELETE' - try: - if self.copysource is None: - data, mode = backend.getfile(self.fname) - self.exists = True - else: - data, mode = store.getfile(self.copysource)[:2] - self.exists = backend.exists(self.fname) + if self.copysource is None: + data, mode = backend.getfile(self.fname) + else: + data, mode = store.getfile(self.copysource)[:2] + if data is not None: + self.exists = self.copysource is None or backend.exists(self.fname) self.missing = False if data: self.lines = mdiff.splitnewlines(data) @@ -622,7 +626,7 @@ class patchfile(object): l = l[:-2] + '\n' nlines.append(l) self.lines = nlines - except IOError: + else: if self.create: self.missing = False if self.mode is None: @@ -1380,6 +1384,8 @@ def _applydiff(ui, fp, patcher, backend, data, mode = None, None if gp.op in ('RENAME', 'COPY'): data, mode = store.getfile(gp.oldpath)[:2] + # FIXME: failing getfile has never been handled here + assert data is not None if gp.mode: mode = gp.mode if gp.op == 'ADD': @@ -1404,15 +1410,13 @@ def _applydiff(ui, fp, patcher, backend, elif state == 'git': for gp in values: path = pstrip(gp.oldpath) - try: - data, mode = backend.getfile(path) - except IOError, e: - if e.errno != errno.ENOENT: - raise + data, mode = backend.getfile(path) + if data is None: # The error ignored here will trigger a getfile() # error in a place more appropriate for error # handling, and will not interrupt the patching # process. + pass else: store.setfile(path, data, mode) else: