darcs

Issue 1300 darcs record --delete-logfile should only delete after a successful record.

Title darcs record --delete-logfile should only delete after a successful record.
Priority bug Status resolved
Milestone Resolved in
Superseder Nosy List Kamil, darcs-devel, dmitry.kurochkin, jaredj, kowey, markstos, thorkilnaur
Assigned To
Topics ProbablyEasy

Created on 2009-01-03.14:26:52 by markstos, last changed 2010-03-15.11:01:03 by Kamil.

Messages
msg6961 (view) Author: markstos Date: 2009-01-03.14:26:48
It appears that "darcs record --delete-log" file will delete the the log file
whether the record was successful or not (ie: all tests passed). 

So, if you do this:

darcs record --logfile=foo.txt --delete-logfile

And then the tests don't pass, you've lost foo.txt. 

My expection that is that --delete-logfile would only delete the log file after
a successful "record".
msg7651 (view) Author: kowey Date: 2009-04-09.16:05:11
Mark: do you think you could submit a test case for this?

Our current best practices live here:
http://wiki.darcs.net/index.html/RegressionTests

Thanks!
msg8732 (view) Author: kowey Date: 2009-09-07.11:22:59
Here's the test I plan to submit. Looks like this will be very easy to fix.

darcs init      --repo R        # Create our test repos.
cd R
touch f g
touch log
darcs add f
darcs setpref test 'exit 0'
darcs record -am f --test --logfile log --delete-logfile
test ! -e log
darcs setpref test 'exit 1'
touch log
darcs record -am g --test --logfile log --delete-logfile
test -e log
cd ..
msg8830 (view) Author: kowey Date: 2009-09-17.08:17:11
Resolved by

Thu Sep 17 10:03:20 CEST 2009  Kamil Dworakowski <kamil@dworakowski.name>
  * resolve issue1300: logfile deleted on unsucessful record
msg10218 (view) Author: Kamil Date: 2010-03-15.11:00:56
The following patch updated the status of issue1300 to be resolved:

* resolve issue1300: logfile deleted on unsucessful record 
Ignore-this: ead95914b26e0df6a79450a1d955abe3

Don't honour --delete-logfile when the record fails for any reason, a test
failure for instance.

I have changed the definition of get_log not to delete the logfile, but to
return it for deferred deletion. I capitalized on the fact that get_log
was already returning (Just temp_logfile) for deferred deletion.

get_log is an exported name, used in AmendRecord, Tag and Rollback. Some
of them ignore the logfile to delete, though none of them accept
--delete-logfile flag, and thus they don't need to change, nor are they
affected in any way.

Unintended side effect of the change: with --delete-logfile flag present, a
massage '"Logfile left in " ++ filepath' gets printed in case of the test
failure on record. This may actually be desirable so I did not bother to
change it.
History
Date User Action Args
2009-01-03 14:26:52markstoscreate
2009-04-09 16:05:17koweysetstatus: unread -> needs-reproduction
nosy: + jaredj
topic: + ProbablyEasy
messages: + msg7651
2009-08-25 17:37:54adminsetnosy: + darcs-devel, - simon
2009-08-27 14:16:44adminsetnosy: kowey, markstos, darcs-devel, thorkilnaur, jaredj, dmitry.kurochkin
2009-09-07 11:23:01koweysetstatus: needs-reproduction -> needs-implementation
nosy: kowey, markstos, darcs-devel, thorkilnaur, jaredj, dmitry.kurochkin
messages: + msg8732
2009-09-17 08:17:14koweysetstatus: needs-implementation -> resolved
nosy: + kamil
messages: + msg8830
2009-10-23 23:59:11adminsetnosy: + Kamil, - kamil
2010-03-15 11:01:03Kamilsetmessages: + msg10218