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

Tue May 17 13:54:21 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.

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/20160517/b5184e28/attachment.html>