darcs

Patch 635 Generalise the Apply class by introducing an ApplyState associated type

Title Generalise the Apply class by introducing an ApplyState associated type
Superseder Nosy List mornfall
Related Issues
Status accepted Assigned To
Milestone

Created on 2011-06-29.22:02:10 by mornfall, last changed 2011-12-29.21:36:16 by ganesh.

Files
File name Status Uploaded Type Edit Remove
add-a-couple-of-extra-constraints-required-by-ghc6_.dpatch ganesh, 2011-08-14.11:19:48 application/x-darcs-patch
disable-part-of-patch-tests-in-ghc_7-_requires-ghc7_style-impredicativity__.dpatch ganesh, 2011-08-14.11:10:28 application/x-darcs-patch
generalise-the-apply-class-by-introducing-an-applystate-associated-type_.dpatch dead mornfall, 2011-06-29.22:02:09 application/x-darcs-patch
parametrise-part-of-patch_v2-tests-over-the-prim-to-use_.dpatch mornfall, 2011-07-10.10:12:38 application/x-darcs-patch
parametrise-part-of-patch_v2-tests-over-the-prim-to-use_.dpatch mornfall, 2011-07-11.19:55:08 application/x-darcs-patch
unnamed mornfall, 2011-06-29.22:02:09 text/x-darcs-patch
unnamed mornfall, 2011-06-29.22:02:09
unnamed mornfall, 2011-07-10.10:12:38 text/x-darcs-patch
unnamed mornfall, 2011-07-10.10:12:38
unnamed mornfall, 2011-07-11.19:55:08 text/x-darcs-patch
unnamed mornfall, 2011-07-11.19:55:08
unnamed ganesh, 2011-08-14.11:10:28 text/x-darcs-patch
unnamed ganesh, 2011-08-14.11:10:28
unnamed ganesh, 2011-08-14.11:19:48 text/x-darcs-patch
unnamed ganesh, 2011-08-14.11:19:48
See mailing list archives for discussion on individual patches.
Messages
msg14560 (view) Author: mornfall Date: 2011-06-29.22:02:09
Hey there,

this is kind of a jumbo patch... but I don't think it can be made smaller
without introducing broken intermediate states. On the other hand, *most* of it
is fairly mechanical.

Things to look out for:

- the showContextPatch implementation; I changed this one to avoid the hard
  Tree / Hunk dependency in the showContextSeries code; it *might* have been
  premature, but the current version seems to work...
- the ApplyMonad instances are generally somewhat incomplete, and the "generic"
  interface is more or less a sketch
- I have retained the whole pre-existing Tree-specific ApplyMonad interface,
  using a per-method context trick to only make it available to Tree-based
  Prims; it is a compromise that allows most of the actual code to stay
  unchanged and (therefore) avoid possible performance issues
- all higher-level patches are expected to use ApplyState defined by their
  Prims, and when we get equality in superclass constraints, we can actually
  express that... for now, I have included some hacks to avoid part of the
  (ApplyState (PrimOf p)) constraints in Repository code;
- a substantial part of the command code directly relies on Tree being the
  ApplyState of the things it manipulates; we already knew that, but this
  underlines the fact that the interface between Command.* and the rest of
  darcs has the wrong level of abstraction; we probably need to move and
  refactor a lot of code in the long run, to make supporting both V1 and V3
  Prims feasible in a single codebase

If someone can have a quick glance to confirm they can't see anything obviously
wrong, I'll screen the patch.

Yours,
   Petr

PS: This opens the door to implementing an Apply instance for V3 Prims. I'll
proceed with that shortly, and I'll also try to find and/or create properties
that specifically test patch application (without invoking commute, merge and
other primitives, that is).

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

Wed Jun 29 23:11:20 CEST 2011  Petr Rockai <me@mornfall.net>
  * Generalise the Apply class by introducing an ApplyState associated type.
  
  The ApplyState type corresponds to the object that the patches manipulate. For
  traditional (V1) Prims, this is a Tree, representing a filesystem tree. For V3
  Prims, this will likely be an uuid -> object map, or a variation thereof.
  
  In an ideal world, apply would be :: patch -> ApplyState patch -> ApplyState
  patch, but for efficiency reasons, we want to evaluate patch application in a
  monad, and also embed unevaluated monadic computations within the state object
  itself. Therefore, (ApplyState patch) is :: (* -> *) -> *, the parameter being
  a monad, like what Tree expects. In normal Darcs, this will be IO, but most
  operations are expected to be monad-independent. We use Maybe in the testsuite.
Attachments
msg14561 (view) Author: ganesh Date: 2011-06-30.06:41:19
On 29/06/2011 23:02, Petr Ročkai wrote:

> - the showContextPatch implementation; I changed this one to avoid the hard
>   Tree / Hunk dependency in the showContextSeries code; it *might* have been
>   premature, but the current version seems to work...

It might be clearer to solve this at least initially by requiring that
any ApplyState contains a Tree that can be extracted (i.e. have a
superclass on Apply like IsTree (ApplyState p) - as it stands your
changes make the patch hard to check for correctness. If they make sense
anyway they could then be a separate patch that can be looked at in
isolation.

> If someone can have a quick glance to confirm they can't see anything obviously
> wrong, I'll screen the patch.

Other than that, it looks fine.

Ganesh
msg14578 (view) Author: mornfall Date: 2011-07-10.10:12:38
Hi,

this bundle now implements also a basic commute and apply for V3 Prims, over an
ObjectMap model of the repository. The representation needs to be optimised
(both in terms of data structures, as well as in allowing deferred IO as in
Tree, through embedded monadic actions).

The big "generalise the Apply class" patch is redone to accomodate existing
showContextSeries code (well, with minimal changes).

The currently available tests are covered by the implementation, with the
exception of Read/Show instances (I am working on these right now). More tests
are needed though.

Yours,
   Petr

6 patches for repository http://darcs.net:

Tue Jul  5 10:42:35 CEST 2011  Petr Rockai <me@mornfall.net>
  * Parametrise part of Patch.V2 tests over the Prim to use.

Tue Jul  5 10:43:58 CEST 2011  Petr Rockai <me@mornfall.net>
  * Enable tests for V3 prims and V2 patches with V3 prims (all fail).

Sat Jul  9 11:10:38 CEST 2011  Petr Rockai <me@mornfall.net>
  * Generalise the Apply class by introducing an ApplyState associated type.
  
  The ApplyState type corresponds to the object that the patches manipulate. For
  traditional (V1) Prims, this is a Tree, representing a filesystem tree. For V3
  Prims, this will likely be an uuid -> object map, or a variation thereof.
  
  In an ideal world, apply would be :: patch -> ApplyState patch -> ApplyState
  patch, but for efficiency reasons, we want to evaluate patch application in a
  monad, and also embed unevaluated monadic computations within the state object
  itself. Therefore, (ApplyState patch) is :: (* -> *) -> *, the parameter being
  a monad, like what Tree expects. In normal Darcs, this will be IO, but most
  operations are expected to be monad-independent. We use Maybe in the testsuite.

Sat Jul  9 11:44:11 CEST 2011  Petr Rockai <me@mornfall.net>
  * Lift localIdentity into PrimClassify (as anIdentity).

Sat Jul  9 11:59:39 CEST 2011  Petr Rockai <me@mornfall.net>
  * Start filling in gaps in the V3 prims.

Sat Jul  9 19:51:57 CEST 2011  Petr Rockai <me@mornfall.net>
  * Fill in more blanks in V3 Prim implementation.
  
  With the current test coverage (generic Prim tests), we are only missing
  read/write to pass everything. Of course, we want to eventually improve test
  coverage, which should lead to further exposed gaps.
Attachments
msg14585 (view) Author: mornfall Date: 2011-07-11.19:55:08
As the last patch says, we have all tests passing now on the V3
front. Methodology therefore demands we need to write more tests, including
some that'll fail, so we can improve the code accordingly.

Yours,
   Petr

PS: I think this could be screened now. Another quick glance from someone else
would be appreciated first, though.

7 patches for repository http://darcs.net:

Tue Jul  5 10:42:35 CEST 2011  Petr Rockai <me@mornfall.net>
  * Parametrise part of Patch.V2 tests over the Prim to use.

Tue Jul  5 10:43:58 CEST 2011  Petr Rockai <me@mornfall.net>
  * Enable tests for V3 prims and V2 patches with V3 prims (all fail).

Sat Jul  9 11:10:38 CEST 2011  Petr Rockai <me@mornfall.net>
  * Generalise the Apply class by introducing an ApplyState associated type.
  
  The ApplyState type corresponds to the object that the patches manipulate. For
  traditional (V1) Prims, this is a Tree, representing a filesystem tree. For V3
  Prims, this will likely be an uuid -> object map, or a variation thereof.
  
  In an ideal world, apply would be :: patch -> ApplyState patch -> ApplyState
  patch, but for efficiency reasons, we want to evaluate patch application in a
  monad, and also embed unevaluated monadic computations within the state object
  itself. Therefore, (ApplyState patch) is :: (* -> *) -> *, the parameter being
  a monad, like what Tree expects. In normal Darcs, this will be IO, but most
  operations are expected to be monad-independent. We use Maybe in the testsuite.

Sat Jul  9 11:44:11 CEST 2011  Petr Rockai <me@mornfall.net>
  * Lift localIdentity into PrimClassify (as anIdentity).

Sat Jul  9 11:59:39 CEST 2011  Petr Rockai <me@mornfall.net>
  * Start filling in gaps in the V3 prims.

Sat Jul  9 19:51:57 CEST 2011  Petr Rockai <me@mornfall.net>
  * Fill in more blanks in V3 Prim implementation.
  
  With the current test coverage (generic Prim tests), we are only missing
  read/write to pass everything. Of course, we want to eventually improve test
  coverage, which should lead to further exposed gaps.

Mon Jul 11 21:39:51 CEST 2011  Petr Rockai <me@mornfall.net>
  * Implement rudimentary Show/Read instances for V3 prims. All tests pass now.
Attachments
msg14654 (view) Author: ganesh Date: 2011-08-14.11:10:28
This patch was in screened but not tracked. Seems to be logically part of this
bundle.

1 patch for repository /home/ganesh/darcs/darcs-screened-temp:

Tue Jun 28 13:39:37 BST 2011  Petr Rockai <me@mornfall.net>
  * Disable part of Patch tests in GHC<7 (requires GHC7-style impredicativity).
Attachments
msg14657 (view) Author: ganesh Date: 2011-08-14.11:19:48
Another patch in screened but not the tracker.

1 patch for repository /home/ganesh/darcs/darcs-screened-temp:

Tue Jun 28 13:38:53 BST 2011  Petr Rockai <me@mornfall.net>
  * Add a couple of extra constraints required by GHC6.
Attachments
msg14881 (view) Author: ganesh Date: 2011-12-28.21:21:52
I'm accepting this without further review as it's been ages.
History
Date User Action Args
2011-06-29 22:02:10mornfallcreate
2011-06-30 06:41:19ganeshsetmessages: + msg14561
2011-07-10 10:12:39mornfallsetfiles: + unnamed, parametrise-part-of-patch_v2-tests-over-the-prim-to-use_.dpatch, unnamed
messages: + msg14578
2011-07-11 19:55:08mornfallsetfiles: + unnamed, parametrise-part-of-patch_v2-tests-over-the-prim-to-use_.dpatch, unnamed
messages: + msg14585
2011-08-14 11:10:29ganeshsetfiles: + unnamed, disable-part-of-patch-tests-in-ghc_7-_requires-ghc7_style-impredicativity__.dpatch, unnamed
messages: + msg14654
title: Generalise the Apply class by introducing an ApplyStat... -> Disable part of Patch tests in GHC<7
2011-08-14 11:19:48ganeshsetfiles: + unnamed, add-a-couple-of-extra-constraints-required-by-ghc6_.dpatch, unnamed
messages: + msg14657
title: Disable part of Patch tests in GHC<7 -> Add a couple of extra constraints required by GHC6
2011-08-14 11:20:15ganeshsettitle: Add a couple of extra constraints required by GHC6 -> Generalise the Apply class by introducing an ApplyState associated type
2011-12-27 20:11:48ganeshsetstatus: needs-screening -> needs-review
2011-12-28 21:21:52ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg14881
2011-12-29 21:36:16ganeshsetstatus: accepted-pending-tests -> accepted