Here is a fix for issue864 -- sort of.
Issue 864 requests that a behavior from 1.0.9 is restored in
2.1.0. I believe the behavior in 1.0.9 is wrong, and is a
regression from an earlier version of darcs.
The behavior has however not been restored in 2.1.0. Instead a
bug has been introduced that makes the decision logic for when
to require the --force flag flawed and unreliable, and the
Issue864 test exposes this bug by showing that the --force flag
is required when doing the replace on a moved file, but not when
the file is not moved.
The first patch fixes this bug, and probably restores everything
to as it was in 1.0.9.
The second patch fixes what I believe is the real regression,
but changes the way darcs has worked for a long time without
complaints from anyone. Indeed, the only complaint (as issue 864
shows) was that darcs did NOT behave in this way.
The third patch rewrites the issue864 test, as it starts to fail
again when the "fix" in patch two is applied.
I am still a proponent of patch two. It is consistent with the
documentation of Replace in the manual. The rewritten issue864
test is more or less a translation of the description in the
manual of how Replace behaves, or at least used to behave. The
--force flag has an -f shorthand so it's easy to type.
I think the problem is that users don't know about the --force
flag, and that the warning message is unhelpful. A better
thought out warning with a suggestion to use --force or -f might
learn darcs users to appreciate the warning, because it actually
means something, and darcs users should want to know when and
why they want to do a forced replace.
Sun Oct 5 01:57:19 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* resolve issue864: check non-force replace against pending
The replace was checked against pure pristine, so the answer to if it could
be applied to pending without force was sometimes wrong.
Sun Oct 5 02:03:16 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* make replace safe (again)
If the new token exists in either pending or working, the force flag must
be used, because otherwise some unrecorded changes may be destroyed when
those tokens are modified to allow the replace.
Sun Oct 5 02:52:23 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* rewrite issue864 test to check for the real cause of the bug
--
Tommy Pettersson <[EMAIL PROTECTED]>
Sun Oct 5 01:57:19 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* resolve issue864: check non-force replace against pending
The replace was checked against pure pristine, so the answer to if it could
be applied to pending without force was sometimes wrong.
Sun Oct 5 02:03:16 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* make replace safe (again)
If the new token exists in either pending or working, the force flag must
be used, because otherwise some unrecorded changes may be destroyed when
those tokens are modified to allow the replace.
Sun Oct 5 02:52:23 CEST 2008 Tommy Pettersson <[EMAIL PROTECTED]>
* rewrite issue864 test to check for the real cause of the bug
New patches:
[resolve issue864: check non-force replace against pending
Tommy Pettersson <[EMAIL PROTECTED]>**20081004235719
The replace was checked against pure pristine, so the answer to if it could
be applied to pending without force was sometimes wrong.
] move ./bugs/issue864_replace_in_moved.sh ./tests/issue864_replace_in_moved.sh
hunk ./src/Darcs/Commands/Replace.lhs 29
import Darcs.Commands
import Darcs.Arguments
import Darcs.Repository ( withRepoLock, ($-),
- add_to_pending,
+ add_to_pending, slurp_pending,
amInRepository, slurp_recorded_and_unrecorded,
applyToWorking,
)
hunk ./src/Darcs/Commands/Replace.lhs 136
unless (is_tok toks tok) $ fail $ "'"++tok++"' is not a valid token!"
checkToken old
checkToken new
- (cur, work) <- slurp_recorded_and_unrecorded repository
+ (_, work) <- slurp_recorded_and_unrecorded repository
+ cur <- slurp_pending repository
pswork <- (concatFL . unsafeFL) `liftM` sequence (map (repl toks cur work) fs)
add_to_pending repository pswork
applyToWorking repository opts pswork `catch` \e ->
[make replace safe (again)
Tommy Pettersson <[EMAIL PROTECTED]>**20081005000316
If the new token exists in either pending or working, the force flag must
be used, because otherwise some unrecorded changes may be destroyed when
those tokens are modified to allow the replace.
] hunk ./src/Darcs/Commands/Replace.lhs 152
then do putStrLn $ "Skipping file '"++f_fp++"' which isn't in the repository."
return NilFL
else if ForceReplace `elem` opts ||
- isJust (apply_to_slurpy (tokreplace f_fp toks old new) work) ||
+ isJust (apply_to_slurpy (tokreplace f_fp toks old new) work) &&
isJust (apply_to_slurpy (tokreplace f_fp toks old new) cur)
then return (get_force_replace f toks work)
else do putStrLn $ "Skipping file '"++f_fp++"'"
[rewrite issue864 test to check for the real cause of the bug
Tommy Pettersson <[EMAIL PROTECTED]>**20081005005223] hunk ./tests/issue864_replace_in_moved.sh 5
# Issue 864
-# darcs mv file1 file2
-# edit file2
-# darcs replace fails if new token includes some existing edits
-
-# Regression in darcs2 - it works in darcs1 1.0.9
+# Test that we are checking the replace against pending and not pristine.
+# Move one file in place of another without recording the moves, and
+# try to do a replace in that file.
set -ev
hunk ./tests/issue864_replace_in_moved.sh 11
+not () { "$@" && exit 1 || :; }
+
rm -rf temp
mkdir temp
cd temp
hunk ./tests/issue864_replace_in_moved.sh 18
darcs init
cat <<EOF > file1
-aa
-bb
-cc
-aa
-bb
-cc
+ aa
+ bb
+ cc
EOF
hunk ./tests/issue864_replace_in_moved.sh 22
-darcs add file1
-darcs record -a -m "0" --author X
-
-cat <<EOF > file1
-aaaa
-bb
-cc
-aa
-bb
-cc
+cat <<EOF > file2
+ xx
+ yy
+ zz
EOF
hunk ./tests/issue864_replace_in_moved.sh 27
-
-darcs replace aa aaaa file1 > out
-cat out
-grep 'Skipping file' out && exit 1
+darcs add file1 file2
+darcs record -a -m "0" --author X
hunk ./tests/issue864_replace_in_moved.sh 30
+# Simple failing replace to know our tests really works.
+
+darcs replace xx yy file2 > out
+grep 'already contains' out
+
+# Try to trick darcs
+
+darcs mv file2 file3
darcs mv file1 file2
hunk ./tests/issue864_replace_in_moved.sh 40
-cat <<EOF > file2
-aaaa
-beebee
-cc
-aa
-bb
-cc
-EOF
+# The file named file2 in recorded contains: xx yy zz
+# Before issue864 was fixed, darcs would wrongly check
+# for the new token in the recorded version of the file,
+# so this replace would fail when it should not.
hunk ./tests/issue864_replace_in_moved.sh 45
-darcs replace bb beebee file2 > out
-cat out
-grep 'Skipping file' out && exit 1
+darcs replace bb yy file2 > out
+not grep 'already contains' out
+
+# Test that chained replaces are ok (bb is now a "free" target)
+
+darcs replace aa bb file2 > out
+not grep 'already contains' out
+
+# Tests that we check for the new token in *both* pending and working.
+# (make dd a non-free target)
+
+cat <<EOF >> file2
+ dd
+EOF
+darcs replace cc dd file2 > out
+grep 'already contains' out
+
cd ..
rm -rf temp
Context:
[Add a shell test template.
Eric Kow <[EMAIL PROTECTED]>**20081003095005
This provides helper functions and a basic repository
setup. The idea is that when making a new shell test,
you start by making a copy of the template.
]
[Reformat Darcs.CommandsAux comments as haddock.
Eric Kow <[EMAIL PROTECTED]>**20080927123217]
[haddock documentation for ColorPrinter
Tommy Pettersson <[EMAIL PROTECTED]>**20081003175214]
[only show 'diffing dir' when debugging.
David Roundy <[EMAIL PROTECTED]>**20081001134124
Ignore-this: 277810d9083e36b42f27fa7ac4c47386
]
[fix test issue1110, remove duplicates of cd ..
Tommy Pettersson <[EMAIL PROTECTED]>**20081003182126
They got us out of the test dir, up in the file tree hierarchy, to the
darcs root dir, and beyond, where the test continued to run test commands
and cleanups (ooops!!)
]
[TAG 2.1.0pre3
Eric Kow <[EMAIL PROTECTED]>**20081002091241]
Patch bundle hash:
00a406eddf4c2d5b98858325e62280a125c89c86
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
