Index: subversion/tests/clients/cmdline/lock_tests.py =================================================================== --- subversion/tests/clients/cmdline/lock_tests.py (revision 16269) +++ subversion/tests/clients/cmdline/lock_tests.py (arbetskopia) @@ -1299,6 +1299,63 @@ print "Error: expected repository lock information not found." raise svntest.Failure +#---------------------------------------------------------------------- +def unlock_already_unlocked_files(sbox): + "(un)lock set of files, one already (un)locked" + + sbox.build() + wc_dir = sbox.wc_dir + + # Deliberately have no direct child of A as a target + iota_path = os.path.join(wc_dir, 'iota') + lambda_path = os.path.join(wc_dir, 'A', 'B', 'lambda') + alpha_path = os.path.join(wc_dir, 'A', 'B', 'E', 'alpha') + gamma_path = os.path.join(wc_dir, 'A', 'D', 'gamma') + + svntest.actions.run_and_verify_svn(None, None, [], 'lock', + '--username', svntest.main.wc_author2, + '--password', svntest.main.wc_passwd, + '--no-auth-cache', + '-m', 'lock several', + iota_path, lambda_path, alpha_path) + + expected_status = svntest.actions.get_virginal_state(wc_dir, 1) + expected_status.tweak('iota', 'A/B/lambda', 'A/B/E/alpha', writelocked='K') + svntest.actions.run_and_verify_status(wc_dir, expected_status) + + error_msg = ".*Path '/A/B/E/alpha' is already locked by user '" + \ + svntest.main.wc_author2 + "'.*" + svntest.actions.run_and_verify_svn(None, None, error_msg, + 'lock', + '--username', svntest.main.wc_author2, + '--password', svntest.main.wc_passwd, + '--no-auth-cache', + alpha_path, gamma_path) + expected_status.tweak('A/D/gamma', writelocked='K') + svntest.actions.run_and_verify_status(wc_dir, expected_status) + + svntest.actions.run_and_verify_svn(None, None, [], 'unlock', + '--username', svntest.main.wc_author2, + '--password', svntest.main.wc_passwd, + '--no-auth-cache', + lambda_path) + + expected_status.tweak('A/B/lambda', writelocked=None) + svntest.actions.run_and_verify_status(wc_dir, expected_status) + + error_msg = ".*No lock on path '/A/B/lambda'.*" + svntest.actions.run_and_verify_svn(None, None, error_msg, + 'unlock', + '--username', svntest.main.wc_author2, + '--password', svntest.main.wc_passwd, + '--no-auth-cache', + '--force', + iota_path, lambda_path, alpha_path) + + + expected_status.tweak('iota', 'A/B/E/alpha', writelocked=None) + svntest.actions.run_and_verify_status(wc_dir, expected_status) + ######################################################################## # Run the tests @@ -1332,6 +1389,7 @@ Skip(lock_and_exebit2, (os.name != 'posix')), commit_xml_unsafe_file_unlock, repos_lock_with_info, + unlock_already_unlocked_files, ] if __name__ == '__main__': Index: subversion/libsvn_ra_svn/client.c =================================================================== --- subversion/libsvn_ra_svn/client.c (revision 16269) +++ subversion/libsvn_ra_svn/client.c (arbetskopia) @@ -1553,15 +1553,13 @@ { ra_svn_session_baton_t *sess = session->priv; svn_ra_svn_conn_t *conn = sess->conn; - apr_array_header_t *list; apr_hash_index_t *hi; - int i; svn_ra_svn_item_t *elt; svn_error_t *err, *callback_err = NULL; apr_pool_t *subpool = svn_pool_create(pool); const char *status; - apr_array_header_t *condensed_tgt_paths; - condensed_tgt_paths = apr_array_make(pool, 1, sizeof(const char *)); + svn_lock_t *lock; + apr_array_header_t *list = NULL; SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w((?c)b(!", "lock-many", comment, steal_lock)); @@ -1576,11 +1574,9 @@ svn_pool_clear(subpool); apr_hash_this(hi, &key, NULL, &val); path = key; - APR_ARRAY_PUSH(condensed_tgt_paths, const char *) = path; revnum = val; - SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?r)", - path, *revnum)); + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?r)", path, *revnum)); } SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!))")); @@ -1590,50 +1586,74 @@ /* Pre-1.3 servers don't support 'lock-many'. If that fails, fall back * to 'lock'. */ if (err && err->apr_err == SVN_ERR_RA_SVN_UNKNOWN_CMD) - return ra_svn_lock_compat(session, path_revs, comment, steal_lock, - lock_func, lock_baton, pool); + { + svn_error_clear(err); + return ra_svn_lock_compat(session, path_revs, comment, steal_lock, + lock_func, lock_baton, pool); + } - /* Unknown error */ if (err) return err; - /* svn_ra_svn_read_cmd_response() is unusable as it parses the params, - * instead of returning a list over which to iterate. This means - * failure status must be handled explicitly. */ - err = svn_ra_svn_read_tuple(conn, pool, "wl", &status, &list); - if (strcmp(status, "failure") == 0) - return svn_ra_svn__handle_failure_status(list, pool); + /* Loop over responses to get lock tokens. */ + for (hi = apr_hash_first(pool, path_revs); hi; hi = apr_hash_next(hi)) + { + const void *key; + const char *path; - if (err && !SVN_ERR_IS_LOCK_ERROR(err)) - return err; + apr_hash_this(hi, &key, NULL, NULL); + path = key; - for (i = 0; i < list->nelts; ++i) - { - svn_lock_t *lock; - const char *condensed_tgt_path; - svn_pool_clear(subpool); - condensed_tgt_path = APR_ARRAY_IDX(condensed_tgt_paths, i, const char *); - elt = &APR_ARRAY_IDX(list, i, svn_ra_svn_item_t); + SVN_ERR(svn_ra_svn_read_item(conn, subpool, &elt)); + if (elt->kind == SVN_RA_SVN_WORD && strcmp(elt->u.word, "done") == 0) + break; + if (elt->kind != SVN_RA_SVN_LIST) return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, - _("Lock element not a list")); + _("Lock response not a list")); - err = parse_lock(elt->u.list, subpool, &lock); - if (err) - return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, err, - _("Unable to parse lock data")); + SVN_ERR(svn_ra_svn_parse_tuple(elt->u.list, subpool, "wl", &status, + &list)); + if (strcmp(status, "failure") == 0) + err = svn_ra_svn__handle_failure_status(list, subpool); + else if (strcmp(status, "success") == 0) + { + SVN_ERR(parse_lock(list, subpool, &lock)); + err = NULL; + } + else + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, + _("Unknown status for lock command")); + if (lock_func) - callback_err = lock_func(lock_baton, condensed_tgt_path, TRUE, + callback_err = lock_func(lock_baton, path, TRUE, err ? NULL : lock, err, subpool); + else + callback_err = SVN_NO_ERROR; + svn_error_clear(err); + if (callback_err) return callback_err; } + /* If we didn't break early above, and the whole hash was traversed, + read the final "done" from the server. */ + if (!hi) + { + SVN_ERR(svn_ra_svn_read_item(conn, pool, &elt)); + if (elt->kind != SVN_RA_SVN_WORD || strcmp(elt->u.word, "done") != 0) + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, + _("Didn't receive end marker for lock " + "responses")); + } + + SVN_ERR(svn_ra_svn_read_cmd_response(conn, pool, "")); + svn_pool_destroy(subpool); return SVN_NO_ERROR; @@ -1653,14 +1673,17 @@ apr_hash_index_t *hi; apr_pool_t *subpool = svn_pool_create(pool); svn_error_t *err, *callback_err = NULL; + svn_ra_svn_item_t *elt; + const char *status = NULL; + apr_array_header_t *list = NULL; + const void *key = NULL; + const char *path = NULL; SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(b(!", "unlock-many", break_lock)); for (hi = apr_hash_first(pool, path_tokens); hi; hi = apr_hash_next(hi)) { - const void *key; - const char *path; void *val; const char *token; @@ -1672,8 +1695,8 @@ token = val; else token = NULL; - - SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?c)", path,token)); + + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?c)", path, token)); } SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!))")); @@ -1684,33 +1707,72 @@ * to 'unlock'. */ if (err && err->apr_err == SVN_ERR_RA_SVN_UNKNOWN_CMD) - return ra_svn_unlock_compat(session, path_tokens, break_lock, lock_func, - lock_baton, pool); + { + svn_error_clear(err); + return ra_svn_unlock_compat(session, path_tokens, break_lock, lock_func, + lock_baton, pool); + } if (err) return err; - err = svn_ra_svn_read_cmd_response(conn, pool, ""); - - if (err && !SVN_ERR_IS_UNLOCK_ERROR(err)) - return err; - + /* Loop over responses to unlock files. */ + hi = apr_hash_first(pool, path_tokens); for (hi = apr_hash_first(pool, path_tokens); hi; hi = apr_hash_next(hi)) { - const void *key; - const char *path; + svn_pool_clear(subpool); - svn_pool_clear(subpool); + SVN_ERR(svn_ra_svn_read_item(conn, subpool, &elt)); + + if (elt->kind == SVN_RA_SVN_WORD && (strcmp(elt->u.word, "done") == 0)) + break; + apr_hash_this(hi, &key, NULL, NULL); path = key; + if (elt->kind != SVN_RA_SVN_LIST) + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, + _("Unlocked path not parseable")); + + SVN_ERR(svn_ra_svn_parse_tuple(elt->u.list, subpool, "wl", &status, + &list)); + + if (strcmp(status, "failure") == 0) + err = svn_ra_svn__handle_failure_status(list, subpool); + else if (strcmp(status, "success") == 0) + { + SVN_ERR(svn_ra_svn_parse_tuple(list, subpool, "c", &path)); + err = SVN_NO_ERROR; + } + else + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, + _("Unknown status for unlock command")); + if (lock_func) - callback_err = lock_func(lock_baton, path, FALSE, NULL, err, subpool); + callback_err = lock_func(lock_baton, path, FALSE, NULL, err, + subpool); + else + callback_err = SVN_NO_ERROR; + svn_error_clear(err); + if (callback_err) return callback_err; } + /* If we didn't break early above, and the whole hash was traversed, + read the final "done" from the server. */ + if (!hi) + { + SVN_ERR(svn_ra_svn_read_item(conn, pool, &elt)); + if (elt->kind != SVN_RA_SVN_WORD || strcmp(elt->u.word, "done") != 0) + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, + _("Didn't receive end marker for unlock " + "responses")); + } + + SVN_ERR(svn_ra_svn_read_cmd_response(conn, pool, "")); + svn_pool_destroy(subpool); return SVN_NO_ERROR; Index: subversion/libsvn_ra_svn/protocol =================================================================== --- subversion/libsvn_ra_svn/protocol (revision 16269) +++ subversion/libsvn_ra_svn/protocol (arbetskopia) @@ -337,7 +337,11 @@ lock-many params: ( [ comment:string ] steal-lock:bool ( (path:string [ current-rev:number ] ) ... ) ) - response: ( ( lock:lockdesc ) ... ) + Before sending response, server sends lock cmd status and descriptions, + ending with "done". + lock-info: ( success ( lock:lockdesc ) ) | ( failure ( err:error ) ) + | done + response: ( ) unlock params: ( path:string [ token:string ] break-lock:bool ) @@ -345,6 +349,9 @@ unlock-many params: ( break-lock:bool ( ( path:string [ token:string ] ) ... ) ) + Before sending response, server sends unlocked paths, ending with "done". + pre-response: ( success ( path:string ) ) | ( failure ( err:error ) ) + | done response: ( ) get-lock Index: subversion/svnserve/serve.c =================================================================== --- subversion/svnserve/serve.c (revision 16269) +++ subversion/svnserve/serve.c (arbetskopia) @@ -1594,23 +1594,21 @@ apr_array_header_t *params, void *baton) { server_baton_t *b = baton; - apr_array_header_t *locks, *lock_cmds; + apr_array_header_t *path_revs; const char *comment; svn_boolean_t steal_lock; - int i; + apr_size_t i; apr_pool_t *subpool; - struct lock_cmd { - const char *path; - const char *full_path; - svn_revnum_t current_rev; - svn_lock_t *l; - }; + const char *path; + const char *full_path; + svn_revnum_t current_rev; + svn_lock_t *l; + svn_error_t *err = NULL, *write_err; SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?c)bl", &comment, &steal_lock, - &locks)); + &path_revs)); subpool = svn_pool_create(pool); - lock_cmds = apr_array_make(pool, locks->nelts, sizeof(struct lock_cmd)); /* Because we can only send a single auth reply per request, we send a reply before parsing the lock commands. This means an authz @@ -1618,57 +1616,66 @@ an error. */ SVN_ERR(must_have_access(conn, pool, b, svn_authz_write, NULL, TRUE)); - /* Loop through the lock commands. */ - for (i = 0; i < locks->nelts; ++i) + /* Loop through the lock requests. */ + for (i = 0; i < path_revs->nelts; ++i) { - struct lock_cmd *cmd = apr_pcalloc(pool, sizeof(struct lock_cmd)); - svn_ra_svn_item_t *item = &APR_ARRAY_IDX(locks, i, svn_ra_svn_item_t); + svn_ra_svn_item_t *item = &APR_ARRAY_IDX(path_revs, i, + svn_ra_svn_item_t); + svn_pool_clear(subpool); + if (item->kind != SVN_RA_SVN_LIST) return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, - "Lock commands should be list of lists"); + "Lock requests should be list of lists"); - SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, pool, "c(?r)", - &cmd->path, &cmd->current_rev)); + SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, pool, "c(?r)", &path, + ¤t_rev)); - cmd->full_path = svn_path_join(b->fs_path, - svn_path_canonicalize(cmd->path, subpool), - pool); + full_path = svn_path_join(b->fs_path, + svn_path_canonicalize(path, subpool), + subpool); - if (! lookup_access(pool, b, svn_authz_write, cmd->full_path, TRUE)) + if (! lookup_access(pool, b, svn_authz_write, full_path, TRUE)) return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL, NULL), NULL); - APR_ARRAY_PUSH(lock_cmds, struct lock_cmd) = *cmd; - } + err = svn_repos_fs_lock(&l, b->repos, full_path, + NULL, comment, FALSE, + 0, /* No expiration time. */ + current_rev, + steal_lock, pool); - /* Loop through each path to be locked. */ - for (i = 0; i < lock_cmds->nelts; i++) - { - struct lock_cmd *cmd = &APR_ARRAY_IDX(lock_cmds, i, struct lock_cmd); - - SVN_CMD_ERR(svn_repos_fs_lock(&cmd->l, b->repos, cmd->full_path, - NULL, comment, 0, - 0, /* No expiration time. */ - cmd->current_rev, - steal_lock, pool)); + if (err) + { + if (SVN_ERR_IS_LOCK_ERROR(err)) + { + write_err = svn_ra_svn_write_cmd_failure(conn, pool, err); + svn_error_clear(err); + err = NULL; + SVN_ERR(write_err); + } + else + break; + } + else + { + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "w!", "success")); + SVN_ERR(write_lock(conn, subpool, l)); + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "!")); + } } - SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(!", "success")); + svn_pool_destroy(subpool); - for (i = 0; i < lock_cmds->nelts; i++) - { - struct lock_cmd *cmd = &APR_ARRAY_IDX(lock_cmds, i, struct lock_cmd); + /* NOTE: err might contain a fatal locking error from the loop above. */ + write_err = svn_ra_svn_write_word(conn, pool, "done"); + if (!write_err) + SVN_CMD_ERR(err); + svn_error_clear(err); + SVN_ERR(write_err); + SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, "")); - svn_pool_clear(subpool); - SVN_ERR(write_lock(conn, subpool, cmd->l)); - } - - SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)")); - - svn_pool_destroy(subpool); - return SVN_NO_ERROR; } @@ -1685,7 +1692,7 @@ full_path = svn_path_join(b->fs_path, svn_path_canonicalize(path, pool), pool); - /* Username required unless force was specified. */ + /* Username required unless break_lock was specified. */ SVN_ERR(must_have_access(conn, pool, b, svn_authz_write, full_path, ! break_lock)); @@ -1702,29 +1709,25 @@ { server_baton_t *b = baton; svn_boolean_t break_lock; - apr_array_header_t *unlock_tokens, *unlock_cmds; + apr_array_header_t *unlock_tokens; int i; apr_pool_t *subpool; - struct unlock_cmd { - const char *path; - const char *full_path; - const char *token; - }; + const char *path; + const char *full_path; + const char *token; + svn_error_t *err = NULL, *write_err; SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "bl", &break_lock, &unlock_tokens)); - unlock_cmds = - apr_array_make(pool, unlock_tokens->nelts, sizeof(struct unlock_cmd)); - - /* Username required unless force was specified. */ + /* Username required unless break_lock was specified. */ SVN_ERR(must_have_access(conn, pool, b, svn_authz_write, NULL, ! break_lock)); subpool = svn_pool_create(pool); - /* Loop through the unlock commands. */ + + /* Loop through the unlock requests. */ for (i = 0; i < unlock_tokens->nelts; i++) { - struct unlock_cmd *cmd = apr_pcalloc(pool, sizeof(struct unlock_cmd)); svn_ra_svn_item_t *item = &APR_ARRAY_IDX(unlock_tokens, i, svn_ra_svn_item_t); @@ -1732,39 +1735,49 @@ if (item->kind != SVN_RA_SVN_LIST) return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL, - "Unlock command should be a list of lists"); + "Unlock request should be a list of lists"); - SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, subpool, "c(?c)", - &cmd->path, &cmd->token)); + SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, subpool, "c(?c)", &path, + &token)); - cmd->full_path = svn_path_join(b->fs_path, - svn_path_canonicalize(cmd->path, subpool), - pool); + full_path = svn_path_join(b->fs_path, + svn_path_canonicalize(path, subpool), + subpool); - if (! lookup_access(pool, b, svn_authz_write, cmd->full_path, + if (! lookup_access(subpool, b, svn_authz_write, full_path, ! break_lock)) return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL, NULL), NULL); - APR_ARRAY_PUSH(unlock_cmds, struct unlock_cmd) = *cmd; + err = svn_repos_fs_unlock(b->repos, full_path, token, break_lock, + subpool); + if (err) + { + if (SVN_ERR_IS_UNLOCK_ERROR(err)) + { + write_err = svn_ra_svn_write_cmd_failure(conn, pool, err); + svn_error_clear(err); + err = NULL; + SVN_ERR(write_err); + } + else + break; + } + else + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "w(c)", "success", + path)); } - /* Loop through each path to be unlocked. */ - for (i = 0; i < unlock_cmds->nelts; i++) - { - struct unlock_cmd *cmd = &APR_ARRAY_IDX(unlock_cmds, i, - struct unlock_cmd); + svn_pool_destroy(subpool); - svn_pool_clear(subpool); - SVN_CMD_ERR(svn_repos_fs_unlock(b->repos, cmd->full_path, - cmd->token, break_lock, subpool)); - } - + /* NOTE: err might contain a fatal unlocking error from the loop above. */ + write_err = svn_ra_svn_write_word(conn, pool, "done"); + if (! write_err) + SVN_CMD_ERR(err); + svn_error_clear(err); SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, "")); - svn_pool_destroy(subpool); - return SVN_NO_ERROR; }