|
Created on 2023-06-17.12:18:20 by bfrk, last changed 2023-07-29.10:49:04 by ganesh.
See mailing list archives
for discussion on individual patches.
msg23330 (view) |
Author: bfrk |
Date: 2023-06-17.12:18:08 |
|
I regard these as internal refactors. Dropping support for --[un-]compress
(where it concerns hashed files) strictly speaking affects the UI but since
these options are "technical" I regard them as internal. Similarly, whether
or not to display certain messages and in what way is a user-visible change,
but it does not touch what darcs does to you repo or the working copy.
10 patches for repository http://darcs.net/screened:
patch 12b07218135b311f4fa0c5cc4319973dfbaa1020
Author: Ben Franksen <ben.franksen@online.de>
Date: Sat Mar 27 08:09:13 CET 2021
* improve the interface of D.R.Inventory
This combines the internal functions readInventoryPrivate and
readPatchesFromInventoryEntries into readOneInventory, for use by
upgradeOldStyleRebase.
patch 84765e19bee46b2ac1c6393d3125810f1e2f816a
Author: Ben Franksen <ben.franksen@online.de>
Date: Sun Nov 27 13:11:14 CET 2022
* add Darcs.Repository.Transaction
This is to avoid import cycles when adding functionality to new modules
under Darcs.Repository, since this usually requires that we add things to
revertRepositoryChanges and finalizeRepositoryChanges.
patch 6cd64873eaf0bb962ee6622c3a5f903850001584
Author: Ben Franksen <ben.franksen@online.de>
Date: Sat Nov 26 01:35:40 CET 2022
* remove no longer needed export of diffHashLists
patch a1e63cc27cd56ba6c8f7a2043a5483b7f5de8c72
Author: Ben Franksen <ben.franksen@online.de>
Date: Tue Nov 29 01:25:28 CET 2022
* cleanup cleanPristineDir and move from D.R.Pristine to D.R.Traverse
patch 5ed3cca90043c212cc56f44fd89b9e07a218cf01
Author: Ben Franksen <ben.franksen@online.de>
Date: Mon Jun 27 15:02:09 CEST 2022
* repair: do not clean pristine dir
I found that it can be quite useful that darcs never does any cleaning
automatically (except for convert export where it actually makes sense),
since this allows to restore things that would otherwise be completely lost
e.g. after an erroneous obliterate.
patch 0f79880de0e328eaa79db700d62fcd1b798a9d8c
Author: Ben Franksen <ben.franksen@online.de>
Date: Sun Nov 27 19:56:06 CET 2022
* always store hashed files in compressed form
This removes the --[un-]compress option from most commands. The only
remaining command that supports it is `push` since there it is used to
control whether the bundle is sent in compressed or uncompressed from.
Sending uncompressed is needed for compatibility with older (pre 2.5) remote
darcs versions. The `optimize uncompress` subcommand is still available, as
an uncompressed repository may be nice to have for debugging. Note that we
are still able to detect and read uncompressed hashed files.
Rationale: This removes an unnecessary complication (passing around an
argument of type Compression wherever we may want to write a hashed file).
It is also better to have only one binary format for hashed files.
patch c36367a6dedcdf905928fb3d36bfb4cb6515f08e
Author: Ben Franksen <ben.franksen@online.de>
Date: Sat Dec 3 12:54:53 CET 2022
* clean up D.R.Clone.copyRepoOldFashioned
patch ac55c760fa0668aa440f7cf22dffbe63f463a6e5
Author: Ben Franksen <ben.franksen@online.de>
Date: Sat Dec 3 13:23:24 CET 2022
* remove the ugly ApplyDir type in favour of Invertible wrapper
This is much cleaner and allows us to give a more precise type signature for
applyToTentativePristine. This also makes tentativelyRemovePatches_ the
primitive and defines tentativelyAddPatch_ as a simple wrapper; this is more
symmetric to tentativelyRemovePatches_ and also more efficient.
patch 5e41b359d4ca90106041018368beff557e109ab1
Author: Ben Franksen <ben.franksen@online.de>
Date: Sat Dec 3 15:21:38 CET 2022
* eliminate the ugly and low-level applyToTentativePristineCwd
patch 0409f6095f45b8096b2e8615e0d5a37ca9199600
Author: Ben Franksen <ben.franksen@online.de>
Date: Thu Dec 15 16:53:16 CET 2022
* applyToTentativePristine: replace verbose message with progress reporting
This removes the verbosity parameter to that function and everywhere it gets
passed around just because we call that function somewhere down the line.
Instead we add progress reporting also when adding patches, symmetrical to
what we already did when removing patches.
Attachments
|
msg23597 (view) |
Author: ganesh |
Date: 2023-07-16.20:04:19 |
|
Reviewing incrementally
> * improve the interface of D.R.Inventory
> * add Darcs.Repository.Transaction
> * remove no longer needed export of diffHashLists
Fine
> * cleanup cleanPristineDir and move from D.R.Pristine to
D.R.Traverse
I think you've removed a call to okayHash, was that intentional?
|
msg23602 (view) |
Author: ganesh |
Date: 2023-07-16.20:42:12 |
|
> * repair: do not clean pristine dir
> * always store hashed files in compressed form
OK
> * clean up D.R.Clone.copyRepoOldFashioned
OK (I haven't really checked/understood the changes)
> * remove the ugly ApplyDir type in favour of Invertible wrapper
> This also makes tentativelyRemovePatches_ the
> primitive and defines tentativelyAddPatch_ as a simple wrapper
I got very confused by this note in the patch description until
I realised it meant to say make tentatively*Add*Patches_ the
primitive :-)
> (mkInvertible ps)
> invert $ mkInvertible
For what it's worth I find this a bit obfuscated and would probably
find just Fwd and Rev easier to read, if we chose to export them
from D.P.Invertible.
Anyway, the patch is basically fine.
|
msg23603 (view) |
Author: ganesh |
Date: 2023-07-16.20:44:00 |
|
> * eliminate the ugly and low-level applyToTentativePristineCwd
Did you intend to drop the bunchFL calls in this change?
> * applyToTentativePristine: replace verbose message with progress
reporting
Looks fine.
|
msg23610 (view) |
Author: bfrk |
Date: 2023-07-18.19:05:23 |
|
>> * cleanup cleanPristineDir and move from D.R.Pristine to D.R.Traverse
>
> I think you've removed a call to okayHash, was that intentional?
I think so: we want to remove all files in the pristine subdirectory that
are not reachable from the root. Even more so (in german i'd say "erst
recht") if they don't even have a valid name for a hashed file.
|
msg23611 (view) |
Author: bfrk |
Date: 2023-07-18.19:06:56 |
|
>> This also makes tentativelyRemovePatches_ the
>> primitive and defines tentativelyAddPatch_ as a simple wrapper
>
> I got very confused by this note in the patch description until
> I realised it meant to say make tentatively*Add*Patches_ the
> primitive :-)
OMG, I am sorry about that!
|
msg23612 (view) |
Author: bfrk |
Date: 2023-07-18.19:14:01 |
|
>> (mkInvertible ps)
>> invert $ mkInvertible
> For what it's worth I find this a bit obfuscated and would probably
> find just Fwd and Rev easier to read, if we chose to export them
> from D.P.Invertible.
Hmm, I don't get what you mean. We start with a patch that's not invertible.
We apply mkIvertible, to makeit invertible. Then we actually invert it.
What's obfuscated about this?
|
msg23613 (view) |
Author: bfrk |
Date: 2023-07-18.19:32:12 |
|
>> * eliminate the ugly and low-level applyToTentativePristineCwd
>
> Did you intend to drop the bunchFL calls in this change?
Yes, because I don't see what they're good for. If you happen to know and can
convince me that it makes a difference, I'll gladly put them back.
I didn't check to confirm it, but I suspect they come from a time before we
had the TreeMonad, with its automatic limiting of memory use by regularly
dumping TreeItems to disk (depending on the sum of the sizes of file changes).
Indeed, applying patches to pristine is pretty fast and I have never seen it
leak memory, even for very long histories like that of darcs itself. In fact I
have often been thinking that we should use a similar techique when applying
patches to working.
|
msg23614 (view) |
Author: ganesh |
Date: 2023-07-18.19:40:48 |
|
> We apply mkIvertible, to makeit invertible. Then we actually invert it.
> What's obfuscated about this?
It was clear once I dug into the details, but I think my mind immediately
assumed that mkInvertible was actually inverting something and got
particularly confused in the case where we just call `mkInvertible` and
not `invert`.
Not a big deal, anyway.
|
msg23615 (view) |
Author: ganesh |
Date: 2023-07-18.19:43:33 |
|
> > I think you've removed a call to okayHash, was that intentional?
> I think so: we want to remove all files in the pristine subdirectory
> that are not reachable from the root. Even more so (in german i'd say
> "erst recht") if they don't even have a valid name for a hashed file.
I see, so essentially any junk in there at all gets dropped. That seems
fair enough. It's a minor behaviour change but I doubt people are
using that folder to stash other content!
|
msg23622 (view) |
Author: bfrk |
Date: 2023-07-19.20:17:32 |
|
> It's a minor behaviour change but I doubt people are
> using that folder to stash other content!
https://xkcd.com/1172
|
|
Date |
User |
Action |
Args |
2023-06-17 12:18:20 | bfrk | create | |
2023-06-17 12:22:49 | bfrk | set | status: needs-screening -> needs-review |
2023-07-16 20:04:21 | ganesh | set | status: needs-review -> review-in-progress messages:
+ msg23597 |
2023-07-16 20:42:16 | ganesh | set | messages:
+ msg23602 |
2023-07-16 20:44:00 | ganesh | set | messages:
+ msg23603 |
2023-07-18 19:05:23 | bfrk | set | messages:
+ msg23610 |
2023-07-18 19:06:57 | bfrk | set | messages:
+ msg23611 |
2023-07-18 19:14:02 | bfrk | set | messages:
+ msg23612 |
2023-07-18 19:32:14 | bfrk | set | messages:
+ msg23613 |
2023-07-18 19:40:48 | ganesh | set | messages:
+ msg23614 |
2023-07-18 19:43:33 | ganesh | set | status: review-in-progress -> accepted-pending-tests messages:
+ msg23615 |
2023-07-19 20:17:32 | bfrk | set | messages:
+ msg23622 |
2023-07-29 10:49:04 | ganesh | set | status: accepted-pending-tests -> accepted |
|