This patch adds the global-cache subcommand to the optimize command.
This subcommand takes as arguments directories, search repositories 
within those directories, and delete files in the global cache 
that are not used for these repositories.

The patch still lacks some details, for example:
- Generalizing functions such as: getRecursiveContents, 
getRecursiveDarcsRepos in the module Commands.Optimize, 
and getRecursiveContentsFullPath in the module Util.File.
- What should happen if I run the command without directories as arguments.
- Help string.

But I send it so you can start reviewing it.

1 patch for repository http://darcs.net/screened:

Wed Jul 16 09:04:20 ART 2014  Marcio Diaz <marcio.diaz@gmail.com>
  * Garbage collection of global cache.

Attachments

• garbage-collection-of-global-cache_.dpatch
• patch-preview.txt
• unnamed

Some partial comments:

> hunk ./src/Darcs/Repository/HashedRepo.hs 670
>  -- with inventory" hashes. This function does not attempt to download
missing
>  -- inventory files.
> -listPatchesLocal :: IO [String]
> -listPatchesLocal = do
> -    inventory <- readInventoryPrivate darcsdir hashedInventory
> +listPatchesLocal :: String -> IO [String]
> +listPatchesLocal darcsDir = do
> +    inventory <- readInventoryPrivate darcsDir hashedInventory
>      followStartingWiths (fst inventory) (getPatches inventory)
>      where
>          followStartingWiths Nothing patches = return patches

How does this interact with lazy repositories, as we discussed last week?

I think we also discussed an approach of just GCing the user
repositories first, then GCing the global cache. Which algorithm is
being used in this patch?

> hunk ./src/Darcs/Repository/HashedRepo.hs 658
>      if b then readInventoryPrivate dir invName
>          else return (Nothing, [])
>
> -
>  -- |listInventoriesLocal this function does not attempt to retrieve
missing
>  -- inventory files.
>  listInventoriesLocal :: IO [String]

Please amend out this unnecessary whitespace change (adds noise, risk of
conflicts with other changes).

> +-- | getRecursiveDarcsRepos returns all directories under topdir that
have
> +-- a child directory named darcsdir ("_darcs").
> +getRecursiveDarcsRepos :: FilePath -> IO [FilePath]
> +getRecursiveDarcsRepos topdir = do
> +  names <- getDirectoryContents topdir
> +  let properNames = filter (`notElem` [".", ".."]) names
> +  paths <- forM properNames $ \name -> do
> +    let path = topdir </> name
> +    isDir <- doesDirectoryExist path
> +    if isDir && (name /= darcsdir)
> +      then getRecursiveDarcsRepos path
> +      else if name == darcsdir

This logic feels a bit convoluted. Wouldn't it lead to a file named
"_darcs" being recognised? I think the outer if should check 'isDir',
and the inner if check 'name == darcsdir'

On Wed, Jul 16, 2014 at 8:13 PM, Ganesh Sittampalam <bugs@darcs.net> wrote:

>
> Ganesh Sittampalam <ganesh@earth.li> added the comment:
>
> Some partial comments:
>
> > hunk ./src/Darcs/Repository/HashedRepo.hs 670
> >  -- with inventory" hashes. This function does not attempt to download
> missing
> >  -- inventory files.
> > -listPatchesLocal :: IO [String]
> > -listPatchesLocal = do
> > -    inventory <- readInventoryPrivate darcsdir hashedInventory
> > +listPatchesLocal :: String -> IO [String]
> > +listPatchesLocal darcsDir = do
> > +    inventory <- readInventoryPrivate darcsDir hashedInventory
> >      followStartingWiths (fst inventory) (getPatches inventory)
> >      where
> >          followStartingWiths Nothing patches = return patches
>
> How does this interact with lazy repositories, as we discussed last week?
>

It doesn't interact with lazy repositories. I just realized that we can only
read the hashed_inventory file from the repository and then read the other
inventories from the global cache.


> I think we also discussed an approach of just GCing the user
> repositories first, then GCing the global cache. Which algorithm is
> being used in this patch?
>

This patch doesn't garbage collect the repositories. I don't like the idea
of ​​doing two things at the same time, and there is a command that does
that.


> > hunk ./src/Darcs/Repository/HashedRepo.hs 658
> >      if b then readInventoryPrivate dir invName
> >          else return (Nothing, [])
> >
> > -
> >  -- |listInventoriesLocal this function does not attempt to retrieve
> missing
> >  -- inventory files.
> >  listInventoriesLocal :: IO [String]
>
> Please amend out this unnecessary whitespace change (adds noise, risk of
> conflicts with other changes).
>
>
Ok.


> > +-- | getRecursiveDarcsRepos returns all directories under topdir that
> have
> > +-- a child directory named darcsdir ("_darcs").
> > +getRecursiveDarcsRepos :: FilePath -> IO [FilePath]
> > +getRecursiveDarcsRepos topdir = do
> > +  names <- getDirectoryContents topdir
> > +  let properNames = filter (`notElem` [".", ".."]) names
> > +  paths <- forM properNames $ \name -> do
> > +    let path = topdir </> name
> > +    isDir <- doesDirectoryExist path
> > +    if isDir && (name /= darcsdir)
> > +      then getRecursiveDarcsRepos path
> > +      else if name == darcsdir
>
> This logic feels a bit convoluted. Wouldn't it lead to a file named
> "_darcs" being recognised? I think the outer if should check 'isDir',
> and the inner if check 'name == darcsdir'
>
>
The idea is to recognize the parents of "_darcs" directories.


> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/patch1180>
> __________________________________
>

Attachments

• unnamed

Now the patch interact with lazy repos, it only read
the hashed_inventory from the repos. Other inventories, 
patches and pristine files are read from the global cache.

Also, I fixed the convoluted logic of the function getRecursiveDarcsRepos.

1 patch for repository http://darcs.net/screened:

Fri Jul 18 12:26:52 ART 2014  Marcio Diaz <marcio.diaz@gmail.com>
  * Garbage collection of global cache.

Attachments

• garbage-collection-of-global-cache_.dpatch
• patch-preview.txt
• unnamed

Looks good - what's left to do (if anything)?

Changes from last patch:
- Commented several functions in the Repository.HashedRepo module.
- New description of the optimize global-cache command (added optimizeHelpGlobalCache variable).
- Moved getRecursiveDarcsRepos from UI.Commands.Optimize to Repository.Util.
- Moved getRecursiveContents from UI.Commands.Optimize to Util.File.

1 patch for repository http://darcs.net/screened:

Wed Jul 23 10:39:21 ART 2014  Marcio Diaz <marcio.diaz@gmail.com>
  * Garbage collection of global cache.

Attachments

• garbage-collection-of-global-cache_.dpatch
• patch-preview.txt
• unnamed

Changes:
- Without arguments returns a message saying that the command does nothing.

Ganesh: I think there is nothing more to do, except maybe a test.
1 patch for repository http://darcs.net/screened:

Wed Jul 23 11:07:00 ART 2014  Marcio Diaz <marcio.diaz@gmail.com>
  * Garbage collection of global cache.

Attachments

• garbage-collection-of-global-cache_.dpatch
• patch-preview.txt
• unnamed

Fixed some typo:
- Changed "hashed_file" for "hashed_inventory" 
in the documentation of some functions. 
1 patch for repository http://darcs.net/screened:

Wed Jul 23 14:30:05 ART 2014  Marcio Diaz <marcio.diaz@gmail.com>
  * Garbage collection of global cache.

Attachments

• garbage-collection-of-global-cache_.dpatch
• patch-preview.txt
• unnamed

I'm pushing this to screened now so please send updates as followup 
patches.

I tried it and it failed with:

$ dist/build/darcs/darcs optimize global-cache ../..
darcs.exe: ../../temp-rebase-extra/_darcs/hashed_inventory: 
openBinaryFile: does not exist (No such file or directory)

I think it's because I have a corrupt repo in my home directory, but the 
code should be tolerant to that. Could you fix that?

Changes:
- Function maybeIdentifyRepository is now used to check the repositories.
- Directories inventories, patches and pristine.hashed are created if don't exist.

1 patch for repository http://darcs.net/screened:

patch 3590f61fba6302ee71d8aec9951b493d5b689c7c
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date:   Tue Jul 29 07:45:47 ART 2014
  * Fixed file does not exist error when using darcs optimize global-cache.

Attachments

• fixed-file-does-not-exist-error-when-using-darcs-optimize-global_cache_.dpatch
• patch-preview.txt
• unnamed

Changes:
- I made several changes to avoid unnecessary recursions of the function getRecursiveDarcsRepos.
Now the variable repoPaths only contains different paths.

1 patch for repository http://darcs.net/screened:

patch 3a4431c09ef354dbbe2a140659299648f291ce13
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date:   Tue Jul 29 09:17:24 ART 2014
  * Fixed file does not exist error when using darcs optimize global-cache.

Attachments

• fixed-file-does-not-exist-error-when-using-darcs-optimize-global_cache_.dpatch
• patch-preview.txt
• unnamed

I've screened this last patch.

I'd like to see both subcommands:

  cache                optimize global cache for access through shell
commands like ls/rm
  global-cache         garbage collect global cache


merged into one. I think we can stay with the name "cache", and just
describe it as "garbage collect global cache".  The transition from
non-bucketed to bucketed is something that happens once, so I don't feel
it deserves a separate subcommand. If someone thinks of particular cases
where for performance or testing sake we want to keep them separated,
shout asap.

Also I think users will expect the command to work by default by taking
their home directory.

I mean, without passing any argument, it should print something like
"Will clean global cache according to repositories present in $HOME (<-
expand this)", and proceed.

Marcio, yell here if you are still working on this. I may be writing
the followup patches myself soon.

Here's a first attempt at a followup. I'm not screening it
yet in case it lacks something.

1 patch for repository http://darcs.net:

patch 8ed115f087e9356a6dd9c5768341ff76d5bc97d3
Author: Guillaume Hoffmann <guillaumh@gmail.com>
Date:   Mon Oct 13 20:23:22 ART 2014
  * followup work on garbage collecting of global cache
  * do bucketting and garbage collection in single command
    'optimize cache'
  * no longer recurse in dot-directories when looking for
    repotories
  * no longer consider OF repositories
  * when no directory given look into home

Attachments

• followup-work-on-garbage-collecting-of-global-cache.dpatch
• patch-preview.txt
• unnamed

Screening this one.

1 patch for repository http://darcs.net:

patch c3f56af0d782e8ce1d9a32e303ac32e36004f25a
Author: Guillaume Hoffmann <guillaumh@gmail.com>
Date:   Tue Oct 14 09:18:32 ART 2014
  * followup work on garbage collecting of global cache
  * do bucketting and garbage collection in single command
    'optimize cache'
  * no longer recurse in dot-directories when looking for
    repotories
  * no longer consider OF repositories
  * when no directory given look into home

Attachments

• followup-work-on-garbage-collecting-of-global-cache.dpatch
• patch-preview.txt
• unnamed

I'm accepting the whole bundle with this help string patch.

1 patch for repository http://darcs.net:

patch 2df506c413629e43e44f62af9df42267bd3becd5
Author: Guillaume Hoffmann <guillaumh@gmail.com>
Date:   Tue Oct 21 11:13:36 ART 2014
  * optimize cache help string

Attachments

• optimize-cache-help-string.dpatch
• patch-preview.txt
• unnamed