[darcs-devel] darcs patch: add test for replace that messes with un... (and 3 more)

Eric Y. Kow eric.kow at gmail.com
Sun Jan 28 10:43:12 PST 2007


Hey David, Tommy, all.

David, could you have a second look at these?

> The result is that now a Repository "knows" if it should be ignoring
> the time stamps or not.  This refactoring could lead to others, since
> there are certainly functions that should no longer need to have the
> [DarcsFlag] passed.

Yeah, not having both sounds good, but...

In the Repository.lhs module, there is a get_unrecorded function that
now has oldopts and moreopts.  Can moreopts go away?

Also, I'm slightly unhappy about passing [] to identifyRepository in
cases where the flags are irrelevant.  I don't have a clear objections
to it just misgivings about it coming back to haunt us in the future.
Maybe it's a more general feeling that passing flags around just isn't
explicit enough for Repository?

I was considering maybe a Bool or a record, but this seems also
inconvenient and messy because you'd have to convert from the flags.
As a slight consolation, I'll note that Diff.gendiff uses a tuple
of Bools, so if we were to come up with some kind of record, maybe
we could refactor against that.

> Thu Jan 25 07:38:03 PST 2007  Tommy Pettersson <ptp at lysator.liu.se>
>   * add test for replace that messes with unrecorded hunks

> echo "x" > foo
> darcs rec -am x
> echo "y" > foo
> darcs replace x y foo
> 
> # this fails
> echo "hej" > foo
> darcs rec -am hej
> echo "hopp" > foo
> darcs replace hej hopp foo

Thanks for the test!

If I'm reading this right, these are the same test but just repeated so
that we can slip into the condition where it fails, right?  There is
nothing about the first test that makes it any less prone to failure
(buggy pending) than the second one, is there?  Would it maybe make
sense to capture this by having the same test repeated in a for loop,
say "darcs replace hej$x hopp$x" where $x is a counter?  Or rather,
maybe just keeping one with --ignore-times; would that do the trick?

> Sat Jan 27 16:07:28 PST 2007  David Roundy <droundy at darcs.net>
>   * refactor: add opts into Repository.

See random apprehensions above.  Actually, I was going to take the
patch, but since I am rejecting the bugfix, I am in no hurry.

> Sat Jan 27 16:18:26 PST 2007  David Roundy <droundy at darcs.net>
>   * fix bugs in replace.sh script--running wrong darcs.

Yep. Going in.

> Sat Jan 27 16:22:06 PST 2007  David Roundy <droundy at darcs.net>
>   * fix bug triggered in replace.sh

I think this one introduces a bug:

# so far so good
darcs init
echo hej > a
darcs add a
darcs record -am hej
# uh-oh 
sed -e s/hej/xxx/ a > a.tmp; mv -f a.tmp a
darcs replace --ignore-times xxx hej a

This succeeds, and I don't think it should.  I believe the problem is
that we've become too permissive in that we treat the apply suceeding
in working as being good enough when we should really be checking that
it suceeds in both working and pristine.

Also, I instinctively agree that replace should accept --ignore-times,
but I don't understand where it's getting used.  If it is getting used,
should it maybe be used all the time?

Unrelated: I notice also a smart_diff in the Replace code.  Should it be
paranoid_diff, rather?

-- 
Eric Kow                     http://www.loria.fr/~kow
PGP Key ID: 08AC04F9         Merci de corriger mon français.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 186 bytes
Desc: not available
Url : http://lists.osuosl.org/pipermail/darcs-devel/attachments/20070128/386424ac/attachment.pgp


More information about the darcs-devel mailing list