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

Martin von Zweigbergk martinvonz at google.com
Wed May 18 09:21:57 EDT 2016


"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/20160518/10af38c3/attachment.html>


More information about the Mercurial-devel mailing list