Boost.DLL formal review is ongoing
The formal review of Boost.DLL library by Antony Polukhin is nearing the end of the first week. As of today, over 100 people read through the library's tutorial, suggesting quite some interest. However, no reviews were submitted yet. Please take the time to post your thoughts - even if you don't have time to do a full review, or try the library, comments on design and interfaces are still very valuable. The summary of the library features and review checklists are reproduced below. Boost.DLL is a C++98 library for comfortable work with DLL and DSO. Library provides a portable across platforms way to: - load libraries at runtime - import and export any native functions and variables - make alias names for C++ mangled functions and symbols - query libraries/objects and executables for sections and exported symbols - self loading and self querying - getting program and module location by exported symbol The documentation can be found at: http://apolukhin.github.io/Boost.DLL/index.html and the source can be obtained at: https://github.com/apolukhin/Boost.DLL Please post your reviews on the mailing list and if possible, answer the following questions: - Should the library be accepted? - How useful is it? - What's your evaluation of - Design - Implementation - Documentations - Tests - How much effort did you put into your evaluation? - Did you attempt to use the library? On what systems and compilers? Thanks, Volodya _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hi I am attempting to get to grips with DLL to see if I can do a review. I have cloned the source code and run the tests. I am struggling with the documentation as I cannot find instructions how to compile and link the example files: getting_started.cpp getting_started_library.cpp shared_lib_path.hpp Should getting_started_library.cpp be put into a shared library? I have not found a makefile. I am using clang 3.6 and libc++ on Ubuntu Linux. Thanks John
________________________________________ From: Boost [boost-bounces@lists.boost.org] on behalf of Fletcher, John P [j.p.fletcher@aston.ac.uk] Sent: 03 July 2015 18:03 To: boost@lists.boost.org Subject: Re: [boost] Boost.DLL formal review is ongoing Hi
I am attempting to get to grips with DLL to see if I can do a review.
I have cloned the source code and run the tests.
I am struggling with the documentation as I cannot find instructions how to compile and link the example files:
getting_started.cpp getting_started_library.cpp shared_lib_path.hpp
Should getting_started_library.cpp be put into a shared library? I have not found a makefile.
I am using clang 3.6 and libc++ on Ubuntu Linux.
Thanks
John
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
I am now buiding getting_started_library.cpp to getting_started_library.so
I am linking the getting started with that and also with
-ldl -lboost_filesystem -lboost_system
I am getting errors like this and the link fails.
/tmp/getting_started-f60894.o: In function `shared_lib_path(boost::filesystem::path const&, std::__1::basic_string
2015-07-03 23:12 GMT+03:00 Fletcher, John P
I am getting errors like this and the link fails.
/tmp/getting_started-f60894.o: In function `shared_lib_path(boost::filesystem::path const&, std::__1::basic_string
, std::__1::allocator > const&)': getting_started.cpp:(.text+0x217c): undefined reference to `boost::filesystem::path_traits::dispatch(boost::filesystem::directory_entry const&, std::__1::basic_string &)' getting_started.cpp:(.text+0x222a): undefined reference to `boost::filesystem::path_traits::convert(char const*, char const*, std::__1::basic_string , std::__1::allocator >&, std::__1::codecvt const&)' getting_started.cpp:(.text+0x2544): undefined reference to `boost::filesystem::path_traits::dispatch(boost::filesystem::directory_entry const&, std::__1::basic_string &)' How do I resolve these errors related to boost::filesystem::path_traits ?
It looks like you have Boost headers and libraries of different versions. Try to recompile the boost_filesystem library. Simplest way to run tests is to: cd boost_checkout_location ./bootstrap.sh # on Windows use bootstrap.bat cd libs git clone https://github.com/apolukhin/Boost.DLL dll cd dll/test ../../../b2 -a # here `-a` will force the rebuild of dependent libraries, including boost_filesystem -- Best regards, Antony Polukhin
El 04/07/2015 a las 11:19, Antony Polukhin escribió:
It looks like you have Boost headers and libraries of different versions. Try to recompile the boost_filesystem library.
I really like the idea of the library, and the interface seems good. I don't think I will have time to do a review, but my main comment would be related to dependencies. I very much like a DLL library without any dependencies on other boost types in its interface (boost::function, boost::filesystem). Even no dependencies in the implementation would be good, although this is much easier to deal with. I could think about a "core" vrsion of Boost.DLL that uses plain const char* names and pointers to functions and another version which could offer more goodies. The basic version could be easily interoperable with std::function/filesystem types (a simple wrapper could do the job) and it could be also used by other low-level boost libraries trying to load some DLLs. Just my 2 cents Ion
2015-07-06 11:47 GMT+03:00 Ion Gaztañaga
El 04/07/2015 a las 11:19, Antony Polukhin escribió:
It looks like you have Boost headers and libraries of different versions. Try to recompile the boost_filesystem library.
I really like the idea of the library, and the interface seems good. I don't think I will have time to do a review, but my main comment would be related to dependencies.
I very much like a DLL library without any dependencies on other boost types in its interface (boost::function, boost::filesystem). Even no dependencies in the implementation would be good, although this is much easier to deal with.
I could think about a "core" vrsion of Boost.DLL that uses plain const char* names and pointers to functions and another version which could offer more goodies. The basic version could be easily interoperable with std::function/filesystem types (a simple wrapper could do the job) and it could be also used by other low-level boost libraries trying to load some DLLs.
There's no dependency on shared_ptr and boost::function if headers starting with import_ are not used. However filesystem::path and system::error_code are integrated tightly in core. Your request is not the first, so probably such feature is highly required by library users. Is it OK to leave dependencies to Boost.Predef, string_ref and Boost.WinAPI? -- Best regards, Antony Polukhin
El 06/07/2015 a las 22:48, Antony Polukhin escribió:
Your request is not the first, so probably such feature is highly required by library users. Is it OK to leave dependencies to Boost.Predef, string_ref and Boost.WinAPI?
That would be great. Why string_ref and not const char *, symbols are not null-terminated? Ion
2015-07-07 1:25 GMT+03:00 Ion Gaztañaga
El 06/07/2015 a las 22:48, Antony Polukhin escribió:
Your request is not the first, so probably such feature is highly required by library users. Is it OK to leave dependencies to Boost.Predef, string_ref and Boost.WinAPI?
That would be great. Why string_ref and not const char *, symbols are not null-terminated?
I like string_ref because it allows to provide single overload for std::string and const char* parameters. Probably you're right and `const char*` is sufficient. -- Best regards, Antony Polukhin
On 6 Jul 2015 at 23:48, Antony Polukhin wrote:
There's no dependency on shared_ptr and boost::function if headers starting with import_ are not used. However filesystem::path and system::error_code are integrated tightly in core.
All of those are in recent STLs, the only remaining one without Filesystem is libc++.
Your request is not the first, so probably such feature is highly required by library users. Is it OK to leave dependencies to Boost.Predef, string_ref and Boost.WinAPI?
APIBind does enough of Predef to probably eliminate that. string_ref is probably best replaced with string_view, which is standalone. WinAPI is replaceable with windows.h :) If you do decide to make the conversion, APIBind will save you a ton of work and let one code base be both a Boost library and a standalone library. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Before I give my review, I would like to first ask some open questions here and see if I understood some points correctly. * Is Mac OS totally unsupported? * When would support for it be possible? As an avid Mac OS user, I feel excluded. Don't know if this is an issue for anybody else, though. * Why is the default section named "boostdll"? * Why not use the platform default? * Can't the "on_unload" have a similar API in the library? Thank you, Rodrigo Madera
On 7 July 2015 at 17:48, Rodrigo Madera
* Is Mac OS totally unsupported? * When would support for it be possible?
As an avid Mac OS user, I feel excluded. Don't know if this is an issue for anybody else, though.
Where is it stated that MacOS is not supported? I can't find anythinga bout this and the limitations page does not say anything about this.
On Tue, Jul 7, 2015 at 1:26 PM, Klaim - Joël Lamotte
Where is it stated that MacOS is not supported? I can't find anythinga bout this and the limitations page does not say anything about this.
Joël, In https://github.com/apolukhin/Boost.DLL/blob/master/README.md, there is a comment about the *tests* not working in Mac OS yet. I verified it by trying to compile them, and they failed. Also, I couldn't compile the first tutorial manually using clang. Did you succeed in doing so? I'll triple check to see if I have something wrong. Regards, Rodrigo Madera
On Tue, Jul 7, 2015 at 1:53 PM, Fletcher, John P
Rodrigo
Also, I couldn't compile the first tutorial manually using clang.
I have compiled tutorials 1 to 3 with clang 3.6 with C++11 and libc++. This is on Ubuntu Linux 12.04
Thank you, Mr. Fletcher. I should have been more clear: clang 3.6 using Mac OS 10.10.3. Regards, Rodrigo Madera
2015-07-07 18:48 GMT+03:00 Rodrigo Madera
Before I give my review, I would like to first ask some open questions here and see if I understood some points correctly.
* Is Mac OS totally unsupported?
It is supported and must work (in theory). Unfortunately I have no device with MacOS so it was impossible for me to run tests.
* When would support for it be possible?
If the library will be accepted into Boost, then the library will be tested by the Boost regressions testers. It means 100% support for MacOs, Android and other platforms.
As an avid Mac OS user, I feel excluded. Don't know if this is an issue for anybody else, though.
I'd like to have an online service for testing on MacOS or at least a virtual machine that supports MacOS out of the box. Unfortunately I've found none... That must be a disturbing alarm for the vendor: developers can not test software on the platform!
* Why is the default section named "boostdll"?
Section name must be a valid C identifier, so it seemed like a nice idea to use that section name for Boost.DLL
* Why not use the platform default?
This seems like a more user friendly solution. If a user is exporting symbol with an alias name, then there's a chance that some day the user will need to query exported symbols. Putting symbols into the default section will make them indistinguishable from other symbols.
* Can't the "on_unload" have a similar API in the library?
If the user needs to do something on library unload, then a global/static variable with user-defined destructor is the solution. Such solution is much more flexible and does not force user to use `on_unload` API (which is not ideal). I suspect that `global/static variable` to be more common. But you're right, this must be more explicitly specified in docs. Typo with `[callplugcpp_tutorial6]` also must be fixed. -- Best regards, Antony Polukhin
On 07.07.2015, at 20:16, Antony Polukhin
wrote: It is supported and must work (in theory). Unfortunately I have no device with MacOS so it was impossible for me to run tests.
Performing configuration checks
- 32-bit : no (cached)
- 64-bit : yes (cached)
- arm : no (cached)
- mips1 : no (cached)
- power : no (cached)
- sparc : no (cached)
- x86 : yes (cached)
- symlinks supported : yes (cached)
...patience...
...patience...
...found 1819 targets...
...updating 115 targets...
darwin.compile.c++ ../../../bin.v2/libs/dll/test/darwin-4.2.1/debug/link-static/static_plugin.o
In file included from ../example/tutorial4/static_plugin.cpp:8:
In file included from ../example/tutorial4/static_plugin.hpp:8:
In file included from ../../../boost/dll/alias.hpp:15:
In file included from ../../../boost/dll/shared_library.hpp:23:
In file included from ../../../boost/dll/detail/posix/shared_library_impl.hpp:11:
../../../boost/dll/shared_library_load_mode.hpp:219:45: error: use of undeclared identifier 'RTLD_DEEPBIND'
rtld_deepbind = RTLD_DEEPBIND,
^
In file included from ../example/tutorial4/static_plugin.cpp:8:
In file included from ../example/tutorial4/static_plugin.hpp:8:
In file included from ../../../boost/dll/alias.hpp:15:
In file included from ../../../boost/dll/shared_library.hpp:23:
In file included from ../../../boost/dll/detail/posix/shared_library_impl.hpp:12:
../../../boost/dll/detail/posix/path_from_handle.hpp:16:10: fatal error: 'link.h' file not found
#include libtest_library.dylib for lack of libmy_plugin_sum.dylib for lack of libmy_plugin_aggregator.dylib for lack of libon_unload_lib.dylib for lack of libgetting_started_library.dylib for lack of library1.dylib for lack of library2.dylib for lack of librefcounting_plugin.dylib for lack of libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
darwin.compile.c++ ../../../bin.v2/libs/dll/test/darwin-4.2.1/debug/link-static/threading-multi/static_plugin.o
In file included from ../example/tutorial4/static_plugin.cpp:8:
In file included from ../example/tutorial4/static_plugin.hpp:8:
In file included from ../../../boost/dll/alias.hpp:15:
In file included from ../../../boost/dll/shared_library.hpp:23:
In file included from ../../../boost/dll/detail/posix/shared_library_impl.hpp:11:
../../../boost/dll/shared_library_load_mode.hpp:219:45: error: use of undeclared identifier 'RTLD_DEEPBIND'
rtld_deepbind = RTLD_DEEPBIND,
^
In file included from ../example/tutorial4/static_plugin.cpp:8:
In file included from ../example/tutorial4/static_plugin.hpp:8:
In file included from ../../../boost/dll/alias.hpp:15:
In file included from ../../../boost/dll/shared_library.hpp:23:
In file included from ../../../boost/dll/detail/posix/shared_library_impl.hpp:12:
../../../boost/dll/detail/posix/path_from_handle.hpp:16:10: fatal error: 'link.h' file not found
#include libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped libtest_library.dylib...
...skipped
On Tue, Jul 7, 2015 at 3:50 PM, Thomas Trummer
On 07.07.2015, at 20:16, Antony Polukhin
wrote: It is supported and must work (in theory). Unfortunately I have no device with MacOS so it was impossible for me to run tests.
Performing configuration checks
- 32-bit : no (cached) - 64-bit : yes (cached)
(Snipped). I sent an email in private to Antony about the same errors. After some quick analysis, link.h must be ifdefed against BOOST_OS_MACOS. Simple enough, fixed on my fork. However, more importantly, in Linux we have RTLD_DEEPBIND [1], but in OS X we don't [2]. We have something *similar* in RTLD_FIRST but that most likely will break code and it's not entirely what and application will want. I will test rapidly here and see if the some tutorials continue to work after that, as well as the tests. Regards, Rodrigo Madera [1] http://linux.die.net/man/3/dlopen [2] https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPa...
Another problem is getting the image path [1]. There is a problem because we also don't have dlinfo() on OS X. I Googled and found a link on SO [2] that could help in fixing this. I can try to get this part working but only at weekends, if help is needed. Regards, Rodrigo Madera [1] File: dll/detail/posix/path_from_handle.hpp [2] http://stackoverflow.com/questions/20481058/find-pathname-from-dlopen-handle...
2015-07-07 22:20 GMT+03:00 Rodrigo Madera
Another problem is getting the image path [1]. There is a problem because we also don't have dlinfo() on OS X.
I Googled and found a link on SO [2] that could help in fixing this.
I can try to get this part working but only at weekends, if help is needed.
Regards, Rodrigo Madera
[1] File: dll/detail/posix/path_from_handle.hpp [2]
http://stackoverflow.com/questions/20481058/find-pathname-from-dlopen-handle...
Thanks for the hints and for giving the build output! Committed the fixes for MacOS issues to github repo. There's a high chance that there'll be some problems with boost::dll::library_info on MacOS, because it contains a lot of untested MacOS specific code. Could you please send me the libtest_library.dylib via private mail, so I could do the tests. -- Best regards, Antony Polukhin
On Wed, Jul 8, 2015 at 3:39 PM, Antony Polukhin
Thanks for the hints and for giving the build output! Committed the fixes for MacOS issues to github repo.
There's a high chance that there'll be some problems with boost::dll::library_info on MacOS, because it contains a lot of untested MacOS specific code. Could you please send me the libtest_library.dylib via private mail, so I could do the tests.
Absolutely. Will continue this conversation privately. Regards, Rodrigo Madera
On 3 July 2015 at 15:50, Vladimir Prus
The formal review of Boost.DLL library by Antony Polukhin is nearing the end of the first week. As of today, over 100 people read through the library's tutorial, suggesting quite some interest. However, no reviews were submitted yet. Please take the time to post your thoughts - even if you don't have time to do a full review, or try the library, comments on design and interfaces are still very valuable.
I started an experimentation to check a few things related to plugin systems specific to my projects and want to finish it before giving a final review. I still can do a pre-review using my experience so far, but wait for my confirmation before counting my review.
Please post your reviews on the mailing list and if possible, answer the following questions:
- Should the library be accepted?
YES, it should be accepted.
- How useful is it?
- It helps solving a common problem (the code managing plugins in an application). - It does it without the usual constraints (interface only in C for example) - this part I am actually challenging with my test code. - It's header only and cross platform. - It does not do more than helping getting functions and objects from shared libraries (like previous boost.extend did IIRC). - I tried to use several other such libraries before and so far it's the less intrusive.
- What's your evaluation of - Design
I consider the solutions provided to solve each usual issue with plugin systems to be simple and clear, easy to use (as long as you understand what loading plugins imply). Until I check in some complex cases, I think the design is very good. The symbol name magic might be tricky to understand sometime but it's an issue which is a side effect of providing a simple tool to manage symbole names, so it's still good. One thing I was wondering: would it be possible to provide a way to load shared libraries from raw memory? Or does the OS APIs all requires that the library have to be on a file sytem? - Implementation
I didn't look at the implementation yet.
- Documentations
It's good. I pointed a few things but globally it's working for me. Note: http://apolukhin.github.io/Boost.DLL/boost/dll/symbol_location.html I do not understand how to use this function, what is T here? Also, I did a lot of different plugin systems in the past so I am not sure what someone unexposed to this kind of system would think of the code. I suspect that the table in the QuickStart page might scare such people at first, but in the same time I find it very useful as a summary of how to use the library. I didn't have any problem with the tutorial so far.
- Tests
I didn't run nor read the tests provided with the library.
- How much effort did you put into your evaluation?
I read all the doc without references pages, a big part of references pages. The small test project I am trying to setup is a complex "worse case" plugin system matching my most difficult project (for which plugins are a vital part of the design), so once I do that I'll have a good idea of potential issues in non trivial cases.
- Did you attempt to use the library? On what systems and compilers?
Yes as said before I have a small test on-going but it's not finished yet. My attempt is currently on VS2013 64bit on Windows 7 and 8.1. I would like to try it with VS2015RC but Boost 1.58 does not compile correctly with this compiler. I will try it with gcc and clang on linux once my test is finished and working on Windows. I expect all the platforms to behave the same in my test, so we'll see.
2015-07-04 0:16 GMT+03:00 Klaim - Joël Lamotte
Note: http://apolukhin.github.io/Boost.DLL/boost/dll/symbol_location.html
I do not understand how to use this function, what is T here?
T could be any variable or function that you're interested in: int var; void foo() {} int main() { dll::symbol_location(var); // returns program location dll::symbol_location(foo); // returns program location dll::symbol_location(std::cerr); // probably returns location of libstdc++ dll::symbol_location(std::placeholders::_1); // probably returns location of libstdc++ dll::symbol_location(errno); // probably returns location of libc } I'll clarify the docs, thanks for the note! -- Best regards, Antony Polukhin
On 3 Jul 2015 at 16:50, Vladimir Prus wrote:
- Should the library be accepted?
Yes, conditional on the items below.
- How useful is it?
It is useful, though not as useful as it could be.
- What's your evaluation of - Design
The design is conservative and unsurprising, and is very similar to probably what 90% of us here would have chosen, myself included if one was tasked with an uncontroversial solution. I cannot fault it. My only qualm really, and this is entirely my fault, is there is a lack of type safety when loading symbols to a given type. This is my fault because I had said I would write a demangler which could check types matched, and I haven't had the time. If this feature existed, it could detect type mismatches and therefore ABI mismatch rather than just hoping a segfault should happen. A poor man's implementation could instead do demangle both symbols into a string, and do a nasty platform specific regex transformation into strings which are comparable. Slow and inaccurate though compared to a demangler based design.
- Implementation
I don't see any good reason why this library needs to be dependent on Boost. It uses little in Boost not also present in the C++ 11 STL, the only major blocker I noticed is some limited use of the MPL which is easily replaced with minimal constexpr as supported by VS2015. CONDITION: It should therefore support standalone usage decoupled from the rest of Boost on C++ 11 compilers. It can still use Boost on C++ 98 compilers. CONDITION: Namespace is not inline versioned on C++ 11 compilers. It should be. Otherwise implementation is very solid. I wouldn't expect any different from Antony.
- Documentations
CONDITION: BoostBook pages are missing badges for the CI test status. Readme has them though. CONDITION: No ability to launch example code in online web compiler. There is no acknowledgements section, and for good form there probably should be. Otherwise very good. I think I had sent Antony a list of docs problems last year, and he must have fixed them because I spotted nothing which particularly bothered me (that other reviews haven't already reported).
- Tests
CONDITION: No Appveyor integration. CONDITION: I saw a Coverity scan, but no clang-tidy + clang static analyser. A MSVC static analyser pass would do no harm either. CONDITION: The unit tests are not run under valgrind memcheck by default. Travis doesn't seem to have OS X testing enabled. The docs mention OS X isn't working yet, so that is understandable. Tests seem more functional than unit testing, but that is acceptable. I'm the same in AFIO.
- How much effort did you put into your evaluation?
About an hour, though I have been watching the development for a long time. I even promised some code I failed to deliver upon :(
- Did you attempt to use the library? On what systems and compilers?
Not on this occasion, but I did last year on Linux and Windows. It worked fine then. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 7/4/15 9:31 AM, Niall Douglas wrote:
On 3 Jul 2015 at 16:50, Vladimir Prus wrote:
- Should the library be accepted?
Yes, conditional on the items below.
A number of "Conditions" follows Just so I understand. Do you think the library should not be accepted if it fails to fulfill any one of the "Conditions" listed? Robert Ramey
On 4 Jul 2015 at 9:57, Robert Ramey wrote:
- Should the library be accepted?
Yes, conditional on the items below.
A number of "Conditions" follows
Just so I understand. Do you think the library should not be accepted if it fails to fulfill any one of the "Conditions" listed?
Sorry if my tone was a bit harsh in my review. I just wrote a difficult email regarding the Boost SC before it, and I am not in a good mood from that. My review was my recommendation to the review manager. I believe he can choose from people's reviews what to include in his final report, so he could choose to not follow any or all of my recommendations. That's always been the way till now surely? When I review managed Antony's TypeIndex, I tried to list the consensus points raised most commonly as the basis of the decision, and I mentioned those outlier points in a footnote at the end more for people's interest and completeness. Knowing Antony, I can't see any of those conditions being a problem except the standalone support. That's a *lot* of additional work. I would love if he invested that work, but it is no personal problem for me if he chooses not to. And I would very much understand if he didn't. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 04-Jul-15 8:23 PM, Niall Douglas wrote:
On 4 Jul 2015 at 9:57, Robert Ramey wrote:
- Should the library be accepted?
Yes, conditional on the items below.
A number of "Conditions" follows
Just so I understand. Do you think the library should not be accepted if it fails to fulfill any one of the "Conditions" listed?
Sorry if my tone was a bit harsh in my review. I just wrote a difficult email regarding the Boost SC before it, and I am not in a good mood from that.
My review was my recommendation to the review manager. I believe he can choose from people's reviews what to include in his final report, so he could choose to not follow any or all of my recommendations. That's always been the way till now surely? When I review managed Antony's TypeIndex, I tried to list the consensus points raised most commonly as the basis of the decision, and I mentioned those outlier points in a footnote at the end more for people's interest and completeness.
That is my understanding as well. Regarding these particular conditions, to avoid surprises I should say that all of them are reasonable, but don't necessary qualify as hard preconditions. Say, having no Boost dependencies might be good for some users, but having 'no Boost dependencies' as hard requirement for accepting a Boost library is not very logical. Likewise, CI is a good thing, but not accepting a library for lack of CI will be unfair to the library author, and requiring a particular CI service will be unnecessary promotion. Therefore, while all these conditions will certainly be listed in the review report, I don't think Antony should rush to implement them until the review is over and result is published. Thanks, Volodya
On 7 Jul 2015 at 9:50, Vladimir Prus wrote:
Regarding these particular conditions, to avoid surprises I should say that all of them are reasonable, but don't necessary qualify as hard preconditions.
Say, having no Boost dependencies might be good for some users, but having 'no Boost dependencies' as hard requirement for accepting a Boost library is not very logical.
I can agree with this. It's more a usability/user friendliness thing really, plus some corporations explicitly ban any library with the word "Boost" in its title, so if you can make your library standalone it will gain a not insigificant increase in potentially paying userbase.
Likewise, CI is a good thing, but not accepting a library for lack of CI will be unfair to the library author, and requiring a particular CI service will be unnecessary promotion.
I profoundly disagree on the first point. Any new library should be using a per-commit CI period. The fact such services are free, and they make such an enormous improvement to github pull requests and not breaking build accidentally make them one of the key quality and productivity leap forwards of the past decade. To not have your library per-commit CI enabled is enormously retrograde, and if a library before review here refused to have some per-commit CI in place I would vote for rejection personally because such a library could never be of Boost quality. On the second point, of promoting one service over another, for sure it'd be great if Boost provided a common Jenkins CI for all Boost libraries. We don't, so until then the most popular free CI by a long shot are Travis for Linux and OS X and Appveyor for Windows. It's hard to not promote a service when those two are so utterly dominant in open source. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 07-Jul-15 2:56 PM, Niall Douglas wrote:
I profoundly disagree on the first point. Any new library should be using a per-commit CI period. The fact such services are free, and they make such an enormous improvement to github pull requests and not breaking build accidentally make them one of the key quality and productivity leap forwards of the past decade. To not have your library per-commit CI enabled is enormously retrograde, and if a library before review here refused to have some per-commit CI in place I would vote for rejection personally because such a library could never be of Boost quality
There is a number of existing Boost libraries, and other open-source projects, whose quality is quite excellent despite having no per-commit CI - for example because the maintainer has more compilers installed locally, and more virtual machines, than all cloud CI systems combined. Calling these maintainers retrograde is hardly appropriate. Thanks, Volodya
On 14 Jul 2015 at 10:07, Vladimir Prus wrote:
On 07-Jul-15 2:56 PM, Niall Douglas wrote:
I profoundly disagree on the first point. Any new library should be using a per-commit CI period. The fact such services are free, and they make such an enormous improvement to github pull requests and not breaking build accidentally make them one of the key quality and productivity leap forwards of the past decade. To not have your library per-commit CI enabled is enormously retrograde, and if a library before review here refused to have some per-commit CI in place I would vote for rejection personally because such a library could never be of Boost quality
There is a number of existing Boost libraries, and other open-source projects, whose quality is quite excellent despite having no per-commit CI - for example because the maintainer has more compilers installed locally, and more virtual machines, than all cloud CI systems combined.
Firstly, my comment was qualified with the qualifier "new libraries". Secondly, you appear to be confusing integration testing with continuous testing. The former is of the type you mention, the latter can be as simple as a simple "does the library and its testsuite compile without warnings, static warnings nor errors on 80% of compilers and platforms?" I would challenge any Boost library maintainer to fail to add a simple "does it build?" Travis CI per commit test in any time amount exceeding one hour, especially now Travis has just added the new opt-in super fast Amazon EC2 instance support which is breathtakingly fast. With a second hour they can patch in per-commit clang-tidy, clang static analyser and MSVC static analyser support too. Antony has coverity-scan in his as well, I personally found it chokes badly on AFIO code and coverity officially refused to fix their tool. Still, could be useful for some. Thirdly, you are not adding per-commit CI testing for yourself. You are adding it for *everybody* *else* specifically anyone who forks your library on github, fixes a bug and sends you a pull request. *They* are the people you add per-commit CI testing for, not you. It's for this reason that AFIO is CI tested both by Jenkins (for me with full fat testing) and by Travis and Appveyor (for anyone who forks AFIO with "did you break me?" testing).
Calling these maintainers retrograde is hardly appropriate.
The best description is "anti-social" as David Sankel would put it. It's also a question of branding and marketing, as having CI status badges on your github project sends a strong easily recognised signal that you care about quality. For the minimal effort involved to add support, it's a no-brainer. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
2015-07-04 19:31 GMT+03:00 Niall Douglas
On 3 Jul 2015 at 16:50, Vladimir Prus wrote:
- Should the library be accepted?
Yes, conditional on the items below.
- How useful is it?
It is useful, though not as useful as it could be.
- What's your evaluation of - Design
The design is conservative and unsurprising, and is very similar to probably what 90% of us here would have chosen, myself included if one was tasked with an uncontroversial solution. I cannot fault it.
My only qualm really, and this is entirely my fault, is there is a lack of type safety when loading symbols to a given type. This is my fault because I had said I would write a demangler which could check types matched, and I haven't had the time. If this feature existed, it could detect type mismatches and therefore ABI mismatch rather than just hoping a segfault should happen.
A poor man's implementation could instead do demangle both symbols into a string, and do a nasty platform specific regex transformation into strings which are comparable. Slow and inaccurate though compared to a demangler based design.
Symbol demangling is not a silver bullet. Following functions and variables
may have the exactly same names in shared objects (and most of them have in
GCC!):
extern "C" {
BOOST_SYMBOL_EXPORT void foo(int, char*);
BOOST_SYMBOL_EXPORT int foo;
}
BOOST_SYMBOL_EXPORT void foo();
BOOST_SYMBOL_EXPORT boost::variant
- Implementation
I don't see any good reason why this library needs to be dependent on Boost. It uses little in Boost not also present in the C++ 11 STL, the only major blocker I noticed is some limited use of the MPL which is easily replaced with minimal constexpr as supported by VS2015.
CONDITION: It should therefore support standalone usage decoupled from the rest of Boost on C++ 11 compilers. It can still use Boost on C++ 98 compilers.
I'd rather not do a standalone version. If everything goes OK, I was planning to make two C++ proposals: * [[export]], [[export "valid_c_name"]], [[import]], [[import "valid_c_name"]] * shared_library (without all the *alias() magic, because [[export "valid_c_name"]] makes a true alias) + library_info So I think it's better to concentrate on a proposals and get the superior version in next Standard then adding new unrelated features to prototype. CONDITION: Namespace is not inline versioned on C++ 11 compilers. It
should be.
+1. But I'd rather emulate them: namespace boost { namespace dll { namespace v1 { // all the code } // namespace v1 using namespace v1; }} // namespace boost::dll Otherwise implementation is very solid. I wouldn't expect any
different from Antony.
Thanks :)
- Documentations
CONDITION: BoostBook pages are missing badges for the CI test status. Readme has them though.
Will be fixed.
CONDITION: No ability to launch example code in online web compiler.
I few days before the review I've started experimenting with online compilation of source codes from "Boost C++ Application Development Cookbook". I've stopped on http://coliru.stacked-crooked.com/ service. They have REST API, so it's possible to directly embed examples in documentation and allow users to modify them: http://en.cppreference.com/w/cpp/utility/tuple (See the "Example" on the bottom of the page). After I end the experiments on "Boost C++ Application Development Cookbook", I'll probably do something with the DLL docs (and with LexicalCast, TypeIndex, Variant ...)
There is no acknowledgements section, and for good form there probably should be.
+1
Otherwise very good. I think I had sent Antony a list of docs problems last year, and he must have fixed them because I spotted nothing which particularly bothered me (that other reviews haven't already reported).
- Tests
CONDITION: No Appveyor integration.
Have not seen this service before. I'll take a look at it.
CONDITION: I saw a Coverity scan, but no clang-tidy + clang static analyser. A MSVC static analyser pass would do no harm either.
There are now regression testers in Boost that run static analysers: http://www.boost.org/development/tests/develop/developer/summary.html Adding additional auto test runners may be an overkill.
CONDITION: The unit tests are not run under valgrind memcheck by default.
I do not know how to make tests run with valgrind by default without affecting regression testers that do not have valgrind installed.
Travis doesn't seem to have OS X testing enabled. The docs mention OS X isn't working yet, so that is understandable.
They have a few OS X guests and a huge queue of projects that require OS X. I'm somewhere in the middle of the queue.
Tests seem more functional than unit testing, but that is acceptable. I'm the same in AFIO.
- How much effort did you put into your evaluation?
About an hour, though I have been watching the development for a long time. I even promised some code I failed to deliver upon :(
- Did you attempt to use the library? On what systems and compilers?
Not on this occasion, but I did last year on Linux and Windows. It worked fine then.
Thanks for the review! -- Best regards, Antony Polukhin
On 5 Jul 2015 at 13:42, Antony Polukhin wrote:
I few days before the review I've started experimenting with online compilation of source codes from "Boost C++ Application Development Cookbook". I've stopped on http://coliru.stacked-crooked.com/ service. They have REST API, so it's possible to directly embed examples in documentation and allow users to modify them: http://en.cppreference.com/w/cpp/utility/tuple (See the "Example" on the bottom of the page).
After I end the experiments on "Boost C++ Application Development Cookbook", I'll probably do something with the DLL docs (and with LexicalCast, TypeIndex, Variant ...)
You may find the scripting at https://svn.boost.org/trac/boost/wiki/BestPracticeHandbook#a14.USERFRI ENDLINESS:Considerlettingpotentialuserstryyourlibrarywithasinglemousec lick of use.
CONDITION: The unit tests are not run under valgrind memcheck by default.
I do not know how to make tests run with valgrind by default without affecting regression testers that do not have valgrind installed.
I'd be happy if an extra Travis pass ran valgrind memcheck. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Here is a contribution to the review.
Please post your reviews on the mailing list and if possible, answer the following questions:
- Should the library be accepted? Yes - How useful is it? It provides something not otherwise available. - What's your evaluation of - Design This seems to be consistent. - Implementation This also is consistent and clear - Documentations This is where I had problems. I went to the section "Getting Started" in the manual. This I found confusing as it did not contain a simple example. It also contained this statement
"To start with the library you only need to include
- Tests
I installed the library into Boost 1.58.0 and the tests all ran with gcc 4.6 once I had also installed the compiled libraries.
- How much effort did you put into your evaluation?
I have worked to understand the examples in the tutorials and have built tutorials 1 to 3.
- Did you attempt to use the library? On what systems and compilers?
I have the tutorials 1 to 3 running with these systems gcc 4.6 and the compiled libraries for boost 1.58.0 (also compiled with gcc 4.6) clang 3.6 and libc++ 3.6 with C++11. This also uses the compiled libraries compiled with gcc 4.6 except that I have to compile some of the files for boost_filesystem again with clang 3.6. This is because of something to do with the use of wchar in the file "shared_lib_path.hpp". Thank you to Antony Polukhin who helped with my earlier message on this problem. I hope that is helpful. John Fletcher
Thanks, Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
2015-07-07 18:52 GMT+03:00 Fletcher, John P
This is where I had problems. I went to the section "Getting Started" in the manual. This I found confusing as it did not contain a simple example. It also contained this statement
"To start with the library you only need to include
header." which is not true for the getting started example which uses the boost_filesystemand boost_system libraries.
Also, the first example here is a C++11 export, so I jumped to the (wrong) conclusion that C++11 is needed.
Some of that is my fault, but I think that "Getting Started" should be rewritten to be clear for someone wanting to build a small example.
It would also help for that small example to make clear that the main code and the library need to be compiled separately. It would help to have a pointer to where to find how to build a linkable library for different compilers and systems, though I realise having all that information for different systems is too much to ask.
There is a certain amount of advice on how to compile for Linux, including some compiler flags. It would be very helpful if that could be gathered in one location so that all of it can be applied.
Thanks for multiple hints on how to improve the documents. You're not the first who dislikes the "Getting started" section, it requires a good rewrite. -- Best regards, Antony Polukhin
I've been unable to spend enough time to complete a review of this library, so I can't recommend or discourage its inclusion in Boost. I expect to be able to complete a review sometime next week, if the review manager is inclined to wait for it. In the meantime, I'll note a few things I've observed in my initial study: 1. The docs need serious editing. 2. The motivation section is too terse. It doesn't do a good job of explaining the value of the library. 3. I question exposing functions like this_line_location() and program_location(). I'd rather see special factory functions or enumerators to signal the behavior those pathnames enable. 4. A generic term I use for DLLs, shared objects, etc. is "dynamic library." I suggest that the documentation define that term once, up front, and then use it everywhere else rather than repeating the "DLL, shared object (DSO), etc." phrase. I'd also encourage using that name for the library itself (Boost.DynamicLibrary) and for the shared_object class. ___ Rob (Sent from my portable computation engine)
2015-07-13 21:47 GMT+03:00 Rob Stewart
4. A generic term I use for DLLs, shared objects, etc. is "dynamic library." I suggest that the documentation define that term once, up front, and then use it everywhere else rather than repeating the "DLL, shared object (DSO), etc." phrase. I'd also encourage using that name for the library itself (Boost.DynamicLibrary) and for the shared_object class.
I was trying to keep namings platform neutral: "dynamic library" is too Windows specific, "shared object" is too POSIX specific... Let it bee DLL for library abbreviation and "shared object" mostly in docs and source codes. However it does not seem so right and platform neutral right now, especially when dll::shared_object and dll::library_info are nearby in code. It may be profitable to rename dll::library_info into dll::shared_object_info/dll::binary_info/???
I expect to be able to complete a review sometime next week, if the review manager is inclined to wait for it.
Anyway, I'm interested in your opinion, so please send the review when you'll be ready. Any library related comments from any reviewer are welcomed even after the review end. -- Best regards, Antony Polukhin
On July 13, 2015 4:24:33 PM EDT, Antony Polukhin
2015-07-13 21:47 GMT+03:00 Rob Stewart
: <...> 4. A generic term I use for DLLs, shared objects, etc. is "dynamic library." I suggest that the documentation define that term once, up front, and then use it everywhere else rather than repeating the "DLL, shared object (DSO), etc." phrase. I'd also encourage using that name for the library itself (Boost.DynamicLibrary) and for the shared_object class.
I was trying to keep namings platform neutral: "dynamic library" is too Windows specific, "shared object" is too POSIX specific...
The Windows term is Dynamic Link Library, not Dynamic Library. The POSIX term is Dynamic Shared Object. They are both libraries that can be loaded dynamically rather than statically, so, Dynamic Library is the more generic name. The Apple terms are, I find, Dynamic Library or Bundle, though I don't know the specific differences between them and whether you support both or even should. Obviously, Dynamic Library isn't so generic when you add Apple to the mix.
Let it bee DLL for library abbreviation and "shared object" mostly in docs and source codes. However it does not seem so right and platform neutral right now,
Exactly. "DLL" is very much Windows specific. "shared_object" is POSIX specific. Combining them is actually confusing. For example, seeing "shared_object" in your "DLL" library implies a cross-platform adapter to DSOs for Windows rather than a generic dynamic library class.
especially when dll::shared_object and dll::library_info are nearby in code. It may be profitable to rename dll::library_info into dll::shared_object_info/dll::binary_info/???
I'd use "dl" as the namespace name "dynamic_library" as the main class name, and keep "library_info". "binary_info" could describe most anything that's binary.
I expect to be able to complete a review sometime next week, if the review manager is inclined to wait for it.
Anyway, I'm interested in your opinion, so please send the review when you'll be ready.
Any library related comments from any reviewer are welcomed even after the review end.
Of course. ___ Rob (Sent from my portable computation engine)
If we are open to bikeshedding, then I suggest that the library should not be named after the subject it operates on (whether we call them dynamic link libraries or shared objects), but rather on the functionality it provides. The purpose of the library is to load and find symbols in the subject, so something like Boost.Loader looks like a better alternative to me (knowing full well that it does not load arbitrary files.)
On 14 July 2015 at 12:37, Bjorn Reese
If we are open to bikeshedding, then I suggest that the library should not be named after the subject it operates on (whether we call them dynamic link libraries or shared objects), but rather on the functionality it provides.
The purpose of the library is to load and find symbols in the subject, so something like Boost.Loader looks like a better alternative to me (knowing full well that it does not load arbitrary files.)
In that case, "symbol loader" or "shared library loader" would be more fitting than "loader" which could be about other domains. Personally I don't care much about the name of the library in this case because almost all the names evoked here seems fine to me, event the platform-specific evocation ones. Note: Boost.SharedObject is a name already used by Artyom's (cppcms) non-boost library which was suggested several time around here but not submitted.
In that case, "symbol loader" or "shared library loader" would be more fitting than "loader" which
could be about other domains.
I was also thinking along the lines of LL for either Lib Loader or Library Loader. Or maybe PLL (Platform-independent Library Loader) might be a phonetic option that shows some relationship to DLL but is different enough?
Personally I don't care much about the name of the library in this case because almost all the names evoked here seems fine to me, event the platform-specific evocation ones.
+1 -- This message is subject to the CSIR's copyright terms and conditions, e-mail legal notice, and implemented Open Document Format (ODF) standard. The full disclaimer details can be found at http://www.csir.co.za/disclaimer.html. This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. Please consider the environment before printing this email.
On 7/14/2015 7:43 AM, Ralf Globisch wrote:
In that case, "symbol loader" or "shared library loader" would be more fitting than "loader" which
could be about other domains.
I was also thinking along the lines of LL for either Lib Loader or Library Loader.
Or maybe PLL (Platform-independent Library Loader) might be a phonetic option that shows some relationship to DLL but is different enough?
Personally I don't care much about the name of the library in this case because almost all the names evoked here seems fine to me, event the platform-specific evocation ones.
+1
Perhaps you could call it Dynamic Library Loader. And then refer to it using the acronym DLL.
On 13 July 2015 at 21:24, Antony Polukhin
I was trying to keep namings platform neutral: "dynamic library" is too Windows specific, "shared object" is too POSIX specific... Let it bee DLL for library abbreviation and "shared object" mostly in docs and source codes.
On Mac OS, the files in question are generally named “libSomething.dylib”, whereas on Windows the equivalent file would be “something.dll”. That seems to directly contradict your assertion that “dynamic library” is more Windows-specific, since “dylib” looks more like a contraction of that phrase than “dll” to me. More generally, and fwiw, in my mind I connect “DLL” very tightly to Windows, to the extent that if somebody says “DLL” I think “we're talking about something which is exclusively related to Windows”. I tend to use instead the term “shared library” when referring to these files in a platform-neutral way (in other words, when I say “shared library” I mean .dll on Windows, .dylib on Mac OS, and .so on Linux, for example). In that way (none of these platforms uses a contraction of “shared library” as its way of visually identifying these files in the filesystem) I try to separate cross-platform discussion from platform-specific discussion. To me, “shared object” feels, as you say, very POSIX-specific. By way of comparison, the “file” utility on this Mac OS machine returns, for example, “/opt/local/lib/libgtk-3.0.dylib: Mach-O 64-bit dynamically linked shared library x86_64”. A similar example on Ubuntu Linux: “/lib/libfuse.so.2.8.6: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0x8776ddf840d71df11e0f99388074338a4dbc05c7, stripped”. Finally, on Windows I see of course “/cygdrive/c/Windows/System32/ntdll.dll: PE32+ executable (DLL) (console) x86-64, for MS Windows”. There is in fact no /shared/ nomenclature between all three at all there (because DLLs /are/ executables on Windows!). Something which has the right connotations but which isn't the terminology returned by that utility, or used as a filesystem contraction, for any specific platform seems ideal.
Any library related comments from any reviewer are welcomed even after the review end.
Apologies for the fact that my comments are purely in the area of nomenclature! -- Bill Gallafent.
-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of William Gallafent Sent: 14 July 2015 11:10 To: boost@lists.boost.org Subject: Re: [boost] Boost.DLL formal review is ongoing
On 13 July 2015 at 21:24, Antony Polukhin
wrote: I was trying to keep namings platform neutral: "dynamic library" is too Windows specific, "shared object" is too POSIX specific... Let it bee DLL for library abbreviation and "shared object" mostly in docs and source codes.
On Mac OS, the files in question are generally named “libSomething.dylib”, whereas on Windows the equivalent file would be “something.dll”. That seems to directly contradict your assertion that “dynamic library” is more Windows-specific, since “dylib” looks more like a contraction of that phrase than “dll” to me.
More generally, and fwiw, in my mind I connect “DLL” very tightly to Windows, to the extent that if somebody says “DLL” I think “we're talking about something which is exclusively related to Windows”. I tend to use instead the term “shared library” when referring to these files in a platform-neutral way (in other words, when I say “shared library” I mean .dll on Windows, .dylib on Mac OS, and .so on Linux, for example). In that way (none of these platforms uses a contraction of “shared library” as its way of visually identifying these files in the filesystem) I try to separate cross-platform discussion from platform-specific discussion. To me, “shared object” feels, as you say, very POSIX-specific.
By way of comparison, the “file” utility on this Mac OS machine returns, for example, “/opt/local/lib/libgtk-3.0.dylib: Mach-O 64-bit dynamically linked shared library x86_64”. A similar example on Ubuntu Linux: “/lib/libfuse.so.2.8.6: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0x8776ddf840d71df11e0f99388074338a4dbc05c7, stripped”. Finally, on Windows I see of course “/cygdrive/c/Windows/System32/ntdll.dll: PE32+ executable (DLL) (console) x86-64, for MS Windows”.
There is in fact no /shared/ nomenclature between all three at all there (because DLLs /are/ executables on Windows!). Something which has the right connotations but which isn't the terminology returned by that utility, or used as a filesystem contraction, for any specific platform seems ideal.
Of course - but Bill clearly demonstrates that there is no perfect answer to this. Any new 'portable' term would just confuse everyone! But DLL is short and sweet, and a glance at the documentation will reveal that the library is *not* Windows specific. Perhaps it stands for Dynamic Linked-things Loader? Paul --- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830
Apologies for the fact that my comments are purely in the area of nomenclature!
Ditto.
In article <55969319.8040900@gmail.com>,
Vladimir Prus
- Should the library be accepted?
Looks good to me.
- How useful is it?
It's useful to have a single way of dealing with shared libraries across platforms. I've done it a number of times myself and this library would have made things easier.
- What's your evaluation of - Design
Looks fine.
- Implementation
I didn't inspect the implementation.
- Documentations
The documentation is fine. There are a few minor grammar/typos but I didn't take note of their specific location, sorry.
- Tests
I didn't look at the tests.
- How much effort did you put into your evaluation?
I read through the Motivation, Getting Started and Tutorial. I skimmed the reference. I read the FAQ, Design Rationale and Dependencies section.
- Did you attempt to use the library? On what systems and compilers?
I didn't attempt to use it. -- "The Direct3D Graphics Pipeline" free book http://tinyurl.com/d3d-pipeline The Computer Graphics Museum http://ComputerGraphicsMuseum.org The Terminals Wiki http://terminals.classiccmp.org Legalize Adulthood! (my blog) http://LegalizeAdulthood.wordpress.com
participants (16)
-
Antony Polukhin
-
Bjorn Reese
-
Fletcher, John P
-
Ion Gaztañaga
-
Klaim - Joël Lamotte
-
Michael Marcin
-
Niall Douglas
-
Paul A. Bristow
-
Ralf Globisch
-
Richard
-
Rob Stewart
-
Robert Ramey
-
Rodrigo Madera
-
Thomas Trummer
-
Vladimir Prus
-
William Gallafent