[darcs-devel] [patch424] Support init/get --no-working-dir and pseudo-applying ...

[Eric Kow]

Eric Kow <kowey at darcs.net> added the comment:

The code seems right, but I'm not sure this is really working.  I wrote 4 
tests for it, sort of discovering issues as I started writing tests

1. --no-working-dir => really no working dir
2. --no-worknig-dir --with-working-dir (vice-versa): what happens?
3. what happens if you pull some changes into a --no-working-dir?
4. what happens if you record in a --no-working-dir

The first one works.  #2 fails, but it's pretty trivial to fix, not
a blocker. #3 fails bizarrely (cannot read a particular file in
pristine), and I don't think it's the patch's fault (?).
#4 is just me not knowing what *should* happen, but I suspect that
Florent's already thought of it and decided it's a corner case to
not worry about?  May be misremembering

We also seem to have a whole mess of patches in screened that depend on
this one. I'm tempted to say we should just merge this, mark tests
as failing and figure out where to go from there just so we can unblock
the logjam.  Agreed?

Any chance you'd be free to comment, Florent?
Anybody else want to suggest how to get unstuck?


Support init/get --no-working-dir and pseudo-applying to wd-less repos
----------------------------------------------------------------------
> hunk ./src/Darcs/Arguments.hs 978
> +useWorkingDir :: DarcsOption
> +useWorkingDir =
> +  DarcsMultipleChoiceOption
> +  [ DarcsNoArgOption [] ["with-working-dir"] UseWorkingDir
> +                         "Create a working directory (normal 
repository)",
> +    DarcsNoArgOption [] ["no-working-dir"] UseNoWorkingDir
> +                           "Do not create a working directory (bare 
repository)"]

Why not just --working-dir?

> -              do identifyRepositoryFor repository repodir >>= 
copyRepository
> +              do identifyRepositoryFor repository repodir >>= flip 
copyRepository (not $ UseNoWorkingDir `elem` opts)

Perhaps use getBoolFlag instead?
See the R3 test in my proposed test suite attached

> +      then do
> +        when withWorkingDir $ do
> +          debugMessage "Writing working directory contents..."
> +          createPristineDirectoryTree torepository "."
> +        fetchPatchesIfNecessary opts torepository `catchInterrupt`
> +          (putInfo opts $ text "Using lazy repository.")
> hunk ./src/Darcs/Repository.hs 230
> -                   debugMessage "Writing working directory 
contents..."
> -                   createPristineDirectoryTree torepository "."
> +                   when withWorkingDir $ do
> +                     debugMessage "Writing working directory 
contents..."
> +                     createPristineDirectoryTree torepository "."

Perhaps a chance for a minor refactor using a let/where on writing
the working dir

> +data RepoProperty = Darcs1_0 | Darcs2 | HashedInventory | NoWorkingDir

> -createRepoFormat fs = RF (map rp2ps [HashedInventory]: maybe2)
> +createRepoFormat fs = RF (map rp2ps (HashedInventory:flags2wd): 
maybe2)
> hunk ./src/Darcs/Repository/Format.hs 101
> +          flags2wd = if UseNoWorkingDir `elem` fs
> +                      then [NoWorkingDir]
> +                      else []

OK, no-working-dir exists now on the same level as hashed.

I have the impression thath the minced format discussion is important
here

* http://lists.osuosl.org/pipermail/darcs-users/2010-November/025614.html
* http://bugs.darcs.net/issue1647

Maybe: burn the format mechanism to the ground and design a new one

If Darcs 3 is not readyd by the time we need it: use the existing
format mechanism to require that future Darcs understanding the new
format mechanism.

> +      withCurrentDirectory r $ if Quiet `elem` opt
> +                               then runSilently $ apply patch
> +                               else runTolerantly $ apply patch
> +    return (Repo r ropts rf (DarcsRepository t c))
> hunk ./src/Darcs/Repository/Merge.hs 66
> -            mapM_ backupByCopying $ listTouchedFiles 
standard_resolved_pw
> +       mapM_ backupByCopying $ listTouchedFiles standard_resolved_pw

> hunk ./src/Darcs/Repository/State.hs 139
> +unrecordedChanges _ r@(Repo _ _ rf _) _
> +  | (formatHas NoWorkingDir rf) = do
> +    IsEq <- return $ workDirLessRepoWitness r
> +    return NilFL
> hunk ./src/Darcs/Repository/State.hs 168
> +-- | Witnesses the fact that in the absence of a working directory, we
> +-- pretend that the working dir updates magically to the tentative 
state.
> +workDirLessRepoWitness :: Repository p C(r u t)
> +                          -> (EqCheck C(u t))
> +workDirLessRepoWitness r@(Repo _ _ rf _) | formatHas NoWorkingDir rf =
> +  unsafeCoerceP IsEq
> +                                         | otherwise = NotEq
> +

I'm going to have to take your word for this one...  I *think* I
understand the intention, which is to make sure that the abscence of
files in the working dir is not treated as an unrecorded diff.
This would be important if we wanted to pull patches, for example,
in the attached 'pull R1b' test case

----------
assignedto: gh -> 
status: needs-review -> in-discussion

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch424>
__________________________________
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-working.sh
Type: application/octet-stream
Size: 2239 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-devel/attachments/20111227/3455fdfa/attachment.obj>