On Mon, Jul 01, 2013 at 04:05:54PM +0300, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Mon, Jul 01, 2013 at 15:52:32 +0300:
> > No, we should extend the test to run 'svn ls' or 'svnadmin verify' to
> > ensure that creating the path with \n in it didn't break anything.
Fair enough, that's good enough for me.
But please also check that log -v returns a good list of paths.
With the FSFS \n problem, 'svn ls' was fine but 'log -v' was
broken (dirent data vs changed paths data).
Verify didn't detect the 'log -v' problem (see
http://subversion.tigris.org/issues/show_bug.cgi?id=4343).
Can you also rename allow_newlines to just 'is_fsfs', please?
Thanks!
> Here's the patch implementing that. If I don't hear otherwise, I'll
> commit and nominate for backport to 1.8.1.
>
> Daniel
>
> Index: subversion/tests/libsvn_fs/fs-test.c
> ===================================================================
> --- subversion/tests/libsvn_fs/fs-test.c (revision 1498385)
> +++ subversion/tests/libsvn_fs/fs-test.c (working copy)
> @@ -4959,10 +4959,11 @@ filename_trailing_newline(const svn_test_opts_t *o
> svn_error_t *err;
> svn_boolean_t allow_newlines;
>
> - /* Some filesystem implementations can handle newlines in filenames
> - * and can be white-listed here.
> - * Currently, only BDB supports \n in filenames. */
> - allow_newlines = (strcmp(opts->fs_type, "bdb") == 0);
> + /* The FS API wants \n to be permitted, but FSFS never implemented that,
> + * so for FSFS we expect errors rather than successes in some of the commits.
> + * Use a blacklist approach so that new FSes default to implementing the API
> + * as originally defined. */
> + allow_newlines = (!!strcmp(opts->fs_type, SVN_FS_TYPE_FSFS));
>
> SVN_ERR(svn_test__create_fs(&fs, "test-repo-filename-trailing-newline",
> opts, pool));
> @@ -4992,6 +4993,22 @@ filename_trailing_newline(const svn_test_opts_t *o
> else
> SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
>
> + if (allow_newlines)
> + {
> + static svn_test__tree_entry_t expected_entries[] = {
> + { "foo", NULL },
> + { "bar\n", NULL },
> + { "foo/baz\n", "" },
> + };
> + svn_revnum_t after_rev;
> +
> + SVN_ERR(svn_fs_commit_txn(NULL, &after_rev, txn, subpool));
> + SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(after_rev));
> +
> + SVN_ERR(svn_fs_revision_root(&root, fs, after_rev, pool));
> + SVN_ERR(svn_test__validate_tree(root, expected_entries, 3, pool));
> + }
> +
> return SVN_NO_ERROR;
> }
>
Received on 2013-07-01 15:41:14 CEST