[darcs-users] darcs patch: Add LANGUAGE pragmas for explicit langua... (and 3 more)
tux_rocker at reinier.de
Wed Oct 29 11:48:43 UTC 2008
2008/10/29 David Roundy <droundy at darcs.net>:
>> ] hunk ./src/Darcs/Repository/Internal.lhs 417
>> ftf <- filetype_function
>> Sealed pend <- read_pending repository
>> debugMessage "diffing dir..."
>> + -- the unsafeCoerceP below is necessary to be able to concatenate
>> + -- pend with NilFL to form dif. See http://hpaste.org/11480
>> let diffs = if null files
>> hunk ./src/Darcs/Repository/Internal.lhs 420
>> - then [smart_diff opts ftf cur work]
>> - else catMaybes (map (diff_at_path opts ftf cur work) (map fn2fp files))
>> - let workdiff = foldl (+>+) NilFL diffs
>> + then smart_diff opts ftf cur work
>> + else let diffsPerFile = catMaybes (map (diff_at_path opts ftf cur work) (map fn2fp files))
>> + in foldr (+>+) (unsafeCoerceP NilFL) diffsPerFile
>> dif = if AnyOrder `elem` opts
> There may be other bugs, but this one is sufficient to cause trouble. This
> unsafeCoerceP really is unsafe. Combining patches together in this way has
> serious potential for repository corruption! In particular, if a file is
> referenced more than once in the paths, you can get multiple copies of the
> same change.
> The get_unrecorded_in_files idea is great, and looks like the right sort of
> interface, but concatenating patches from separate diffs is way too
> fragile. A better approach would be to pass all the interesting files into
> a diff_at_paths.
I have to confess that I hadn't thought very hard about the case that
a user would specify overlapping paths (which, as I understand it, is
where the problem lies).
But with darcs-2 semantics, two identical primitive patches just add
up to one copy of that change, right? It's still heavily broken on
darcs-1 repos of course.
> I'm right now pushing a test that demonstrates this bug in whatsnew.
> (Except it passes, since I'm not applying these patches just yet.)
I'll try out the test and see what it does with these patches applied.
> P.S. This patch, of course, on its own couldn't cause repository
> corruption, since whatsnew doesn't modify the repository. But it would
> only be a matter of time before someone realized that we could apply the
> same optimization to record, and then we'd be seriously hosed. Which is
> why patch review is *very* important!
In fact, making record use this function was (and is) in my planning.
I'll just fix it first.
More information about the darcs-users