This is an archive of the discontinued Mercurial Phabricator instance.

Differential D772

treemanifest: don't error out if a local commit doesn't have a tree
ClosedPublic

Authored by durham on Sep 22 2017, 12:23 AM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGX846341ae87fa: treemanifest: don't error out if a local commit doesn't have a tree
Summary

In hybrid tree/flat mode, if a local commit accidentally got marked as public,
then we would attempt to access the server, which of course didn't have the
commit and failed. This caused an error on the client, when in reality we
should've just fallen back to the flat manifest. Let's eat the error and the
message in this case.

Diff Detail

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

Event Timeline

durham created this revision.Sep 22 2017, 12:23 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 22 2017, 12:23 AM
quark added a subscriber: quark.Sep 22 2017, 12:48 AM
quark added inline comments.
fastmanifest/implementation.py
166

except Exception?

treemanifest/__init__.py
1030

I think it will look safer if popbuffer is in a finally block (same as above). This may eat everything if _cleanup raises.

quark accepted this revision.Sep 22 2017, 12:48 AM

Suppose popbuffer gets moved to finally.

This revision is now accepted and ready to land.Sep 22 2017, 12:48 AM
Closed by commit rFBHGX846341ae87fa: treemanifest: don't error out if a local commit doesn't have a tree (authored by durham). · Explain WhySep 22 2017, 11:32 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mfastmanifest/implementation.py (24 lines)
Mtreemanifest/__init__.py (10 lines)

Diff 2020

fastmanifest/implementation.py

# implementation.py # implementation.py
# #
# Copyright 2016 Facebook, Inc. # Copyright 2016 Facebook, Inc.
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
import collections import collections
import os import os
import time import time
import heapq import heapq
from mercurial import manifest, mdiff, revlog, util from mercurial import (
error,
manifest,
mdiff,
revlog,
util,
)
import cachemanager import cachemanager
import cfastmanifest import cfastmanifest
from metrics import metricscollector from metrics import metricscollector
from constants import ( from constants import (
CACHE_SUBDIR, CACHE_SUBDIR,
DEFAULT_MAX_MEMORY_ENTRIES, DEFAULT_MAX_MEMORY_ENTRIES,
) )
return None return None
if self.node in self.treecache: if self.node in self.treecache:
self.__treemanifest = self.treecache[self.node] self.__treemanifest = self.treecache[self.node]
elif self.node == revlog.nullid: elif self.node == revlog.nullid:
store = self.manifestlog.datastore store = self.manifestlog.datastore
self.__treemanifest = cstore.treemanifest(store) self.__treemanifest = cstore.treemanifest(store)
else: else:
store = self.manifestlog.datastore store = self.manifestlog.datastore
self.ui.pushbuffer()
try: try:
store.get('', self.node) store.get('', self.node)
self.__treemanifest = cstore.treemanifest(store, self.__treemanifest = cstore.treemanifest(store,
self.node) self.node)
except KeyError: # The buffer is only to eat certain errors, so show
# non-error messages.
output = self.ui.popbuffer()
if output:
self.ui.status(output)
except (KeyError, error.Abort):
# Record that it doesn't exist, so we don't keep checking # Record that it doesn't exist, so we don't keep checking
# the store. # the store.
self.ui.popbuffer()
# Eat the buffer so we don't print a remote: warning
self.__treemanifest = False self.__treemanifest = False
return None return None
except:
quarkUnsubmitted
Not Done

except Exception?

quark: `except Exception`?
# Other errors should be printed
output = self.ui.popbuffer()
if output:
self.ui.status(output)
raise
return self.__treemanifest return self.__treemanifest
def _incache(self): def _incache(self):
if self.incache or self.debugfastmanifest: if self.incache or self.debugfastmanifest:
return True return True
elif self.cachekey: elif self.cachekey:
return self.cachekey in self.fastcache return self.cachekey in self.fastcache

treemanifest/__init__.py

if missingnodes: if missingnodes:
raise error.Abort(_("unable to download %d trees (%s,...)") % raise error.Abort(_("unable to download %d trees (%s,...)") %
(len(missingnodes), list(missingnodes)[0])) (len(missingnodes), list(missingnodes)[0]))
except bundle2.AbortFromPart as exc: except bundle2.AbortFromPart as exc:
repo.ui.status(_('remote: abort: %s\n') % exc) repo.ui.status(_('remote: abort: %s\n') % exc)
raise error.Abort(_('pull failed on remote'), hint=exc.hint) raise error.Abort(_('pull failed on remote'), hint=exc.hint)
except error.BundleValueError as exc: except error.BundleValueError as exc:
raise error.Abort(_('missing support for %s') % exc) raise error.Abort(_('missing support for %s') % exc)
finally:
# Manually destruct the peer, so we can collect any error output
remote.ui.pushbuffer()
remote._cleanup()
quarkUnsubmitted
Not Done

I think it will look safer if popbuffer is in a finally block (same as above). This may eat everything if _cleanup raises.

quark: I think it will look safer if `popbuffer` is in a finally block (same as above). This may eat…
# Redirect cleanup output to the repo.ui so callers can buffer it if
# desired.
output = remote.ui.popbuffer()
if output:
repo.ui.status(output)
def _registerbundle2parts(): def _registerbundle2parts():
@bundle2.parthandler(TREEGROUP_PARTTYPE2, ('version', 'cache', 'category')) @bundle2.parthandler(TREEGROUP_PARTTYPE2, ('version', 'cache', 'category'))
def treeparthandler2(op, part): def treeparthandler2(op, part):
"""Handles received tree packs. If `cache` is True, the received """Handles received tree packs. If `cache` is True, the received
data goes in to the shared pack cache. Otherwise, the received data data goes in to the shared pack cache. Otherwise, the received data
goes into the permanent repo local data. goes into the permanent repo local data.
""" """