darcs

Patch 2192 use paths from D.R.Paths in D.R.Packs (and 2 more)

Title use paths from D.R.Paths in D.R.Packs (and 2 more)
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2021-06-05.13:41:14 by bfrk, last changed 2021-06-08.05:26:02 by ganesh.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt bfrk, 2021-06-05.13:41:14 text/x-darcs-patch
unnamed bfrk, 2021-06-05.13:41:14 text/plain
use-paths-from-d_r_paths-in-d_r_packs.dpatch bfrk, 2021-06-05.13:41:14 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg22863 (view) Author: bfrk Date: 2021-06-05.13:41:14
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
msg22864 (view) Author: ganesh Date: 2021-06-05.13:55:19
> 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.
msg22865 (view) Author: bfrk Date: 2021-06-05.14:31:58
>> 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.
msg22866 (view) Author: bfrk Date: 2021-06-05.16:09:55
>>> 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.
msg22867 (view) Author: ganesh Date: 2021-06-06.19:14:22
Thanks for the explanation (I guess ideally a bit more would have gone in the
patch description).
History
Date User Action Args
2021-06-05 13:41:14bfrkcreate
2021-06-05 13:55:19ganeshsetmessages: + msg22864
2021-06-05 14:31:59bfrksetmessages: + msg22865
2021-06-05 16:09:55bfrksetmessages: + msg22866
2021-06-05 16:11:30bfrksetstatus: needs-screening -> review-in-progress
2021-06-06 19:14:22ganeshsetstatus: review-in-progress -> accepted-pending-tests
messages: + msg22867
2021-06-08 05:26:02ganeshsetstatus: accepted-pending-tests -> accepted