On Mon, Nov 28, 2011 at 10:12:38PM -0600, Hyrum K Wright wrote:
> On Mon, Nov 28, 2011 at 6:58 PM, Philip Martin
> <philip.martin_at_wandisco.com> wrote:
> > stsp_at_apache.org writes:
> >
> >> Author: stsp
> >> Date: Mon Nov 28 22:34:49 2011
> >> New Revision: 1207663
> >
> >> +def patch_target_no_eol_at_eof(sbox):
> >> + "patch target with no eol at eof"
> >> +
> >> + sbox.build()
> >> + wc_dir = sbox.wc_dir
> >> +
> >> + patch_file_path = make_patch_path(sbox)
> >> + iota_path = os.path.join(wc_dir, 'iota')
> >> +
> >> + iota_contents = [
> >> + "This is the file iota."
> >> + ]
> >> +
> >> + svntest.main.file_write(iota_path, ''.join(iota_contents))
> >> + expected_output = svntest.wc.State(wc_dir, {
> >> + 'iota' : Item(verb='Sending'),
> >> + })
> >> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> >> + expected_status.tweak('iota', wc_rev=2)
> >> + svntest.actions.run_and_verify_commit(wc_dir, expected_output,
> >> + expected_status, None, wc_dir)
> >> + unidiff_patch = [
> >> + "--- iota\t(revision 1)\n",
> >> + "+++ iota\t(working copy)\n",
> >> + "@@ -1,7 +1,7 @@\n",
> >> + "-This is the file iota.\n"
> >> + "\\ No newline at end of file\n",
> >> + "+It is the file 'iota'.\n",
> >> + "\\ No newline at end of file\n",
> >> + ]
> >> +
> >> + svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
> >> +
> >> + iota_contents = [
> >> + "It is the file 'iota'."
> >> + ]
> >> + expected_output = [
> >> + 'U %s\n' % os.path.join(wc_dir, 'iota'),
> >> + ]
> >> +
> >> + expected_disk = svntest.main.greek_state.copy()
> >> + expected_disk.tweak('iota', contents=''.join(iota_contents))
> >> +
> >> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> >> + expected_status.tweak('iota', status='M ', wc_rev=2)
> >> +
> >> + expected_skip = wc.State('', { })
> >> +
> >> + svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
> >> + expected_output,
> >> + expected_disk,
> >> + expected_status,
> >> + expected_skip,
> >> + None, # expected err
> >> + 1, # check-props
> >> + 1) # dry-run
> >
> > This test failed on the buildbot: 'iota' not showing status 'M'. I
> > believe this is because there is no sleep between the commit allowing
> > the patching to be fast enough that the file's timestamp doesn't change
> > and the file shows as unmodified. I can demonstrate that it is a
> > timestamp problem using:
> >
> > Index: subversion/tests/cmdline/patch_tests.py
> > ===================================================================
> > --- subversion/tests/cmdline/patch_tests.py (revision 1207719)
> > +++ subversion/tests/cmdline/patch_tests.py (working copy)
> > @@ -34,6 +34,7 @@
> > import textwrap
> > import zlib
> > import posixpath
> > +import time
> >
> > # Our testing module
> > import svntest
> > @@ -4036,6 +4037,10 @@
> >
> > expected_skip = wc.State('', { })
> >
> > + svntest.actions.run_and_verify_svn(None, None, [], 'up', '-r', '1', wc_dir)
> > + svntest.actions.run_and_verify_svn(None, None, [], 'up', sbox.ospath('iota'))
> > + time.sleep(0.1)
> > +
> > svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
> > expected_output,
> > expected_disk,
> >
> > We need to sleep whenever a test relies on timestamp changes and in this
> > test there are two places: between the initial checkout and the
> > file_write, and between the commit and the patch.
> >
> > Looking at the rest of the file I don't see do_sleep_for_timestamps or
> > explicit sleeps in any of the tests. I suspect some of the others are
> > vulnerable to similar races.
>
> It might also be possible to write an annotation for the test that
> configures the environment to enable the sleep within the libraries
> for that particular test. It would be a bit less granular (on the
> order of a test, not a specific command), but would be easier to
> maintain once the initial plumbing is in place.
That is a good idea.
In the meantime, maybe something like below will help?
Note that 'svn patch' doesn't sleep for timestamps at the moment
so the alternative would be adding calls to sleep() to the tests
which isn't all that pretty.
[[[
* subversion/libsvn_client/patch.c
(apply_patches): Sleep for timestamps after applying the patch.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_patch): Enable sleep for timestamps during 'svn patch'
invocation.
]]]
Index: subversion/tests/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/cmdline/svntest/actions.py (revision 1207768)
+++ subversion/tests/cmdline/svntest/actions.py (working copy)
@@ -1180,7 +1180,9 @@ def run_and_verify_patch(dir, patch_path,
# Update and make a tree of the output.
patch_command = patch_command + args
+ do_sleep_for_timestamps()
exit_code, out, err = main.run_svn(True, *patch_command)
+ no_sleep_for_timestamps()
if error_re_string:
rm = re.compile(error_re_string)
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (revision 1207768)
+++ subversion/libsvn_client/patch.c (working copy)
@@ -2926,6 +2926,8 @@ apply_patches(/* The path to the patch file. */
SVN_ERR(svn_diff_close_patch_file(patch_file, iterpool));
svn_pool_destroy(iterpool);
+ svn_io_sleep_for_timestamps(abs_wc_path, scratch_pool);
+
return SVN_NO_ERROR;
}
Received on 2011-11-29 08:04:39 CET