Submitter | Pierre-Yves David |
---|---|
Date | Jan. 2, 2013, 1:36 a.m. |
Message ID | <a397f42becefc44489d4.1357090560@yamac.lan> |
Download | mbox | patch |
Permalink | /patch/365/ |
State | Accepted |
Commit | 518c1403838f1d869fec357488d9221fea765177 |
Headers | show |
Comments
On Jan 1, 2013, at 7:36 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org> > # Date 1356738018 -3600 > # Node ID a397f42becefc44489d4d2cffb14ac1999aea375 > # Parent d2f15a6470d3566b722379d48da8c88b26f398fd > clfilter: fallback to unfiltered version when linkrev point to filtered history > [...] > diff --git a/mercurial/context.py b/mercurial/context.py > --- a/mercurial/context.py > +++ b/mercurial/context.py > @@ -416,11 +416,30 @@ class filectx(object): > if fileid is not None: > self._fileid = fileid > > @propertycache > def _changectx(self): > - return changectx(self._repo, self._changeid) > + try: > + return changectx(self._repo, self._changeid) > + except error.RepoLookupError: > + # Linkrev may point to any revision in the repository. When the > + # repository is filtered this may lead to `filectx` trying to build > + # `changectx` for filtered revision. In such case we fallback to > + # creating `changectx` on the unfiltered version of the reposition. > + # This fallback should not be an issue because`changectx` from > + # `filectx` are not used in complexe operation that care about > + # filtering. > + # > + # This fallback is a cheap and dirty fix that prevent several > + # crash. It does not ensure the behavior is correct. However the > + # behavior was not correct before filtering either and "incorrect > + # behavior" is seen as better as "crash" > + # > + # Linkrevs have several serious troubles with filtering that are > + # complicated to solve. Proper handling of the issue here should be > + # considered when solving linkrev issue are on the table. > + return changectx(self._repo.unfiltered(), self._changeid) Won't this blow the stack if the linkrev isn't in the changelog? This would only happen on a corrupt repo, but it seems like an unnecessarily spectacular way to fail on a bad filelog. pacem in terris / ??? / ?????? / ????????? / ?? Kevin R. Bullock
On 3 janv. 2013, at 22:41, Kevin Bullock wrote: > On Jan 1, 2013, at 7:36 PM, Pierre-Yves David wrote: > >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org> >> # Date 1356738018 -3600 >> # Node ID a397f42becefc44489d4d2cffb14ac1999aea375 >> # Parent d2f15a6470d3566b722379d48da8c88b26f398fd >> clfilter: fallback to unfiltered version when linkrev point to filtered history >> [...] >> diff --git a/mercurial/context.py b/mercurial/context.py >> --- a/mercurial/context.py >> +++ b/mercurial/context.py >> @@ -416,11 +416,30 @@ class filectx(object): >> if fileid is not None: >> self._fileid = fileid >> >> @propertycache >> def _changectx(self): >> - return changectx(self._repo, self._changeid) >> + try: >> + return changectx(self._repo, self._changeid) >> + except error.RepoLookupError: >> + # Linkrev may point to any revision in the repository. When the >> + # repository is filtered this may lead to `filectx` trying to build >> + # `changectx` for filtered revision. In such case we fallback to >> + # creating `changectx` on the unfiltered version of the reposition. >> + # This fallback should not be an issue because`changectx` from >> + # `filectx` are not used in complexe operation that care about >> + # filtering. >> + # >> + # This fallback is a cheap and dirty fix that prevent several >> + # crash. It does not ensure the behavior is correct. However the >> + # behavior was not correct before filtering either and "incorrect >> + # behavior" is seen as better as "crash" >> + # >> + # Linkrevs have several serious troubles with filtering that are >> + # complicated to solve. Proper handling of the issue here should be >> + # considered when solving linkrev issue are on the table. >> + return changectx(self._repo.unfiltered(), self._changeid) > > Won't this blow the stack if the linkrev isn't in the changelog? This would only happen on a corrupt repo, but it seems like an unnecessarily spectacular way to fail on a bad filelog. No, it's an alternative call, not a recursive one. When the second call fails re exception reach the caller as before.
On 3 Jan 2013, at 5:37 PM, Pierre-Yves David wrote: > On 3 janv. 2013, at 22:41, Kevin Bullock wrote: > >> On Jan 1, 2013, at 7:36 PM, Pierre-Yves David wrote: >> >>> # HG changeset patch >>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org> >>> # Date 1356738018 -3600 >>> # Node ID a397f42becefc44489d4d2cffb14ac1999aea375 >>> # Parent d2f15a6470d3566b722379d48da8c88b26f398fd >>> clfilter: fallback to unfiltered version when linkrev point to filtered history >>> [...] >>> diff --git a/mercurial/context.py b/mercurial/context.py >>> --- a/mercurial/context.py >>> +++ b/mercurial/context.py >>> @@ -416,11 +416,30 @@ class filectx(object): >>> if fileid is not None: >>> self._fileid = fileid >>> >>> @propertycache >>> def _changectx(self): >>> - return changectx(self._repo, self._changeid) >>> + try: >>> + return changectx(self._repo, self._changeid) >>> + except error.RepoLookupError: >>> + # Linkrev may point to any revision in the repository. When the >>> + # repository is filtered this may lead to `filectx` trying to build >>> + # `changectx` for filtered revision. In such case we fallback to >>> + # creating `changectx` on the unfiltered version of the reposition. >>> + # This fallback should not be an issue because`changectx` from >>> + # `filectx` are not used in complexe operation that care about >>> + # filtering. >>> + # >>> + # This fallback is a cheap and dirty fix that prevent several >>> + # crash. It does not ensure the behavior is correct. However the >>> + # behavior was not correct before filtering either and "incorrect >>> + # behavior" is seen as better as "crash" >>> + # >>> + # Linkrevs have several serious troubles with filtering that are >>> + # complicated to solve. Proper handling of the issue here should be >>> + # considered when solving linkrev issue are on the table. >>> + return changectx(self._repo.unfiltered(), self._changeid) >> >> Won't this blow the stack if the linkrev isn't in the changelog? This would only happen on a corrupt repo, but it seems like an unnecessarily spectacular way to fail on a bad filelog. > > No, it's an alternative call, not a recursive one. > When the second call fails re exception reach the caller as before. Ahh, right, okay. pacem in terris / ??? / ?????? / ????????? / ?? Kevin R. Bullock
On 1 Jan 2013, at 7:36 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org> > # Date 1356738018 -3600 > # Node ID a397f42becefc44489d4d2cffb14ac1999aea375 > # Parent d2f15a6470d3566b722379d48da8c88b26f398fd > clfilter: fallback to unfiltered version when linkrev point to filtered history Pushed this series to crew as 518c1403838f. pacem in terris / ??? / ?????? / ????????? / ?? Kevin R. Bullock
Patch
diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -416,11 +416,30 @@ class filectx(object): if fileid is not None: self._fileid = fileid @propertycache def _changectx(self): - return changectx(self._repo, self._changeid) + try: + return changectx(self._repo, self._changeid) + except error.RepoLookupError: + # Linkrev may point to any revision in the repository. When the + # repository is filtered this may lead to `filectx` trying to build + # `changectx` for filtered revision. In such case we fallback to + # creating `changectx` on the unfiltered version of the reposition. + # This fallback should not be an issue because`changectx` from + # `filectx` are not used in complexe operation that care about + # filtering. + # + # This fallback is a cheap and dirty fix that prevent several + # crash. It does not ensure the behavior is correct. However the + # behavior was not correct before filtering either and "incorrect + # behavior" is seen as better as "crash" + # + # Linkrevs have several serious troubles with filtering that are + # complicated to solve. Proper handling of the issue here should be + # considered when solving linkrev issue are on the table. + return changectx(self._repo.unfiltered(), self._changeid) @propertycache def _filelog(self): return self._repo.file(self._path)