This adds a useful command to fetch patches for later application or examination. Darcs fetch repo -o file (or darcs fetch repo -O dir) works just as expected, and takes more or less the same arguments as pull. In the same vein, I think I'll add an unapply command to unpull a patch and keep a copy of it as a bundle (which then can be used as a lightweight darcs stash). I know these would be two more commands, but they are very useful, easily learned (almost the same arguments as existing commands), and with clear semantics.


Florent

Mon Nov  2 17:55:51 CET 2009  Florent Becker <florent.becker@ens-lyon.org>
  * Refactor Darcs.Commands.Pull

Mon Nov  2 18:09:47 CET 2009  Florent Becker <florent.becker@ens-lyon.org>
  * Add a fetch command to get remote patches into a bundle

Attachments

• refactor-darcs_commands_pull.dpatch
• unnamed

Hi Jason, if you could take this on (for correctness), that would be great.  

We'll still need to work out on list whether or not it's really a good idea to
apply this but having the technical aspect down would be good.

Here is the review:
http://lists.osuosl.org/pipermail/darcs-users/2009-November/022400.html

This has been sitting for a while, post review.  Should it be marked as 
obsoleted or similar?

Sorry, i've procrastinated a bit. I intend to resubmit it soon, together
with unrecord -o. I'll open a new patch then.

This is the updated version of the fetch work, along with a -o option
to obliterate. This options allows to stash recorded changes into a 
bundle. Use apply to restore them. The last two patches can be
applied separately to get only one of the features.

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


Thu Feb  4 15:04:34 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Put presentParticiple into the English module

Mon Feb 15 16:13:11 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Refactor Darcs.Commands.Pull

Mon Feb 15 16:28:08 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * demonadify useAbsoluteOrStd

Mon Feb 15 16:39:09 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * export stdout in RepoPath

Tue Feb 16 14:12:50 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * extract a few functions from Send

Tue Feb 16 15:06:08 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add a fetch command

Tue Feb 16 15:12:59 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add a -o option to obliterate
  This allows basic stashing of patches.

Attachments

• put-presentparticiple-into-the-english-module.dpatch
• unnamed

the grumpy old man's decision regarding fetch is here:
http://permalink.gmane.org/gmane.comp.version-control.darcs.user/22303

please ignore the previous bundle

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

Thu Feb  4 15:04:34 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Put presentParticiple into the English module

Mon Feb 15 16:13:11 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Refactor Darcs.Commands.Pull

Mon Feb 15 16:28:08 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * demonadify useAbsoluteOrStd

Mon Feb 15 16:39:09 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * export stdout in RepoPath

Tue Feb 16 15:59:32 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * extract a few functions from Send

Mon Feb 15 18:27:22 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add a fetch command

Tue Feb 16 16:17:14 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add -o option to obliterate, to keep a backup of the patch
  With that patch, you can mkdir _darcs/trash and add "obliterate
  output-auto-name _darcs/trash" to your defaults file. This gives
  you a rudimentary trash for obliterated patches.

Attachments

• put-presentparticiple-into-the-english-module.dpatch
• unnamed

It looks like these patches need to be updated to apply against the current 
version of darcs:
$ darcs apply ~/Downloads/put-presentparticiple-into-the-english-module.dpatch 
Backing up ./src/Darcs/Hopefully.hs(-darcs-backup0)
Backing up ./src/Darcs/Commands/Pull.lhs(-darcs-backup0)
There are conflicts in the following files:
./src/Darcs/Commands/Pull.lhs ./src/Darcs/Hopefully.hs

darcs failed:  Refusing to apply patches leading to conflicts.
If you would rather apply the patch and mark the conflicts,
use the --mark-conflicts or --allow-conflicts options to apply
These can set as defaults by adding
 apply mark-conflicts
to _darcs/prefs/defaults in the target repo. 


Sorry for not realizing that there were patches to be applied.  I don't think 
roundup emails me when there is ticket activity. Or if it did, it wasn't clear 
to me that I was the one who should take the next step.

Jason

I've taken the liberty of doing darcs apply -i --skip-conflicts (yay,
Ganesh!) to help this along a little.

Re-assigning to Florent.  I've added a personal todo to add a detector
to our patch tracker so that when the state flips to amend-requested, it
is automatically assigned to the patch creator.  Obsessive automation of
stupid things so that humans can spend more time doing things that
humans are good at.

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

Mon Feb 15 16:13:11 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Refactor Darcs.Commands.Pull

Fri Mar 19 14:31:33 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Resolve conflicts in Commands.Pull

These two patches also belong to bundle patch185


Tue Feb 16 15:59:32 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * extract a few functions from Send

Mon Mar 22 17:51:59 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * fix conflicts in Send

Tue Feb 16 16:17:14 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add -o option to obliterate, to keep a backup of the patch
  With that patch, you can mkdir _darcs/trash and add "obliterate
  output-auto-name _darcs/trash" to your defaults file. This gives
  you a rudimentary trash for obliterated patches.
  

Mon Feb 15 18:27:22 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add a fetch command

Attachments

• refactor-darcs_commands_pull.dpatch
• unnamed

Assigning back to Jason 

(and adding another possible TODO item for myself to tweak the hooks:
if patchstatus is amend-requested and the creator sends something that
looks like a patch-bundle [hard?] set the status back to needs-review
and to the person who set the status to amend-requested)

My apologies once again for not applying this before a conflict arose.

If I apply this and mark the conflicts I see this:
$ darcs whats src/Darcs/Commands/Pull.lhs
What's new in "src/Darcs/Commands/Pull.lhs":

hunk ./src/Darcs/Commands/Pull.lhs 52
+v v v v v v v
+#ifdef GADT_WITNESSES
hunk ./src/Darcs/Commands/Pull.lhs 55
-import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..),
-                             mapFL, nullFL, reverseRL, mapRL )
+#endif
+import Darcs.Patch.Info (PatchInfo)
+import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..), FL(..),
+                             mapFL, nullFL, reverseRL, mapRL, mapFL_FL )
+*************
+#ifdef GADT_WITNESSES
+import Darcs.Patch.Set ( Origin )
+#endif
+import Darcs.Patch.Info (PatchInfo)
+import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..), FL(..),
+                             mapFL, nullFL, reverseRL, mapRL, mapFL_FL )
+*************
+import Darcs.Patch.Info (PatchInfo)
+import Darcs.Patch.Bundle ( make_bundle, patchFilename)
+import Darcs.Patch.Set ( Origin )
+import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..), FL(..),
+                             mapFL, nullFL, reverseRL, mapRL, mapFL_FL )
+*************
+import Darcs.Patch.Info (PatchInfo)
+import Darcs.Patch.Set ( Origin )
+import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..), FL(..),
+                             mapFL, nullFL, reverseRL, mapRL, mapFL_FL )
+^ ^ ^ ^ ^ ^ ^


My guess is that the correct ones are:
hunk ./src/Darcs/Commands/Pull.lhs 52
+#ifdef GADT_WITNESSES
hunk ./src/Darcs/Commands/Pull.lhs 55
+#ifdef GADT_WITNESSES
+import Darcs.Patch.Set ( Origin )
+#endif
+import Darcs.Patch.Info (PatchInfo)
+import Darcs.Patch.Bundle ( make_bundle, patchFilename)
+import Darcs.Witnesses.Ordered ( (:>)(..), (:\/:)(..), RL(..), FL(..),
+                             mapFL, nullFL, reverseRL, mapRL, mapFL_FL )

Sorry!
Jason

Hi Florent, would this be an easy amend for you?

btw the refactoring of Darcs.Commands.Pull has a slight bug in it, it 
changes the list of patches that gets passed to 'definePatches'

Other than the issue in my previous message, the refactoring of pull and 
the associated conflict resolution look fine to me (I've been looking at 
this because it's also part of patch185).

This is the updated version, which also fixes the problem with definePatches

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

Mon Feb 15 16:13:11 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Refactor Darcs.Commands.Pull

Fri Mar 19 14:31:33 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Resolve conflicts in Commands.Pull

Tue Feb 16 15:59:32 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * extract a few functions from Send

Mon Mar 22 17:51:59 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * fix conflicts in Send

Tue Feb 16 16:17:14 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add -o option to obliterate, to keep a backup of the patch
  With that patch, you can mkdir _darcs/trash and add "obliterate
  output-auto-name _darcs/trash" to your defaults file. This gives
  you a rudimentary trash for obliterated patches.
  

Thu Apr 22 13:34:03 CEST 2010  Florent Becker <florent.becker@ens-lyon.org>
  * export the right list of patches when not pulling anything

Thu Apr 22 16:11:33 CEST 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add a fetch command

Attachments

• refactor-darcs_commands_pull.dpatch
• unnamed

I've applied the pull refactoring patches (which are shared with 
patch185). Assigning back to Jason for the other bits as I haven't looked 
at them before and hopefully it's an easy check for him.

Jason appears to be busy (he's unassigned himself), so I'm taking this
one over.

All of these patches have conflicts with HEAD.  I think Florent's been
patient enough with this, so I'm going to try to resolve the conflicts
myself and push.

On Thu, Apr 22, 2010 at 14:28:49 +0000, Florent Becker wrote:
> Mon Feb 15 16:13:11 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * Refactor Darcs.Commands.Pull
> 
> Fri Mar 19 14:31:33 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * Resolve conflicts in Commands.Pull
>
> Thu Apr 22 13:34:03 CEST 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * export the right list of patches when not pulling anything

These were applied by Ganesh (thanks for helping things along).

> Tue Feb 16 15:59:32 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * extract a few functions from Send
> 
> Mon Mar 22 17:51:59 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * fix conflicts in Send
> 
> Tue Feb 16 16:17:14 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * Add -o option to obliterate, to keep a backup of the patch
>   With that patch, you can mkdir _darcs/trash and add "obliterate
>   output-auto-name _darcs/trash" to your defaults file. This gives
>   you a rudimentary trash for obliterated patches.
> 
> Thu Apr 22 16:11:33 CEST 2010  Florent Becker <florent.becker@ens-lyon.org>
>   * Add a fetch command

To make it easier for me to understand this patch, I viewed in a
graphical diff tool according to its expected context

  darcs get --lazy http://darcs.net --context refactor-darcs_commands_pull.dpatch review-patch39
  
  darcs apply -i refactor-darcs_commands_pull.dpatch review-patch39
  # cherry pick the patches Ganesh already applied
  
  darcs apply -i refactor-darcs_commands_pull.dpatch review-patch39
  # apply the rest of them
  
  darcs diff --diff-command="meld %1 %2" --last=4

extract a few functions from Send
---------------------------------
> -                         checkOrRepair, justThisRepo, optimizePristine
> +                         checkOrRepair, justThisRepo, optimizePristine,
> +                         getOutput

> +getOutput :: [DarcsFlag] -> FilePath -> Maybe AbsolutePathOrStd

Hunk move.

> +{-# LANGUAGE CPP, TypeOperators #-}

Was this really needed?

> -      pristine' <- applyToTree (invert $ mapRL_RL hopefully us') pristine
> -      unsig_bundle <- make_bundle (Unified:opts) pristine' common (mapFL_FL hopefully to_be_sent)
> -      bundle <- signString opts unsig_bundle
> +      bundle <- prepareBundle opts common pristine (us':\/:to_be_sent)

> +prepareBundle :: forall p C(x y). RepoPatch p => [DarcsFlag] -> [PatchInfo]
> +                -> Tree IO -> ((RL (PatchInfoAnd p)) :\/: (FL (PatchInfoAnd p))) C(x y)
> +                -> IO Doc
> +prepareBundle opts common pristine (us' :\/: to_be_sent) = do
> +  pristine' <- applyToTree (invert $ mapRL_RL hopefully us') pristine
> +  unsig_bundle <- make_bundle (Unified:opts) pristine' common (mapFL_FL hopefully to_be_sent)
> +  signString opts unsig_bundle

Straightforward refactor.

> -patchDesc :: forall p C(x y) . PatchInfoAnd p C(x y) -> String
> -safeFileChar :: Char -> Char
> -patchFilename :: String -> String

Moved to Darcs.Patch.Bundle (which David rightly observed should really
be called Darcs.PatchBundle)

Add -o option to obliterate, to keep a backup of the patch
----------------------------------------------------------
>  obliterateCmd = genericObliterateCmd "obliterate"
> hunk ./src/Darcs/Commands/Unrecord.lhs 305
>                                    else matchingHead opts allpatches
>    with_selected_last_changes_reversed cmdname opts Nothing
>        (reverseRL patches) $
> -    \ (_ :> ps) ->
> -    case commute (effect ps :> pend) of
> +    \ (kept :> removed) ->
> +    case commute (effect removed :> pend) of

Minor rename from ps to removed (and keep track of kept ones too)

> +        when (isJust $ getOutput opts "") $
> +             savetoBundle opts kept removed

Fair enough.

> +savetoBundle :: RepoPatch p => [DarcsFlag]
> +             -> FL (PatchInfoAnd p) C(x y) -> FL (PatchInfoAnd p) C(z t)
> +             -> IO ()
> +savetoBundle opts kept removed@(x :>: _) = do
> +    bundle <- make_bundle opts undefined (mapFL info kept)
> +              (mapFL_FL hopefully removed)
> +    let filename = patchFilename $ patchDesc x
> +        Just outname = getOutput opts filename
> +    useAbsoluteOrStd writeDocBinFile putDoc outname $ bundle

Dump to file.  We need kept to have the context for the bundle.
I guess this is about as factored out with send as it will get.

Add a fetch command
-------------------
> +fetch = DarcsCommand {
> +         commandName = "fetch",
> +         commandHelp = fetch_help,
> +         commandDescription = fetch_description,
> +         commandExtraArgs = -1,
> +         commandExtraArgHelp = ["[REPOSITORY]..."],
> +         commandCommand = fetch_cmd,
> +         commandPrereq = amInRepository,
> +         commandGetArgPossibilities = getPreflist "repos",
> +         commandArgdefaults = defaultrepo,
> +         commandAdvancedOptions = [repoCombinator,

--union --intersection, etc (fine)

> +                                     nocompress, nolinks,
> +                                     ignoretimes,

Why do we need nocompress, nolinks and ignoretimes?

I ignored the rest of the options

> +fetch_cmd :: [DarcsFlag] -> [String] -> IO ()
> +fetch_cmd opts repos =
> +    withRepoLock opts $- \ repository ->
> +        fetchPatches opts' repos "fetch" repository 
> +                         >>= makeBundle opts' repository
> +    where
> +      opts' = merge_opts opts

Why would we want the merge_opts?  Presumably, mark-conflicts
doesn't really make sense here, does it?

(note this may change the options we accept)

> +makeBundle ::
> +    forall p C(r u t) . (RepoPatch p) => [DarcsFlag] -> Repository p C(r u t) ->
> +    ([PatchInfo], Sealed ((RL (PatchInfoAnd p) :\/: FL (PatchInfoAnd p)) C(r)))
> +    -> IO ()
> +makeBundle opts repository (common, Sealed (_ :\/: to_be_fetched)) =
> +    do
> +      bundle <- make_bundle opts undefined common $
> +                 mapFL_FL hopefully to_be_fetched
> +      let fname = case to_be_fetched of
> +                    (x:>:_)-> patchFilename $ patchDesc x
> +          o = fromMaybe stdOut (getOutput opts fname)
> +      useAbsoluteOrStd writeDocBinFile putDoc o $ bundle

Save a bundle to a file or print it on stdout.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

A few more points now that these patches are in.

Well, we just grew two whole new features (one of them being a new darcs
command!).  Hope we won't regret it.  Better, I hope these features will
put us in a nice position to avoid other new features down the road.

Documentation:
 * The man page entry at the darcs fetch section seems
   to be missing a newline
 * Shouldn't obliterate documentation point out the the
   -O feature for undo?
 * Anything else we need to say?
 * Add global default of -O _darcs/trash to best practices?

Code:
 * My conflict resolution was messy (some redundant imports)
   cleanup on aisle 6 (sorry!)

Tests:
 * Do we need any testing here? (think unexpected interactions?)
   I can't really think of anything... fetch seems pretty self-contained

Functionality:
 * Do we want a stub for darcs unobliterate which says
   something about darcs obliterate -O?
 
-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

This patch bundle (with 9 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-52129da72c27d855b348f2f23844f2e4bdfd1115

This patch bundle (with 9 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-52129da72c27d855b348f2f23844f2e4bdfd1115