darcs

Patch 1611 refactored convert darcs-2 command

Title refactored convert darcs-2 command
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2017-10-04.08:45:16 by bfrk, last changed 2018-02-07.21:48:52 by gh.

Files
File name Status Uploaded Type Edit Remove
refactored-convert-darcs_2-command.dpatch bfrk, 2017-10-04.08:53:09 application/x-darcs-patch
refactored-creation-of-new-repositories.dpatch bfrk, 2017-10-26.20:50:39 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg19696 (view) Author: bfrk Date: 2017-10-04.08:45:15
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Sep 29 22:09:08 CEST 2017
  * refactored convert darcs-2 command
  
  There is no need to merge anything when converting, we can just add the
  patches to the new repo, committing our changes every 100 patches like we
  did before.
  
  To get the types right was a challenge. I added checkRepoIsNoRebase to
  Darcs.Repository.Job so as not to pollute the command implementation
with a
  host of obscure singleton types. Using foldFL_M requires two separate
  newtype wrappers (W2 and W3) because the procedures we want to fold
  simultaneously change two and three witnesses, respectively, and not just
  the last one. It may be possible to encapsulate this somehow, but I leave
  that to future refactorings. In spite of all that effort, we still need to
  coerce the repo parameter in two places, which I extensively
documented: one
  to work around the types of finalizeRepositoryChanges and
  revertRepositoryChanges (which are not precise enough) and another one to
  work around the type of withRepoLock (which is too restrictive).
Attachments
msg19697 (view) Author: bfrk Date: 2017-10-04.08:53:09
Screening a slightly amended version that makes W2 and W3 more symmetric.
Attachments
msg19780 (view) Author: ganesh Date: 2017-10-23.17:28:48
> noteIO "Cannot convert repo with rebase in progress." $

This is a bit misleading: if checkRepoIsNoRebase fails, something 
must have gone seriously wrong, given we're checking the repository 
we only just created. The "is a rebase in progress" check on the 
source (V1) repo will be handled by V1Job.
msg19781 (view) Author: bfrk Date: 2017-10-26.08:10:56
ganesh: right, what we check is the new repo, not the one we convert,
and the check must always succeed. Will fix.
msg19786 (view) Author: bfrk Date: 2017-10-26.20:50:39
It bugged me that we need this run-time check at all, just to make the
types fit. This is stupid.

So I went ahead and fixed the underlying problem, namely that
createRepository does not return a properly typed Repository. With that
change, and a few additional tools we can avoid withRepoLock and work
directly with the returned Repository.

The attached bundle contains the new patch, see its long comment for
details.
Attachments
msg19794 (view) Author: gh Date: 2017-10-28.15:22:11
I screened your last patch, it looks sensible to me and I agree with the
creation of the Create module.
msg19837 (view) Author: gh Date: 2018-02-07.21:48:51
I just realized that Ben's second bundle fixes the small issue pointed
by Ganesh.

Accepting both bundles.
History
Date User Action Args
2017-10-04 08:45:16bfrkcreate
2017-10-04 08:50:52bfrksetfiles: - refactored-convert-darcs_2-command.dpatch
2017-10-04 08:53:09bfrksetstatus: needs-screening -> needs-review
files: + refactored-convert-darcs_2-command.dpatch
messages: + msg19697
2017-10-23 17:28:48ganeshsetmessages: + msg19780
2017-10-26 08:10:57bfrksetmessages: + msg19781
2017-10-26 20:50:40bfrksetfiles: + refactored-creation-of-new-repositories.dpatch
messages: + msg19786
2017-10-28 15:22:11ghsetmessages: + msg19794
2018-02-07 21:48:52ghsetstatus: needs-review -> accepted
messages: + msg19837