Any reviews/objections to this larger patch, now?
Thanks!
DJ
D.J. Heap wrote:
> Philip Martin wrote:
> [snip]
>
>> It's more or less an "obvious fix" :)
>>
>> Does your previous patch act as a regression tests for this?
>>
>
> Yes, I've verified that the regression test in this patch also fails
> without the fullname patch.
>
> Log:
> Fix revert to deal with svn:needs-lock property and read-only-ness.
>
> * subversion\libsvn_wc\translate.h
> (svn_wc__maybe_set_read_only): Update comment.
>
> * subversion\libsvn_wc\log.c
> (install_committed_file): Update call to svn_wc__maybe_set_read_only
> to pass did_set so possible timestamp changes are tracked correctly.
>
> * subversion\libsvn_wc\adm_ops.c
> (revert_admin_things): Add svn:needs-lock as a magic property and
> call svn_wc__maybe_set_read_only to set read-only-ness on the file
> as needed.
>
> * subversion\libsvn_wc\log.h
> (SVN_WC__LOG_MAYBE_READONLY): Update comment.
>
> * subversion\libsvn_wc\translate.c
> (svn_wc__maybe_set_read_only): Changed to look at wc lock token as
> well as svn:needs-lock property before making the file read-only.
>
> * subversion\tests\clients\cmdline\lock_tests.py
> (revert_lock): New regression test for above changes.
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_wc/translate.h
> ===================================================================
> --- subversion/libsvn_wc/translate.h (revision 14510)
> +++ subversion/libsvn_wc/translate.h (working copy)
> @@ -105,8 +105,9 @@
> svn_wc_adm_access_t *adm_access,
> apr_pool_t *pool);
>
> -/* If the SVN_PROP_NEEDS_LOCK property is present, set PATH to
> - read-write. If DID_SET is non-null, then set *DID_SET to TRUE if
> +/* If the SVN_PROP_NEEDS_LOCK property is present and there is no
> + lock token for the file in the working copy, set PATH to
> + read-only. If DID_SET is non-null, then set *DID_SET to TRUE if
> did set PATH read-write, or to FALSE if not. ADM_ACCESS is an
> access baton set that contains PATH. */
> svn_error_t * svn_wc__maybe_set_read_only (svn_boolean_t *did_set,
> Index: subversion/libsvn_wc/log.c
> ===================================================================
> --- subversion/libsvn_wc/log.c (revision 14510)
> +++ subversion/libsvn_wc/log.c (working copy)
> @@ -299,7 +299,11 @@
>
> SVN_ERR (svn_io_remove_file (tmp_wfile, pool));
>
> - SVN_ERR (svn_wc__maybe_set_read_only (NULL, filepath, adm_access, pool));
> + SVN_ERR (svn_wc__maybe_set_read_only (&did_set, filepath, adm_access, pool));
> + if (did_set)
> + /* the file may have been overwritten or its timestamp changed by
> + setting it read-only */
> + *overwrote_working = TRUE;
>
> /* Set the working file's execute bit if props dictate. */
> SVN_ERR (svn_wc__maybe_set_executable (&did_set, filepath, adm_access, pool));
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 14510)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -1246,7 +1246,8 @@
> if ((! strcmp (propchange->name, SVN_PROP_EXECUTABLE))
> || (! strcmp (propchange->name, SVN_PROP_KEYWORDS))
> || (! strcmp (propchange->name, SVN_PROP_EOL_STYLE))
> - || (! strcmp (propchange->name, SVN_PROP_SPECIAL)))
> + || (! strcmp (propchange->name, SVN_PROP_SPECIAL))
> + || (! strcmp (propchange->name, SVN_PROP_NEEDS_LOCK)))
> magic_props_changed = TRUE;
> }
> }
> @@ -1359,7 +1360,7 @@
> (err, apr_psprintf (pool, _("Error restoring text for '%s'"),
> svn_path_local_style (fullpath, pool)));
>
> - /* If necessary, tweak the new working file's executable bit. */
> + SVN_ERR (svn_wc__maybe_set_read_only (NULL, fullpath, adm_access, pool));
> SVN_ERR (svn_wc__maybe_set_executable (NULL, fullpath, adm_access,
> pool));
>
> Index: subversion/libsvn_wc/log.h
> ===================================================================
> --- subversion/libsvn_wc/log.h (revision 14510)
> +++ subversion/libsvn_wc/log.h (working copy)
> @@ -79,7 +79,8 @@
> /* Make file SVN_WC__LOG_ATTR_NAME readonly */
> #define SVN_WC__LOG_READONLY "readonly"
>
> -/* Make file SVN_WC__LOG_ATTR_NAME readonly if needs-lock property is set. */
> +/* Make file SVN_WC__LOG_ATTR_NAME readonly if needs-lock property is set
> + and there is no lock token for the file in the working copy. */
> #define SVN_WC__LOG_MAYBE_READONLY "maybe-readonly"
>
> /* Set SVN_WC__LOG_ATTR_NAME to have timestamp SVN_WC__LOG_ATTR_TIMESTAMP. */
> Index: subversion/libsvn_wc/translate.c
> ===================================================================
> --- subversion/libsvn_wc/translate.c (revision 14510)
> +++ subversion/libsvn_wc/translate.c (working copy)
> @@ -265,6 +265,15 @@
> apr_pool_t *pool)
> {
> const svn_string_t *needs_lock;
> + svn_wc_entry_t* entry;
> +
> + if (did_set)
> + *did_set = FALSE;
> +
> + SVN_ERR (svn_wc_entry (&entry, path, adm_access, FALSE, pool));
> + if ( entry && entry->lock_token )
> + return SVN_NO_ERROR;
> +
> SVN_ERR (svn_wc_prop_get (&needs_lock, SVN_PROP_NEEDS_LOCK, path,
> adm_access, pool));
>
> @@ -275,8 +284,6 @@
> if (did_set)
> *did_set = TRUE;
> }
> - else if (did_set)
> - *did_set = FALSE;
>
> return SVN_NO_ERROR;
> }
> Index: subversion/tests/clients/cmdline/lock_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/lock_tests.py (revision 14510)
> +++ subversion/tests/clients/cmdline/lock_tests.py (working copy)
> @@ -750,8 +750,87 @@
> '--password', svntest.main.wc_passwd,
> '-m', '', file_path_b)
>
> +#----------------------------------------------------------------------
> +# Tests reverting a svn:needs-lock file
> +def revert_lock(sbox):
> + "verify svn:needs-lock behavior with revert"
>
> + sbox.build()
> + wc_dir = sbox.wc_dir
>
> + iota_path = os.path.join(wc_dir, 'iota')
> +
> + mode = stat.S_IWGRP | stat.S_IWOTH | stat.S_IWRITE
> +
> + # set the prop in wc
> + svntest.actions.run_and_verify_svn(None, None, None, 'propset',
> + 'svn:needs-lock', 'foo', iota_path)
> +
> + # commit r2
> + svntest.actions.run_and_verify_svn(None, None, None, 'commit',
> + '--username', svntest.main.wc_author,
> + '--password', svntest.main.wc_passwd,
> + '-m', '', iota_path)
> +
> + # make sure that iota got set to read-only
> + if (os.stat (iota_path)[0] & mode):
> + print "Committing a file with 'svn:needs-lock'"
> + print "did not set the file to read-only"
> + raise svntest.Failure
> +
> + # verify status is as we expect
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
> + expected_status.tweak(wc_rev=1)
> + expected_status.tweak('iota', wc_rev=2)
> + svntest.actions.run_and_verify_status(wc_dir, expected_status)
> +
> + # remove read-only-ness
> + svntest.actions.run_and_verify_svn(None, None, None, 'propdel',
> + 'svn:needs-lock', iota_path)
> +
> + # make sure that iota got read-only-ness removed
> + if (os.stat (iota_path)[0] & mode == 0):
> + print "Deleting the 'svn:needs-lock' property "
> + print "did not remove read-only-ness"
> + raise svntest.Failure
> +
> + # revert the change
> + svntest.actions.run_and_verify_svn(None, None, None, 'revert', iota_path)
> +
> + # make sure that iota got set back to read-only
> + if (os.stat (iota_path)[0] & mode):
> + print "Reverting a file with 'svn:needs-lock'"
> + print "did not set the file back to read-only"
> + raise svntest.Failure
> +
> + # try propdel and revert from a different directory so
> + # full filenames are used
> + extra_name = 'xx'
> +
> + # now lock the file
> + svntest.actions.run_and_verify_svn(None, None, None, 'lock',
> + '--username', svntest.main.wc_author,
> + '--password', svntest.main.wc_passwd,
> + '-m', '', iota_path)
> +
> + # modify it
> + svntest.main.file_append(iota_path, "This line added\n")
> +
> + expected_status.tweak(wc_rev=1)
> + expected_status.tweak('iota', wc_rev=2)
> + expected_status.tweak('iota', status='M ', writelocked='K')
> + svntest.actions.run_and_verify_status(wc_dir, expected_status)
> +
> + # revert it
> + svntest.actions.run_and_verify_svn(None, None, None, 'revert', iota_path)
> +
> + # make sure it is still writable since we have the lock
> + if (os.stat (iota_path)[0] & mode == 0):
> + print "Reverting a 'svn:needs-lock' file (with lock in wc) "
> + print "did not leave the file writable"
> + raise svntest.Failure
> +
> +
> #----------------------------------------------------------------------
> def examine_lock_via_url(sbox):
> "examine the fields of a lock from a URL"
> @@ -781,7 +860,6 @@
> raise svntest.Failure
>
>
> -
> ########################################################################
> # Run the tests
>
> @@ -804,6 +882,7 @@
> lock_non_existent_file,
> out_of_date,
> update_while_needing_lock,
> + revert_lock,
> examine_lock_via_url,
> ]
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 1 19:28:51 2005