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; > } >
