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

Fri May 20 04:12:57 EDT 2016

>
> Certainly, the reduced code churn of this solution seems to outweigh any
> slightly misfit name of this new module IMO.


So I guess its best what can be done now, i sent V3 based on Martin commit

2016-05-19 4:43 GMT+02:00 Matt Harbison <mharbison72 at gmail.com>:

> On Wed, 18 May 2016 09:41:22 -0400, Piotr Listkiewicz <
> piotr.listkiewicz at gmail.com> wrote:
>
>
>>> "store" seemed like the obvious choice given basestore, localstore,
>>> remotestore, wirestore. Perhaps you're saying those should also be
>>> renamed?
>>> Either way, feel free to pull down my change and modify it as you like
>>> and
>>> put your name on it.
>>>
>>
>>
>> They have fine names, problem is about new module name, IMHO abstraction
>> behind module named "store" should be more general than just keeping store
>> factory. Matt, Mads what do you think?
>>
>
> What else do you envision living in that module?  'storemod.openstore()'
> or even 'storemod.open()' reads fine to me, but IDK what other stuff you
> might put in there.  Certainly, the reduced code churn of this solution
> seems to outweigh any slightly misfit name of this new module IMO.
>
>
> 2016-05-18 15:21 GMT+02:00 Martin von Zweigbergk <martinvonz at google.com>:
>>
>> "store" seemed like the obvious choice given basestore, localstore,
>>> remotestore, wirestore. Perhaps you're saying those should also be
>>> renamed?
>>> Either way, feel free to pull down my change and modify it as you like
>>> and
>>> put your name on it.
>>>
>>> On Tue, May 17, 2016, 10:54 Piotr Listkiewicz <
>>> piotr.listkiewicz at gmail.com>
>>> wrote:
>>>
>>> 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.
>>>>>
>>>>
>>>>
>>>> I think thats good idea because there is no clear how to divide lfutil
>>>> to
>>>> overcome import cycle issue. However from design point of view we think
>>>> about different name than "store" because in largefiles it has very
>>>> specific meaning
>>>>
>>>> 2016-05-17 19:31 GMT+02:00 Martin von Zweigbergk <martinvonz at google.com
>>>> >:
>>>>
>>>> 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
>>>>>
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160520/0225735d/attachment.html>