[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Revert and svn:needs-lock

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-04-29 00:43:07 CEST

"D.J. Heap" <dj@shadyvale.net> writes:

> --- subversion/libsvn_wc/adm_ops.c (revision 14500)
> +++ 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,6 +1360,9 @@
> (err, apr_psprintf (pool, _("Error restoring text for '%s'"),
> svn_path_local_style (fullpath, pool)));
>
> + /* If necessary, tweak the read-only-ness of the file. */

I'd leave out that comment, it doesn't add anything.

> + SVN_ERR (svn_wc__maybe_set_read_only (NULL, fullpath, adm_access, pool));
> +
> /* If necessary, tweak the new working file's executable bit. */

Yes, I know you copied it from here, I'd probably delete that one as well :)

> SVN_ERR (svn_wc__maybe_set_executable (NULL, fullpath, adm_access,
> pool));
> Index: subversion/libsvn_wc/translate.c
> ===================================================================
> --- subversion/libsvn_wc/translate.c (revision 14500)
> +++ 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;

Need to document this change in behaviour in translate.h, and check
all uses of the function still make sense.

As per your earlier mail the log.c:install_committed_file use looks
like it should pass did_set.

The log.c:log_do_file_readonly is a bit tricky. Need to change the
SVN_WC__LOG_MAYBE_READONLY comment to mention the lock, just like
translate.h. SVN_WC__LOG_MAYBE_READONLY is only used in one place,
when the lock has been removed so I think it's OK.

> +
> 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 14500)
> +++ subversion/tests/clients/cmdline/lock_tests.py (working copy)
> @@ -750,8 +750,70 @@
> '--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.main.run_svn(None, 'propset', 'svn:needs-lock', 'foo', iota_path)
> +
> + # commit r2
> + svntest.main.run_svn(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
> +
> + # remove read-only-ness
> + svntest.main.run_svn(None, 'propdel', 'svn:needs-lock', iota_path)
> +

Please avoid run_svn if possible, it does very little error checking,
run_and_verify_svn is better.

Use run_and_verify_status (all through the test) to verify the
meta-data changes as well as the file changes.

> + # 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.main.run_svn(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
> +
> + # now lock the file
> + svntest.main.run_svn(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")
> +

Definitely need to check status here!

> + # revert it
> + svntest.main.run_svn(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 +843,6 @@
> raise svntest.Failure
>
>
> -
> ########################################################################
> # Run the tests
>
> @@ -804,6 +865,7 @@
> lock_non_existent_file,
> out_of_date,
> update_while_needing_lock,
> + revert_lock,
> examine_lock_via_url,
> ]
>

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 29 00:44:30 2005

This is an archived mail posted to the Subversion Dev mailing list.