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

patch 21f9eab281c8e032b88fb12cd265db8d3ac1721b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov  8 13:57:12 CET 2020
  * use paths from D.R.Paths in D.R.Packs
  
  This also removes an obsolete debugMessage and then inlines
  fetchFilesUsingCache.

patch 32e6c9e8fe3379c9ebf6fb257a8f70de0d2f2c99
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Mar  5 07:11:08 CET 2021
  * remove some outdated or otherwise unfortunate comments
  
  The "otherwise unfortunate" refers to mentions of repository file paths;
  these will become outdated if we make changes to them.

patch cb84f86214711177eb4290719137da1934598e35
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Feb 27 07:11:18 CET 2021
  * use path definitions from D.R.Paths in a few more places

Attachments

• patch-preview.txt
• unnamed
• use-paths-from-d_r_paths-in-d_r_packs.dpatch

> use paths from D.R.Paths in D.R.Packs
>  This also removes an obsolete debugMessage and then inlines
>  fetchFilesUsingCache.

Does this inlining drop the test for whether the file already
exists? I tried digging through fetchFilesUsingCachePrivate but
couldn't spot any existing check for that, though it might
be buried in the "cache" mechanism.

Rest of the patches are fine, consider them reviewed.

>> use paths from D.R.Paths in D.R.Packs
>>  This also removes an obsolete debugMessage and then inlines
>>  fetchFilesUsingCache.
> 
> Does this inlining drop the test for whether the file already
> exists? I tried digging through fetchFilesUsingCachePrivate but
> couldn't spot any existing check for that, though it might
> be buried in the "cache" mechanism.

The latter. Without going into too much detail, fetchFileUsingCache does
its best to avoid unnecessary work; in particular, before copying or
downloading anything, it first searches for the file in all writable
locations, which always includes the "current" repo. Which means that
the test that was done here is redundant.

>>> use paths from D.R.Paths in D.R.Packs
>>>  This also removes an obsolete debugMessage and then inlines
>>>  fetchFilesUsingCache.
>>
>> Does this inlining drop the test for whether the file already
>> exists? I tried digging through fetchFilesUsingCachePrivate but
>> couldn't spot any existing check for that, though it might
>> be buried in the "cache" mechanism.
> 
> The latter. Without going into too much detail, fetchFileUsingCache does
> its best to avoid unnecessary work; in particular, before copying or
> downloading anything, it first searches for the file in all writable
> locations, which always includes the "current" repo. Which means that
> the test that was done here is redundant.

Indeed it may be harmful, since it suppresses one of the side-effects of
fetchFileUsingCache, which is that the file gets hard-linked across all
writable locations (if possible), i.e. usually between the current repo
and the per-user cache. Which means that subsequence clones may not be
able to make full use of the cache.

Thanks for the explanation (I guess ideally a bit more would have gone in the
patch description).