[darcs-users] [patch429] accept issue1977 (and 1 more)

[gh]

gh <guillaumh at gmail.com> added the comment:

2010/11/9 Florent Becker <bugs at darcs.net>:
>
> Florent Becker <florent.becker at ens-lyon.org> added the comment:
>
>> The good thing is that repositories with that _darcs/pristine.none
>> file keep on being recognized as NoPristine repositories, so from the
>> retro-compatibility point of view, I think this change is OK.
>
> Can you add a test that we keep this file when it exists?

I have tried writing such a test but when you think about the possible
cases you get:

* either test that "darcs repair" leaves pristine.none in a hashed
repository, which is a useless test since hashed repos always have a
hashed prsitine.
* either test that "darcs repair" leaves pristine.none in an OF
repository. I tried to do that but I realized that "darcs repair"
never works in an OF repository without the _darcs/pristine/
directory, before or after this bundle. So "darcs repair" just fails
in that case.

In short, the fix for issue1977 in this bundle does not fix the case
of OF repositories, but only for hashed repositories. OTOH it does not
degrade the behaviour of darcs for the OF case.

Since by looking at the code it is clear that no pristine.none or
current.none file is removed, I suggest we can do without such a test,
what do you think?


>>  data Pristine
>> -  = NoPristine !String
>> +  = NoPristine
>>    | PlainPristine !String
>>    | HashedPristine
>
> What would the String parameter represent before?

It was either "pristine.none" or "current.none", according to the
function identifyPristine before this bundle.
It was also "aack?" when the function
Darcs.Repository.Internal.maybeIdentifyRepository needed to return a
dummy object of the type IdentifyRepo when successfully identifying a
remote repository.

> Ok, any reason not to use the NoPristine constructor? Now that it has
> no argument, there is no reason to keep the nopristine constant around.

Indeed, I'll send a patch removing this function.


> hunk ./src/Darcs/Repository/Pristine.hs 116
>>      bug "3 HashedPristine is not implemented yet."
>>
>>  createPristineFromWorking :: Pristine -> IO ()
>> -createPristineFromWorking (NoPristine _) = return ()
>> +createPristineFromWorking NoPristine = return ()
>>  createPristineFromWorking (PlainPristine n) = cloneTreeExcept
> [darcsdir] "." n
>>  createPristineFromWorking HashedPristine =
>>      bug "HashedPristine is not implemented yet."
>
> ok (isn't the  bug "HashedPristine is not implemented yet." a lie?)
>

Yes the bug strings are wrong: these cases can never be reacher in the
current state of the code.
Writing a patch for that. What a plate of spaghetti.


>>  easyCreatePartialsPristineDirectoryTree :: FilePathLike fp => [fp] ->
> Pristine
>>                                          -> FilePath -> IO Bool
>> -easyCreatePartialsPristineDirectoryTree _ (NoPristine _) _ = return
> False
>> +easyCreatePartialsPristineDirectoryTree _ NoPristine _ = return False
>>  easyCreatePartialsPristineDirectoryTree prefs (PlainPristine n) p
>>   = clonePartialsTree n p (map toFilePath prefs) >> return True
>>  easyCreatePartialsPristineDirectoryTree _ HashedPristine _ =
>
> A haddock for easy… would be welcome.

OK I'll try.

Guillaume

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch429>
__________________________________