[quickbook] diagnosing a missing [endsect]?
I just noticed a missing [endsect] in the middle of one of the .qbk files in the Core documentation. The main .qbk file contains [include addressof.qbk] [include checked_delete.qbk] [include demangle.qbk] [include enable_if.qbk] [include explicit_operator_bool.qbk] [include ignore_unused.qbk] [include is_same.qbk] [include lightweight_test.qbk] [include no_exceptions_support.qbk] [include noncopyable.qbk] [include null_deleter.qbk] [include ref.qbk] [include scoped_enum.qbk] [include swap.qbk] [include typeinfo.qbk] and in the middle of lightweight_test.qbk an [endsect] was missing. The end result of it was that the following includes went into the [section] opened in lightweight_test.qbk, as subsections. This wasn't noticeable from a cursory look so it went unnoticed for a while. Is it worth diagnosing this problem in some way? We probably can't disallow unbalanced sections in an [include], but this could be a warning? (If someone pays it any attention, of course.) Presumably, at the end of the file there'll be a [section] left open? This could be made an error so it'd be harder to miss.
On 15 March 2017 at 23:21, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Is it worth diagnosing this problem in some way? We probably can't disallow unbalanced sections in an [include], but this could be a warning? (If someone pays it any attention, of course.)
There already is a warning if a section isn't closed at the end of any file which starts with a docinfo block: $ cd libs/core/doc $ git checkout 064cfd3d732621bea36733ca3c4e513c83654418^ $ quickbook core.qbk Generating Output File: core.xml core.qbk: warning: Missing [endsect] detected at end of file. Although I imagine it can easily get lost in all the output from boost build, doxygen and boostbook. Could add an option to make it an error I suppose. An include which doesn't have a docinfo block is mostly treated like a straight textual include, which is why the warning will appear in the containing file. For [quickbook 1.6] documents, included files with a docinfo block are treated more like documents in their own right, so quickbook will warn about unclosed sections in that file. Tricky thing for boost documentation is that the 'library' element doesn't allow any children which have quickbook docinfo blocks. In boostbook sections can have a 'sectioninfo' block, so maybe there should be a docinfo block for sections (which confusingly would have to be called something other than 'section'). Or a way of indicating that a file (or perhaps all files) should be self-contained. It might be better to rethink how structural elements (book, part, chapter, library, etc.) and 'section' work so that they're more consistent.
Daniel James wrote:
There already is a warning if a section isn't closed at the end of any file which starts with a docinfo block:
$ cd libs/core/doc $ git checkout 064cfd3d732621bea36733ca3c4e513c83654418^ $ quickbook core.qbk Generating Output File: core.xml core.qbk: warning: Missing [endsect] detected at end of file.
Although I imagine it can easily get lost in all the output from boost build, doxygen and boostbook.
It does, that's the problem.
Could add an option to make it an error I suppose.
I can't imagine why we'd want that to not be an error, although at this point making it an error would probably break something that used to work fine. Thinking about it a bit more, what'd be really nice to have is a way to validate .qbk files that could be invoked from Travis/Appveyor, so that we can run that in PRs. But the problem here is that this validator would ideally be written in Python, so as to not require building Quickbook. (Similar to how I had to partially port Boostdep to Python to be able to use it in CI to install dependencies.)
Thinking about it a bit more, what'd be really nice to have is a way to validate .qbk files that could be invoked from Travis/Appveyor, so that we can run that in PRs. But the problem here is that this validator would ideally be written in Python, so as to not require building Quickbook.
On second thought, the idea of rewriting Quickbook in Python is a bit stupid. There is a prebuilt Quickbook in libboost-dev, but it's 1.46 on Ubuntu Precise which Travis uses, so it will not contain our hypothetical strict validation options. We could bite the bullet and build it from source, I suppose. But... D:\tmp2\boost>python2.7.exe tools/boostdep/depinst/depinst.py ../tools/quickbook Installing module core Installing module preprocessor Installing module iterator Installing module algorithm Installing module tuple Installing module bind Installing module smart_ptr Installing module unordered Installing module assert Installing module range Installing module program_options Installing module foreach Installing module filesystem Installing module lexical_cast Installing module config Installing module spirit Installing module iostreams Installing module utility Installing module locale Installing module move Installing module functional Installing module system Installing module tti Installing module concept_check Installing module io Installing module integer Installing module array Installing module regex Installing module static_assert Installing module container Installing module predef Installing module proto Installing module detail Installing module any Installing module type_traits Installing module numeric/conversion Installing module function Installing module tokenizer Installing module random Installing module phoenix Installing module mpl Installing module endian Installing module variant Installing module conversion Installing module fusion Installing module serialization Installing module optional Installing module pool Installing module function_types Installing module exception Installing module thread Installing module typeof Installing module math Installing module throw_exception Installing module type_index Installing module atomic Installing module intrusive Installing module winapi Installing module date_time Installing module chrono Installing module lambda Installing module ratio Installing module rational Hmm. :-)
We could bite the bullet and build [Quickbook] from source, I suppose.
There's something wrong with the DIST_BIN logic in Quickbook's Jamfile, because: D:\tmp2\boost>b2 tools/quickbook Performing configuration checks - 32-bit : yes (cached) - arm : no (cached) - mips1 : no (cached) - power : no (cached) - sparc : no (cached) - x86 : yes (cached) - symlinks supported : no (cached) - junctions supported : yes (cached) - hardlinks supported : yes (cached) ...patience... ...patience... ...found 3485 targets... ...updating 5 targets... common.mkdir D:\tmp2\boost\D:\tmp2 The filename, directory name, or volume label syntax is incorrect. if not exist "D:\tmp2\boost\D:\tmp2\\" mkdir "D:\tmp2\boost\D:\tmp2" ...failed common.mkdir D:\tmp2\boost\D:\tmp2... ...skipped D:\tmp2\boost\D:\tmp2\boost for lack of D:\tmp2\boost\D:\tmp2... ...skipped D:\tmp2\boost\D:\tmp2\boost\dist for lack of D:\tmp2\boost\D:\tmp2\boost... ...skipped D:\tmp2\boost\D:\tmp2\boost\dist\bin for lack of D:\tmp2\boost\D:\tmp2\boost\dist... ...skipped <pD:\tmp2\boost\D:\tmp2\boost\dist\bin>quickbook.exe for lack of D:\tmp2\boost\D:\tmp2\boost\dist\bin... ...failed updating 1 target... ...skipped 4 targets... Also, the customary location for the build Jamfile is in build/, so that the usual invocation b2 tools/quickbook/build works, although that's a bit pedantic.
On 16 March 2017 at 14:29, Peter Dimov via Boost <boost@lists.boost.org> wrote:
There's something wrong with the DIST_BIN logic in Quickbook's Jamfile, because:
It looks like the problem is that 'is-rooted' in 'tools/build/src/util/path.jam' doesn't support windows paths, so 'path.root' in the same file is concatenating absolute paths, giving 'D:\tmp2\boost\D:\tmp2\boost\dist\bin'. But I can't check that as I'm not set up on windows. If I'm right then is-rooted really needs to be fixed, but can you try the attached patch? It assumes that BOOST_ROOT is always absolute, which I think is the case.
Daniel James wrote:
If I'm right then is-rooted really needs to be fixed, but can you try the attached patch? It assumes that BOOST_ROOT is always absolute, which I think is the case.
This works, quickbook gets built and copied to $BOOST_ROOT/dist/bin. But then C:\Projects\boost-git\boost>dist\bin\quickbook.exe E C:\Projects\boost-git\boost>dist\bin\quickbook.exe --help A C:\Projects\boost-git\boost>dist\bin\quickbook.exe --input-file libs\mp11\doc\mp11.qbk --output-file test.xml G By the look of it, a UTF-16 string is probably being output as 8 bit. I'll try to debug it. (test.xml does end up being created, so it works, it's just the console output that's missing.)
On 18 March 2017 at 21:00, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Daniel James wrote:
If I'm right then is-rooted really needs to be fixed, but can you try the attached patch? It assumes that BOOST_ROOT is always absolute, which I think is the case.
This works, quickbook gets built and copied to $BOOST_ROOT/dist/bin. But then
C:\Projects\boost-git\boost>dist\bin\quickbook.exe E C:\Projects\boost-git\boost>dist\bin\quickbook.exe --help A C:\Projects\boost-git\boost>dist\bin\quickbook.exe --input-file libs\mp11\doc\mp11.qbk --output-file test.xml G
By the look of it, a UTF-16 string is probably being output as 8 bit. I'll try to debug it. (test.xml does end up being created, so it works, it's just the console output that's missing.)
If the console output isn't working, it'd probably be better to just use ASCII rather than waste time fixing the code (which is very messy). It'll be fine for most output, quickbook will still support UTF-8 in the xml output.
This works, quickbook gets built and copied to $BOOST_ROOT/dist/bin. But then
C:\Projects\boost-git\boost>dist\bin\quickbook.exe E C:\Projects\boost-git\boost>dist\bin\quickbook.exe --help A C:\Projects\boost-git\boost>dist\bin\quickbook.exe --input-file libs\mp11\doc\mp11.qbk --output-file test.xml G
By the look of it, a UTF-16 string is probably being output as 8 bit.
My default toolset is msvc-8.0, and it seems that std::wcout doesn't quite work there, although the trick with _setmode(_fileno(stdout), _O_U16TEXT) does enable wprintf et al. So I tried 14.0. Turns out that it's too new: actions.cpp c:\projects\boost-git\boost\tools\quickbook\src\utils.hpp(40): error C2280: 'boost::basic_string_ref<char,std::char_traits<char>>::basic_string_ref<std::allocator<char>>(std::basic_string<char,std::char_traits<char>,std::allocator<char>> &&)': attempting to reference a deleted function .\boost/utility/string_ref.hpp(98): note: see declaration of 'boost::basic_string_ref<char,std::char_traits<char>>::basic_string_ref' I then tried 12.0, same error. Still too new. 11.0 worked though, yay. C:\Projects\boost-git\boost>dist\bin\quickbook.exe --help Allowed options: --help produce help message --version print version string --no-pretty-print disable XML pretty printing --no-self-linked-headers stop headers linking to themselves --indent arg indent spaces --linewidth arg line width --input-file arg input file --output-file arg output file --output-deps arg output dependency file --ms-errors use Microsoft Visual Studio style error & warn message format -I [ --include-path ] arg include path -D [ --define ] arg define macro --image-location arg image location Here's the same error from g++ 6.3 -std=c++1z: tools\quickbook\src\actions.cpp: In function 'void quickbook::role_action(quickbook::state&, quickbook::value)': tools\quickbook\src\actions.cpp:339:35: error: use of deleted function 'boost::basic_string_ref<charT, traits>::basic_string_ref(std::basic_string<CharT, Traits, Alloc>&&) [with Allocator = std::allocator<char>; charT = char; traits = std::char_traits<char>]' state.phrase.get()); ^ In file included from tools\quickbook\src\fwd.hpp:16:0, from tools\quickbook\src\quickbook.hpp:19, from tools\quickbook\src\actions.cpp:24: ./boost/utility/string_ref.hpp:98:9: note: declared here basic_string_ref( std::basic_string<charT, traits, Allocator>&&) ^~~~~~~~~~~~~~~~ This looks like the string_ref issue someone already complained about. Your Travis probably missed it because it tests against 1.63.0 instead of depinst.py'ing its way to freedom, democracy and the American way. (Distant bald eagle cry.)
On 18 March 2017 at 21:47, Peter Dimov via Boost <boost@lists.boost.org> wrote:
This works, quickbook gets built and copied to $BOOST_ROOT/dist/bin. But then
C:\Projects\boost-git\boost>dist\bin\quickbook.exe E C:\Projects\boost-git\boost>dist\bin\quickbook.exe --help A C:\Projects\boost-git\boost>dist\bin\quickbook.exe --input-file libs\mp11\doc\mp11.qbk --output-file test.xml G
By the look of it, a UTF-16 string is probably being output as 8 bit.
My default toolset is msvc-8.0, and it seems that std::wcout doesn't quite work there, although the trick with _setmode(_fileno(stdout), _O_U16TEXT) does enable wprintf et al.
Looking at the code I expected UTF-16 to work for Visual C++ 7.1 and up, so I'm going to download Visual Studio 2008 and give it a go.
tools\quickbook\src\actions.cpp: In function 'void quickbook::role_action(quickbook::state&, quickbook::value)': tools\quickbook\src\actions.cpp:339:35: error: use of deleted function 'boost::basic_string_ref<charT, traits>::basic_string_ref(std::basic_string<CharT, Traits, Alloc>&&) [with Allocator = std::allocator<char>; charT = char; traits = std::char_traits<char>]' state.phrase.get()); ^ In file included from tools\quickbook\src\fwd.hpp:16:0, from tools\quickbook\src\quickbook.hpp:19, from tools\quickbook\src\actions.cpp:24: ./boost/utility/string_ref.hpp:98:9: note: declared here basic_string_ref( std::basic_string<charT, traits, Allocator>&&) ^~~~~~~~~~~~~~~~
IIRC one of the main use cases for string_ref was so that functions would just need a single overload to accept strings, so that's a bit annoying. I'll wait and see if it changes after the beta.
This looks like the string_ref issue someone already complained about. Your Travis probably missed it because it tests against 1.63.0 instead of depinst.py'ing its way to freedom, democracy and the American way. (Distant bald eagle cry.)
Everything looks like a nail? If i wanted to test against master, I would download a snapshot. But even if I did that, the travis tests wouldn't have caught this, as I only run them in C++03 mode. For a few reasons, I don't think Travis is an appropriate place for testing the dependencies of quickbook. Would be better to add it to the main testing setup, but I've never been motivated enough to do that. My main concern is that the automated builds work, and I'm not going to put a lot of effort into guaranteeing constant support for other configurations.
Daniel James wrote:
My default toolset is msvc-8.0, and it seems that std::wcout doesn't quite work there, although the trick with _setmode(_fileno(stdout), _O_U16TEXT) does enable wprintf et al.
Looking at the code I expected UTF-16 to work for Visual C++ 7.1 and up, so I'm going to download Visual Studio 2008 and give it a go.
Not sure you should bother with that; I doubt that anyone uses 8.0 any longer (which is 2005 by the way, 2008 is 9.0). I'll bump my default to something modern. I'm more interested in our original topic, that is, there to be a way to validate a .qbk file such that (a) the warning for a missing [endsect] to be an error (and the same applies to other similar warnings, if there are any) and (b) unmatched [section]/[endsect] in an included .qbk to also be treated the same way if the .qbk marked as self-contained in some unspecified way (not sure that a docinfo block is the best way to mark for our purposes, although I'm not exactly an expert.) People would then be able to add a doc validation step to Travis, or to change their doc Jamfiles to enable strict mode so that "b2 doc" fails properly when it detects such mistakes.
On 22 March 2017 at 03:00, Peter Dimov via Boost <boost@lists.boost.org> wrote:
I'm more interested in our original topic, that is, there to be a way to validate a .qbk file such that (a) the warning for a missing [endsect] to be an error (and the same applies to other similar warnings, if there are any)
I'm working on it: https://github.com/danieljames/quickbook/commit/68f11c3854b69173f321332f2cfa...
and (b) unmatched [section]/[endsect] in an included .qbk to also be treated the same way if the .qbk marked as self-contained in some unspecified way (not sure that a docinfo block is the best way to mark for our purposes, although I'm not exactly an expert.)
Less of a priority. It's been years since I implemented that warning, and this is the first time that's come up.
Daniel James wrote:
On 22 March 2017 at 03:00, Peter Dimov via Boost <boost@lists.boost.org> wrote:
I'm more interested in our original topic, that is, there to be a way to validate a .qbk file such that (a) the warning for a missing [endsect] to be an error (and the same applies to other similar warnings, if there are any)
I'm working on it:
https://github.com/danieljames/quickbook/commit/68f11c3854b69173f321332f2cfa...
Thanks!
On 22 March 2017 at 03:00, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Not sure you should bother with that; I doubt that anyone uses 8.0 any longer (which is 2005 by the way, 2008 is 9.0). I'll bump my default to something modern.
FWIW I installed 2005 but couldn't get it to compile. There's an error in spirit, which is a bit odd, as it's the original spirit which hasn't changed. Seemed to work fine with 2008, but I've changed it (in develop) to not use wide streams for older versions of Visual C++ anyway. I probably should change it to filter out any UTF-8 characters, but they're quite rare in this context, so it should be mostly okay for now.
On 3/27/2017 4:55 AM, Daniel James via Boost wrote:
On 22 March 2017 at 03:00, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Not sure you should bother with that; I doubt that anyone uses 8.0 any longer (which is 2005 by the way, 2008 is 9.0). I'll bump my default to something modern.
FWIW I installed 2005 but couldn't get it to compile.
There is also a VS2005 SP1 at https://support.microsoft.com/en-us/help/928957/visual-studio-2005-service-p.... But i would not worry about VS2005 either too much.
There's an error in spirit, which is a bit odd, as it's the original spirit which hasn't changed. Seemed to work fine with 2008, but I've changed it (in develop) to not use wide streams for older versions of Visual C++ anyway. I probably should change it to filter out any UTF-8 characters, but they're quite rare in this context, so it should be mostly okay for now.
Daniel James wrote:
Everything looks like a nail? If i wanted to test against master, I would download a snapshot. But even if I did that, the travis tests wouldn't have caught this, as I only run them in C++03 mode. For a few reasons, I don't think Travis is an appropriate place for testing the dependencies of quickbook. Would be better to add it to the main testing setup, but I've never been motivated enough to do that. My main concern is that the automated builds work, and I'm not going to put a lot of effort into guaranteeing constant support for other configurations.
Well currently, "b2 doc" would fail for everyone using Visual Studio 2013/2015/2017 or g++ 6. It would have failed for me had I not been using the ancient 8.0 as a default. In general, and this is not directed at you personally, the point of testing develop against boostorg:develop is to catch such breaking changes as the current string_ref one early, before they reach the release. Downstream dependencies effectively serve as an integration test for lower-level libraries.
On 22 March 2017 at 11:49, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Well currently, "b2 doc" would fail for everyone using Visual Studio 2013/2015/2017 or g++ 6. It would have failed for me had I not been using the ancient 8.0 as a default.
Then they file a bug report, and I fix it at a leisurely pace, suggesting they use a binary from the last release that worked. It's not ideal, but that's life for you.
In general, and this is not directed at you personally, the point of testing develop against boostorg:develop is to catch such breaking changes as the current string_ref one early, before they reach the release. Downstream dependencies effectively serve as an integration test for lower-level libraries.
Travis is an awful mechanism for doing that as it only runs when there's a commit to the submodule being tested, which might not be regular. As it happens, the change to string_ref that caused this issue was on the 13th of February, the last commit to quickbook before you reported this was on the 12th of February.
On 22 March 2017 at 22:14, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Daniel James wrote:
Travis is an awful mechanism for doing that as it only runs when there's a commit to the submodule being tested, which might not be regular.
It's a stochastic testing method, three time winner of the highly acclaimed "still better than nothing" award.
It's a negligible difference.
On 22 March 2017 at 22:26, Daniel James <dnljms@gmail.com> wrote:
On 22 March 2017 at 22:14, Peter Dimov via Boost <boost@lists.boost.org> wrote:
Daniel James wrote:
Travis is an awful mechanism for doing that as it only runs when there's a commit to the submodule being tested, which might not be regular.
Travis has now added a 'cron job' feature, so I'm going to use that to run regular tests against master and develop.
There is a prebuilt Quickbook in libboost-dev, but it's 1.46 on Ubuntu Precise which Travis uses, so it will not contain our hypothetical strict validation options. We could bite the bullet and build it from source, I suppose. But...
On further reflection, the obvious way to make Quickbook available for Travis/Appveyor builds is to just publish binaries using the Github release system. We'll still need a way to enable "strict mode" though, either from within the document, or via a command line option.
On 17 March 2017 at 14:46, Peter Dimov via Boost <boost@lists.boost.org> wrote:
There is a prebuilt Quickbook in libboost-dev, but it's 1.46 on Ubuntu Precise which Travis uses, so it will not contain our hypothetical strict validation options. We could bite the bullet and build it from source, I suppose. But...
On further reflection, the obvious way to make Quickbook available for Travis/Appveyor builds is to just publish binaries using the Github release system.
Perhaps something like this? https://github.com/boostorg/website/blob/master/.travis.yml#L13
Daniel James wrote:
On further reflection, the obvious way to make Quickbook available for Travis/Appveyor builds is to just publish binaries using the Github release system.
Perhaps something like this?
https://github.com/boostorg/website/blob/master/.travis.yml#L13
Something like this :-)
participants (4)
-
Daniel James -
degski -
Edward Diener -
Peter Dimov