[darcs-users] [patch88] patch for -- Re: versioned show files

Thomas Hartman thomashartman1 at googlemail.com
Wed Nov 25 22:51:22 UTC 2009


Okay, a patch is attached.

For now the only change I have made from your suggestions is to make
--pending the default if not in a versioned repo, and --no-pending the
default if versioned, otherwise complain.

darcs show files --pending | grep testp # goo
./testpending

darcs show files --pending -p 'replace commutex with commute in
elegant_merge' # fine
darcs: Can't mix pending option (default) with revisioned show files;

Some replies to your other suggestions.

>>> The logic is good: create a snapshot of the repository at a given >>
> patch, then slurp at that date. Why not use virtualTreeIO like in
>>> showContents? I think this would be more performant than slurping
>>> (Petr, can you comment on that?)

I chatted with petr on darcs and he confirmed that darcs is
transitioning from Slurpy to Storage.Hashed, however he was
pessimistic that we would see a significant performance improvement.

<mornfall> patch-tag: Slurpies are being phased out -- hashed-storage
provides better mechanisms to
achieve similar ends.                   [10:51]
<mornfall> patch-tag: Nevertheless, versioned show files is going to be slow
with current repository format, and I don't see that properly
addressed anytime soon. A year or two at least, I'm afraid. Lots of
work needs to go into a new, robust and performant format...

I did look into replacing the slurpy logic in Storage.Hashed with
something similar to what we have in Command.ShowContents, which uses
hash storage. However, I couldn't figure out how to distinguish
between files and directories when doing a dump from Tree IO, which is
necessary for show files
(--files,--no-files,--directories,--no-directories).

If someone can point me in the right direction with regards to this I
would be interested in helping to migrate everything to Hashed
Storage, if that helps.

The current versioned show files using slurpies is very slow for old
revisions, but relatively snappy for recent revisions, which I think
is a much more common use, and probably the best we can do for now.

thartman at ubuntu:~/darcs.net>time darcs show files -p 'Create patch
sequence type' | head -n3 # very slow, becaus this is a way old patch
./Add.lhs
real	0m22.289s
thartman at ubuntu:~/darcs.net>time darcs show files -p 'replace commutex
with commute in elegant_merge' | head -n3 # quite snappy on a more
recent revision
real	0m0.347s

>>> The argument-handling logic could be externalized to the Arguments
> and Flags modules.

Not sure if I understand you here. I did get the feeling looking at
this code that something seems off with the way repos/patchsets are
selected from the arguments passed in. And this bugs me now just in
Showfiles but also ShowIndex, ShowContents, and probably other places.

Seems like the way this should work is there is a function

data RepoThingie = PendingRepo | RecordedRepo | PointInTimeRepo Patch
getRepo :: Monad m => [DarcsFlag] -> m RepoThingie

then you could handle the logic for what kind of repo/patchset we will
be operating in directly from the DarcsFlags, rather than mixed with
other bits of code. I think this would make things a lot more
understandable. Monadic because would throw error if can't match darcs
args to a particular repo.

Currently we are almost there with

Darcs.Match.nonrange_matcher :: Patchy p => [DarcsFlag] -> Maybe (Matcher p)

but this misses the case for --index=n (a single patch a few
"versions" back) and I don't see how to fix this. Basicaly I don't
know how to go from index n to the right patch. Incidentally, this is
also the root of broken darcs show contents --index=1 which I
submitted a bug on yesteday.

http://bugs.darcs.net/issue1705

Cheers, and happy haciking.

thomas.

On Thu, Nov 19, 2009 at 2:00 AM, Florent Becker <bugs at darcs.net> wrote:
>
> Florent Becker <florent.becker at univ-orleans.fr> added the comment:
>
> Sun Nov 15 17:32:06 PST 2009  thomashartman1 at gmail.com
>  * add versioned show files functionality (darcs show files -p 'some
> patch')
>
> Thomas, can you post a follow-up patch to make --no-pending the default
> when invoked with -p? An option that does not work by default is not
> very convenient.
>
> The rest is ok.
>
> New patches:
>
> [add versioned show files functionality (darcs show files -p 'some
> patch')
>>> This is needed for convenience and flagset consistency.
>
>  manifest_cmd :: ([DarcsFlag] -> Slurpy -> [FilePath]) -> [DarcsFlag] ->
> [String] -> IO ()
>  manifest_cmd to_list opts _ = do
> -    list <- (to_list opts) `fmap` withRepository opts slurp
> +    list <- (to_list opts) `fmap` withRepository opts myslurp
>     mapM_ output list
> hunk ./src/Darcs/Commands/ShowFiles.lhs 97
> -    where slurp :: RepoPatch p => Repository p C(r u r) -> IO Slurpy
> -          slurp = if NoPending `notElem` opts
> -                  then slurp_pending else slurp_recorded
> +    where myslurp :: RepoPatch p => Repository p C(r u r) -> IO Slurpy
> +          myslurp = do let isPending = NoPending `notElem` opts
> +                           isRevisioned = have_nonrange_match opts
> +                       case (isPending, isRevisioned) of
> +                          (True,False) -> slurp_pending
> +                          (False,True) -> slurp_revision opts
> +                          (False,False) -> slurp_recorded
> +                          (True,True) -> error $ unlines ["Can't mix
> pending option (default) with revisioned show files; ",
> +                                                          "perhaps you
> want darcs <your cmd> --no-pending ?"]
>           output_null name = do { putStr name ; putChar '\0' }
>           output = if NullFlag `elem` opts then output_null else
> putStrLn
>>> The argument-handling logic could be externalized to the Arguments
> and Flags modules. Either way, -p should imply --no-pending with that
> command (and with all commands since it is only ever used here).
>
> hunk ./src/Darcs/Commands/ShowFiles.lhs 108
> +
> +slurp_revision :: RepoPatch p => [DarcsFlag] -> Repository p C(r u r)
> -> IO Slurpy
> +slurp_revision opts r = withDelayedDir "revisioned.showfiles" $ \_ ->
> do
> +  get_nonrange_match r opts
> +  slurp =<< getCurrentDirectory
> +
>>> The logic is good: create a snapshot of the repository at a given >>
> patch, then slurp at that date. Why not use virtualTreeIO like in
>>> showContents? I think this would be more performant than slurping
>>> (Petr, can you comment on that?)
>
> ----------
> nosy: +florent.becker
> status: needs-review -> amend-requested
> title: patch for -- Re:  versioned show files -> patch for -- Re: versioned show files
>
> __________________________________
> Darcs bug tracker <bugs at darcs.net>
> <http://bugs.darcs.net/patch88>
> __________________________________
>



-- 
Need somewhere to put your code? http://patch-tag.com
Want to build a webapp? http://happstack.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-versioned-show-files-functionality-_darcs-show-files-_p-_some-patch__.dpatch
Type: application/octet-stream
Size: 66800 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20091125/ecada0b1/attachment-0001.obj>


More information about the darcs-users mailing list