C. Michael Pilato wrote on Fri, May 10, 2013 at 18:19:56 -0400: > On 05/10/2013 04:51 PM, Daniel Shahaf wrote: > > For issue #4340, we decided to block filenames containing \n in FSFS and > > filenames containing control characters in libsvn_repos. (I agree with > > that decision.) > > > > However, those changes are now nominated for backport towards 1.7.10 in > > 1.7.x/STATUS, and I voted -0 on them, saying that "[the new] libsvn_repos > > restrictions [are] not suitable for introduction in a patch release" --- > > that is, that libsvn_repos-1.7.10 should not forbid creating fspaths that > > contain control characters, because libsvn_repos-1.7.9 doesn't. > > > > Stefan asked to start a thread about that -0 vote, so here it is :) > > If you look at the "general idea" behind our versioning guidelines[1], the > one relevant goal of the three presented is this one: > > "Upgrading/downgrading between different patch releases in the same > MAJOR.MINOR line never breaks code." >
Agreed. > So long as we don't introduce new APIs or break the calling conventions of > existing ones to accomplish this change, disallowing certain characters in > repository paths does not affect the upgrade/downgrade-ability of the Disagree. Adding a validation to 1.7.10 that didn't exist in 1.7.9 means that client code that worked with a 1.7.9 libsvn would stop working[1] when run with 1.7.10 libsvn. That is precisely "Upgrading within the same minor line never breaks code", isn't it? > codebase. Further, we're merely guarding our entry points against these > troublesome paths, not croaking when we find them already in the repository. Agreed. > As such, it's *not* the case that users find themselves with a new freedom > in 1.7.10 that, if exercised, will cause a downgrade to 1.7.9 to render > their repositories inaccessible. > Agreed again, downgrading would not cause a problem. > In summary: I think the backport is fine. > > Concession: It could be argued that the no-"\n"-in-FSFS change is > independent from the no-control-chars-in-libsvn_repos change. The former is > trivially defensible as a bugfix to guard against a demonstrable data > storage problem. The latter change is, IMO, the only possible controversial > one for a patch release. While I think both backports are fine, I wouldn't > strongly defend the latter. > I already voted +1 on the "No '\n' in FSFS" change; I only have concerns about the "No control-chars in libsvn_repos" change. > Thanks for the feedback. Cheers, Daniel [1] Specifically: svn_repos_fs_commit_txn() would start returning an SVN_ERR_FS_PATH_SYNTAX error instead of succeeding.
