See the issue tracker for explanation.

Yours,
   Petr.

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

Sun Aug  8 14:24:19 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1908: try to create a global cache before checking its availability.

Attachments

• resolve-issue1908_-try-to-create-a-global-cache-before-checking-its-availability_.dpatch
• unnamed

On Sun, Aug 08, 2010 at 12:22:34 +0000, Petr Ročkai wrote:
> Sun Aug  8 14:24:19 CEST 2010  Petr Rockai <me@mornfall.net>
>   * Resolve issue1908: try to create a global cache before checking its availability.

What about users that don't want a global cache?

Looks like our user documentation is lacking: How do users disable the
global cache if they don't want it?  I'm guessing they would have to
make sure ~/.darcs/sources is empty (well, that it excludes the global
cache entry).

If so, would this mechanism support this?  I'm guessing it does because
we only attempt to create the cache if it's in the sources.

Also, does createCache deal correctly with cache entries that are explicitly
marked readonly (in darcs sources, not in the filesystem mind you)?  Or is
that such a corner case that we shouldn't worry about it.

-                                 do
-                                   checkCacheReachability (show e) c
-                                   (fname,x) <- filterBadSources cs >>= ffuc
-                                   do createCache c subdir
-                                      createLink fname (fn c)
-                                      return (fn c, x)
-                                    `catchall`
+                                 do createCache c subdir `catchall` return () -- don't worry
+                                    checkCacheReachability (show e) c
+                                    (fname,x) <- filterBadSources cs >>= ffuc
+                                    do createLink fname (fn c)
+                                       return (fn c, x)
+                                     `catchall`

This looks like it just added the createCache, which seems fine if my questions
above are fine.

Same request to Adolfo: please produce minimal patches to ensure fast review.

Also, the comment could probably say something more useful than "don't worry"
perhaps explaining why we try to create the cache (it may be missing) and why
it's no big deal if it fails

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.

Eric Kow <kowey@darcs.net> writes:
> What about users that don't want a global cache?
I'd suggest adding ALL no-cache to ~/.darcs/defaults to that effect. We
should update the docs/faq/whatever we have, I guess.

Yours,
   Petr.

Eric Kow <kowey@darcs.net> writes:
> Also, does createCache deal correctly with cache entries that are explicitly
> marked readonly (in darcs sources, not in the filesystem mind you)?  Or is
> that such a corner case that we shouldn't worry about it.

This whole block is guarded with "| writable", so I wouldn't worry about
that.

>
> -                                 do
> -                                   checkCacheReachability (show e) c
> -                                   (fname,x) <- filterBadSources cs >>= ffuc
> -                                   do createCache c subdir
> -                                      createLink fname (fn c)
> -                                      return (fn c, x)
> -                                    `catchall`
> +                                 do createCache c subdir `catchall` return () -- don't worry
> +                                    checkCacheReachability (show e) c
> +                                    (fname,x) <- filterBadSources cs >>= ffuc
> +                                    do createLink fname (fn c)
> +                                       return (fn c, x)
> +                                     `catchall`
>
> This looks like it just added the createCache, which seems fine if my questions
> above are fine.
>
> Same request to Adolfo: please produce minimal patches to ensure fast review.
I actually shuffled the createCache out of the inner block. Not sure the
patch could be made any smaller.

> Also, the comment could probably say something more useful than "don't worry"
> perhaps explaining why we try to create the cache (it may be missing) and why
> it's no big deal if it fails
Oh. I didn't intend to actually add that comment :D. The idea is that we
check whether this worked on the next line.

Yours,
   Petr.

On Sun, Aug 08, 2010 at 15:20:02 +0200, Petr Rockai wrote:
> > Also, does createCache deal correctly with cache entries that are explicitly
> > marked readonly (in darcs sources, not in the filesystem mind you)?  Or is
> > that such a corner case that we shouldn't worry about it.
> 
> This whole block is guarded with "| writable", so I wouldn't worry about
> that.

...

> > Same request to Adolfo: please produce minimal patches to ensure fast review.
> I actually shuffled the createCache out of the inner block. Not sure the
> patch could be made any smaller.

Thanks for the clarification.  This would have been more evident to
me if I had actually taken the time to view your patch in a graphical
diff tool, tsk.

For now my workflow is

  darcs apply -i /tmp/foo.dpatch
  darcs-gdiff --last=1

with

  alias darcs-gdiff='darcs diff --diff-command='\''meld %1 %2'\'''

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-1789ccebb70bfa0af2f3b3e0e7ba1ae1e9257e80

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-1789ccebb70bfa0af2f3b3e0e7ba1ae1e9257e80