Re: [lmi] boost::filesystem anomaly?

[Vadim Zeitlin]

On Sat, 11 Jun 2016 17:03:00 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2016-06-11 15:48, Vadim Zeitlin wrote:
...
GC> > trouble is that there are 2 copies of boost_filesystem library in the
GC> > lmi_wx executable. Because of this, fs::no_check used above is not the
GC> > same no_check object that the no_check defined in path_posix_windows.cpp
GC> > that the code in path::m_path_append() compares the checker with, hence 
the
GC> > check for Windows paths is completely skipped, resulting in the bug.
GC> 
GC> I'm really glad you found that,

 Actually, I found wrong, sorry. There is a single function in liblmi.dll,
but the trouble is that there is the real function and its import library
trampoline, which, of course, has a different address. And someone had
found it more than 10 years before me, see

        https://sourceforge.net/p/mingw/mailman/message/16207966/

GC> Okay, I'll wait for your fix.

 The problem is that I have 2 ideas about how to fix this and I'm not sure
which one would be least worst (I can't really call neither of them "best"
with straight face). The first one is to define BOOST_FILESYSTEM_DYN_LINK
and BOOST_FILESYSTEM_SOURCE when building liblmi.dll and also define the
former when building lmi_wx target. This should solve the problem because
a pointer to a DLL-exported function really does point to it (and not to
the trampoline used to jump to it), but is rather brittle and a little
ugly.

 The second one is to modify Boost.Filesystem sources to add an accessor
for no_check, e.g. something similar to this patch:

        http://lists.boost.org/boost-users/att-15643/patch_native.diff

linked from a thread about the same problem on Boost mailing list. This is
arguably simpler, but more error-prone as it would still be possible to use
no_check directly (and wrongly).

 On balance, as much as I don't like modifying lmi build system, I think
the first solution is preferable. What do you think?


GC> BTW, this snippet from 'objects.make'...
GC> 
GC> ----------8<----------
GC> # These object files are used in both an application and a shared
GC> # library that it links to, only for builds that use shared-library
GC> # 'attributes'. This workaround is used merely because we don't yet
GC> # build these objects as a library. TODO ?? The duplication is not
GC> # correct: it validates linking, but the linked applications don't
GC> # run correctly.
GC> 
GC> ifneq (,$(USE_SO_ATTRIBUTES))
GC>   duplicated_objects = $(boost_filesystem_objects) $(xmlwrapp_objects)
GC> endif
GC> ---------->8----------
GC> 
GC> looks like a possible culprit, but it's used only by
GC>   cgi_objects
GC>   cli_objects
GC>   skeleton_objects
GC> so it doesn't seem to account for duplication in the lmi-wx binary.

 Yes, I saw this too and had the same thought and arrived to the same
conclusion. I still don't understand the comment nor the meaning and
intended usage of USE_SO_ATTRIBUTES in lmi more globally...


GC> >  BTW, if we updated to a newer (and more std::filesystem-compatible)
GC> > version of the Boost.Filesystem library which doesn't have the concept of
GC> > "checkers" any more, we wouldn't have to spend time on dealing with this
GC> > problem...
GC> 
GC> I think we should replace boost::regex with std::regex first (and test
GC> the replacement).

 FWIW replacing boost::regex with std::regex is trivial, they have exactly
the same API. However a couple of regexes will need to be adjusted because
C++11 std::regex doesn't support Perl extended syntax for some reason. A
simple grep finds only 4 occurrences of it and 3 of them use the syntax
which is supported by ECMAScript regexes and hence C++11, the only problem
is the use of "(?-s)" to turn off the "s" flag in regex_test.cpp, so this
test would need to be modified to avoid using it, but it shouldn't be a
problem.

GC> Then we'd be using only one non-header boost library,
GC> filesystem--unless they've changed other header-only libraries we use
GC> to require separate compilation.

 No, they didn't. But while I'm all for getting rid of Boost.Regex, there
is no particular urgency to it. Updating Boost.Filesystem would, however,
fix (well, avoid) the problem that we're currently facing.


 Please let me know if you have any preference to either of the solutions
above or see another one I'm missing and I'll make a patch implementing it.

 Thanks in advance,
VZ