Regard this updated bundle as work in progress.

It fixes the build problem. But then I noticed that the log action creates
./_darcs/patches and fills it with the last 300 patches (with the test case
as is; if you leave off the --last=300 it's probably around 12000).

This looks wrong to me. It's fine to use (and fill) the cache (if one is
available), but why create a _darcs/patches directory and fill that? I doubt
this is correct even if ./_darcs happens to exist because we don't want to
clutter our local _darcs/patches directory with files that have nothing to
do with the current repo.

2 patches for repository http://darcs.net/screened:

patch 562750e4c0bb9aecbb8526a03cf17d9de535286c
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 10 20:13:27 CET 2015
  * fixed withRepoLockCanFail by allowing more things to fail
  
  It previously accepted failure when the current repo i.e. "." cannot be
  locked. Now it works even if there is no repo at all or if there is
  a problem with reading from or writing to it.
  
  This fixes a regression, see tests/network/log.sh.

patch 58a5cf81ed9f7b55ac7e779387f53e0ea73177e2
Author: Ben Franksen <benjamin.franksen@helmholtz-berlin.de>
Date:   Tue Mar 10 20:17:38 CET 2015
  * cleanup in Darcs.Repository.Job: indent, haddocks

Attachments

• fixed-withrepolockcanfail-by-allowing-more-things-to-fail.dpatch
• patch-preview.txt
• unnamed

Try this:

> (cd / && darcs log --repo=http://darcs.net --last=3 --debug)

What you get back is the slightly schizophrenic:

Beginning identifying repository http://darcs.net
Done identifying repository http://darcs.net

darcs failed:  Not a repository: http://darcs.net (.:
openBinaryTempFile: permission denied (Permission denied))

HINT: Do you have the right URI for the repository?

This is not acceptable. Either we fix this in 2.10 or we disable the
--repo option for the log command. (Or limit it to local repos, but IIUC
that would be --redodir then.)

I think there's actually a wider problem here. The idea of the withRepoLockCanFail 
call is to make a patch index that will speed up the subsequent log operation. So if 
log is being called on a different repository to the one in the current directory, 
then we're making a patch index in completely the wrong place!

As far as this patch goes, I'm also instinctively nervous about making code too 
tolerant of errors - similar to the catchall problems across the codebase, it can 
end up hiding genuine problems. In this case I would argue that "CanFail" aspect of 
the name should refer to the lock, not the entire call.

So I think it might be better to change log to check the supplied repo and not do 
anything if it's remote. On the other hand if it's a choice between this patch and 
nothing then we should definitely apply it and fix the regression.

I think you are right: a patch index should only be created inside the
repo for which we want to do the log. This would indeed mean to

(a) not even try to create an index if we are doing it for a remote repo
(b) do try to create it if the repo is local
(c) if we are in a repo but are being called with --repo=/somewhere/else
we must completely ignore the current repo

However:

What I discovered is that we have *yet another* problem, namely with
caching. What I observed in msg18295 is *not* caused by Darcs trying to
create an index. You can completely comment out the lines that do that
and still get the same error. It is rather because of the implementation
of log on remote repos: it downloads the patches into the local cache.

A precision: darcs will not attempt to create patch index just because
"log" is ran, but only when "log" is ran with a file argument.

Also the bug happens with darcs 2.8 (cd / && darcs log
--repo=http://darcs.net --last=3 --debug).

Patch1382 solves all these problems at the root and more robustly.