Shelve bugs

[Durham Goode]

I don't think we should disable all hooks during shelve.  Some extensions
might use hooks for repo critical operations (such as my extension the
affects the filelog format) and randomly disabling them will cause random
obscure issues.

I'd rather make the broken hooks more robust to this use case.

On 10/4/13 9:38 AM, "David Soria Parra" <dsp at experimentalworks.net> wrote:

>Yes, they should be bound to transaction.close().
>
>For shelve, I would suggest to find a way to not run any hooks at all.
>We only temporary add the changegroup and/or commit. Therefore we don't
>want any hook to be run at all (no pre or post hook). E.g. think about a
>commit hook that pylints the changed files. Atm hg would happily
>executed the hook on the shelve changeset, which I think isn't a good
>idea.
>
>Opinions on that?
>
>David
>
>On 10/04/2013 03:32 AM, Durham Goode wrote:
>> Including mercurial-devel@
>> 
>> On 10/3/13 5:47 PM, "Durham Goode" <durham at fb.com> wrote:
>> 
>>> I was looking at the unshelve warning a bit more.  Seems the problem is
>>> that localrepo.addchangegroup adds its post processing hook to the lock
>>> and not the transaction.  So if the transaction closes, then another
>>>opens
>>> and edits stuff while the lock is still held, it's possible for the
>>>hook
>>> to run against a repo it wasn't expecting.  The real fix would be to
>>>hook
>>> against transaction close.  But a quick fix might look like:
>>>
>>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>>> --- a/mercurial/localrepo.py
>>> +++ b/mercurial/localrepo.py
>>> @@ -2228,10 +2228,19 @@
>>>                     # `destroyed` will repair it.
>>>                     # In other case we can safely update cache on disk.
>>>                     branchmap.updatecache(self.filtered('served'))
>>> +
>>> +                clstartnode = cl.node(clstart)
>>>                 def runhooks():
>>> +                    # These hooks run when the lock releases, not when
>>> the
>>> +                    # transaction closes. So it's possible for the
>>> changelog
>>> +                    # to have changed since we last saw it.
>>> +                    cl = self.changelog
>>> +                    if not clstartnode in cl:
>>> +                        return
>>> +
>>>                     # forcefully update the on-disk branch cache
>>>                     self.ui.debug("updating the branch cache\n")
>>> -                    self.hook("changegroup",
>>>node=hex(cl.node(clstart)),
>>> +                    self.hook("changegroup", node=hex(clstartnode),
>>>                               source=srctype, url=url)
>>>
>>>                     for n in added:
>>>
>>>
>>> On 10/3/13 2:22 AM, "David Soria Parra" <dsp at experimentalworks.net>
>>>wrote:
>>>
>>>> Hi Durham,
>>>>
>>>> thanks, I'll look into it.
>>>>
>>>> On 10/03/2013 02:01 AM, Durham Goode wrote:
>>>>>
>>>>> 2. Unshelve on a repo with hgsubversion enabled prints a warning:
>>>>> ~/www> hg unshelve --traceback
>>>>> unshelving change 'default'
>>>>> adding changesets
>>>>> adding manifests
>>>>> adding file changes
>>>>> added 1 changesets with 0 changes to 0 files
>>>>> 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>>>> error: changegroup.svn-updatemeta hook raised an exception: unknown
>>>>> revision 'b564b406c8b1809f465e511ec5333c2382328fe2'
>>>>> Traceback (most recent call last):
>>>>>   /data/users/durham/hg/hgext/shelve.py(533)unshelve()
>>>>> -> lockmod.release(lock, wlock)
>>>>>   /data/users/durham/hg/mercurial/lock.py(152)release()
>>>>> -> lock.release()
>>>>>   /data/users/durham/hg/mercurial/lock.py(147)release()
>>>>> -> callback()
>>>>>   /data/users/durham/hg/mercurial/localrepo.py(2235)runhooks()
>>>>> -> source=srctype, url=url)
>>>>>   /data/users/durham/hg/mercurial/localrepo.py(435)hook()
>>>>> -> return hook.hook(self.ui, self, name, throw, **args)
>>>>>   /data/users/durham/hg/mercurial/hook.py(198)hook()
>>>>> -> r = _pythonhook(ui, repo, name, hname, hookfn, args, throw) or r
>>>>>   /data/users/durham/hg/mercurial/hook.py(81)_pythonhook()
>>>>> -> r = obj(ui=ui, repo=repo, hooktype=name, **args)
>>>>>   /site-packages/hgsubversion/hooks/updatemeta.py(16)hook()
>>>>>   /data/users/durham/hg/mercurial/localrepo.py(400)__getitem__()
>>>>> -> return context.changectx(self, changeid)
>>>>>> /data/users/durham/hg/mercurial/context.py(302)__init__()
>>>>> _("unknown revision '%s'") % changeid)
>>>>> RepoLookupError: unknown revision
>>>>> 'b564b406c8b1809f465e511ec5333c2382328fe2'
>>>>
>>>>
>>>
>>>
>>>
>>>
>> 
>> 
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>> 
>
>