[PATCH 01 of 17 V2] largefiles: rename lfutil to storeutil

Martin von Zweigbergk martinvonz at google.com
Tue May 17 13:31:00 EDT 2016


How about moving _openstore into a new module? I tried it to make sure
it worked, but I don't know what you think about it design-wise. Feel
free to pull from
https://www.mercurial-scm.org/repo/users/martinvonz/mercurial/rev/8444e3709b0f.
Tests pass.

On Mon, May 16, 2016 at 6:43 PM, Matt Harbison <mharbison72 at gmail.com> wrote:
> On Sun, 15 May 2016 16:52:40 -0400, Piotr Listkiewicz
> <piotr.listkiewicz at gmail.com> wrote:
>
>>>
>>> You need diff.git = True in your config to get the rename properly
>>> exported here.
>>
>>
>>
>> You are right, thanks for a tip
>>
>>
>>> I'm wondering about this rename in general.  There seem to be utility
>>> functions unrelated to the store in the current lfutil, which get swept
>>> up
>>> in the rename- standin related things, matchers and so forth.  And then
>>> lfutil gets reintroduced in the next patch.  Would it be better to copy
>>> lfutil.py to storeutil.py, and then delete the things in each that belong
>>> to the other?  This way, you don't have to move the stuff that isn't
>>> store
>>> related back.
>>
>>
>>
>>
>>> Could the whole issue be avoided by simply moving basestore stuff from
>>> patch #2 into lfutil?  (Even though it isn't really a utility.)
>>
>>
>>
>> Simple moving _openstore from basestore to lfutil doesn't work, because
>> lfutil is used by localstore and remotestore. For now i don't see other
>> solution than extracting other module from lfutil or introducing new
>> module.
>>
>> lfutil keeps functions that in general belongs to the following groups:
>> 1. moving file from/to store/cache (store_utils)
>>     * for example storepath, findstorepath, copyfromcache
>> 2. standins utilities (stdin_utils)
>>     * for example splitstandin, updatestandinsbymatch,
>> 3. dirstate utilities (dirstate_utils)
>>     * for example openlfdirstate, largefilesdirstate
>> 4. network utilities
>>     * for example storeprotonotcapable, httpsendfile
>> 5. Others that should stay in lfutil
>>     * for example automatedcommithook, getstatuswriter
>>
>> Three first utilities groups has dependencies like this:
>>
>> store_utils <--uses-- standins_utils <--uses-- dirstate_utils
>>
>> Store classes uses following groups from lfutil:
>> - basestore uses store utilities and standins utilities(not counting
>> things
>> needed for _openstore)
>> - remotestore uses network utilities and store utilities
>> - wirestore uses network utilities
>>
>> Network utilities can be moved from lfutil to remotestore. After this
>> *store classes use store utilities and standins utilities.
>>
>> If we move store utilities and standins utilities from lfutil to the other
>> module which won't have any dependency on the other module from
>> largefiles,
>> then we should be able to move _openstore to lfutil without any cycle
>> imoprt.
>>
>> The options are following:
>> 1) Moving store utils and standin utils to separate module
>> 2) Moving store utils, standin utils and dirstate utils to separate
>> modules
>>
>> I have no idea which solution is better and how the new module should be
>> named. Any of those solution makes lfutil significantly smaller but if we
>> move store utils, standin utils and dirstate utils the difference between
>> this patch that renames lfutil and a new solution moving things from
>> lfutil
>> is pretty small.
>>
>> I need a guidance what should i do.
>
>
> I'm not sure what to do either, and the little bit of playing with it made
> me realize that the cycle stuff is more complicated than I assumed.  It
> sounds like #1 may be less code churn, but still sweeps up standin utils
> into a non-obvious module.  But since these are rather cosmetic concerns and
> I don't have any better ideas and names, if nobody else has any ideas, it
> may be the best we can do.
>
>
>
>>
>> 2016-05-13 3:32 GMT+02:00 Matt Harbison <mharbison72 at gmail.com>:
>>
>>> On Thu, 12 May 2016 06:20:23 -0400, liscju <piotr.listkiewicz at gmail.com>
>>> wrote:
>>>
>>> # HG changeset patch
>>>>
>>>> # User liscju <piotr.listkiewicz at gmail.com>
>>>> # Date 1462488595 -7200
>>>> #      Fri May 06 00:49:55 2016 +0200
>>>> # Node ID 6606554248eaeffc7dc43b62f3b9d8128b86b9b6
>>>> # Parent  c641b8dfb98c2ade6995ba3aa341fe4d7b154827
>>>> largefiles: rename lfutil to storeutil
>>>>
>>>> lfutil should be used as higher level module by lfcommands and
>>>> overrides. storeutil should have utilities to deal with store
>>>> and be lower level module. This separation will be helpful in
>>>> resolving cycle dependencies.
>>>>
>>>>
>>> diff -r c641b8dfb98c -r 6606554248ea hgext/largefiles/storeutil.py
>>>>
>>>> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
>>>> +++ b/hgext/largefiles/storeutil.py     Fri May 06 00:49:55 2016 +0200
>>>>
>>>
>>> You need diff.git = True in your config to get the rename properly
>>> exported here.
>>>
>>> I'm wondering about this rename in general.  There seem to be utility
>>> functions unrelated to the store in the current lfutil, which get swept
>>> up
>>> in the rename- standin related things, matchers and so forth.  And then
>>> lfutil gets reintroduced in the next patch.  Would it be better to copy
>>> lfutil.py to storeutil.py, and then delete the things in each that belong
>>> to the other?  This way, you don't have to move the stuff that isn't
>>> store
>>> related back.
>>>
>>> Possibly part of the issue is when I see a reference to "store" in the
>>> name, I think of the .hg/largefiles directory, and/or the global user
>>> cache.  Aside from the cycle checker yelling, it isn't clear to me where
>>> I
>>> should add a new utility method.  Could the whole issue be avoided by
>>> simply moving basestore stuff from patch #2 into lfutil?  (Even though it
>>> isn't really a utility.)  Or maybe just move the handful of path related
>>> methods now in lfutil to storeutil?
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list