> On Dec. 4, 2013, 10:18 p.m., Andrew Stitcher wrote: > > I can see no actual justification for this change in either the review > > comments or the refered bug: > > > > There is nothing per se wrong with this change, I just don't really > > understand why it necessary. > > > > Incidentally '/' works perfectly well as a path separator on windows too > > (even if it is not the default). It is true that the test for an absolute > > path would be different though. > > > > Alan Conway wrote: > It's required because AclPlugin falls over if you give it an aclFile of > the form c:\path\to\aclfile. It currently tests for absolute path with > path[0]=='/'. > The rest of Path is not immediately essential but I though we could use a > place to collect other path-related portability fixes in the future. I looked > at FileSysDir but it's quite specifically for directories. We could refactor > FileSysDir as a subclass of Path if you think that's worthwhile. > > This came up in the context of work on portable python-based tests, but I > wanted to separate this out because it stands alone. I can create another bug > for it if need be.
I think a new bug is definitely required. In fact I'd go so far as to say this is a very serious defect. in the code we are about to release (and have released come to that). - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16018/#review29765 ----------------------------------------------------------- On Dec. 4, 2013, 10:06 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16018/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2013, 10:06 p.m.) > > > Review request for qpid, Andrew Stitcher and Steve Huston. > > > Bugs: QPID-5307 > https://issues.apache.org/jira/browse/QPID-5307 > > > Repository: qpid > > > Description > ------- > > QPID-5307: Added portable Path, check for isAbsolute() > > qpid::sys::Path provides some basic operations on path names, > with implementations for posix and windows. > > Needed by work on a portable test system. > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1546628 > /trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1546628 > /trunk/qpid/cpp/src/qpid/sys/Path.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/sys/posix/Path.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/sys/windows/Path.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/16018/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
