Re: [boost] [xint] Third release is ready, requesting preliminary review

Subject: Re: [boost] [xint] Third release is ready, requesting preliminary review
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2010-05-04 11:32:58

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/04/2010 05:51 AM, Jürgen Hunold wrote:

>>> Well, I'm concentrating on Boost.Build issues. The rest looks nice.
>>> Please find some improvements to you current setup attached. [...]
>>
>> I'm confused... I thought you'd done something different. 
> 
> It seems I should submit smaller pachtes, my fault.

Not at all. The patches were fine, I just thought that you meant
something different.

>> Why did you split up the test program into multiple programs? What
>> benefit does that offer over a single executable?
> 
> Well, as long as you run the tests locally, this might be better. But as soon 
> as your lib is part of the regular regression tests, you have to deal with 
> *lots* of possible errors. This could be:
> - timeout on slow machines due to too long execution time
> - compile and/or link errors 
> 
> And it make things easiser if you only have on "red" entry in the summary page 
> and can just see which test file to look at.
> And smaller test cases are just current Boost standard.

That works, then. Thanks for satisfying my curiosity.

> And I think the patch 
> 
> +#ifdef BOOST_XINT_SINGLE_TEST_PROGRAM
> +    #define BOOST_TEST_DYN_LINK
> +#else
> +    #define BOOST_TEST_MAIN
> +#endif
> 
> is wrong. BOOST_TEST_DYN_LINK should always be defined on the command line when 
> linking against a shared Boost.Test library. So, no source code #define is 
> necessary.

But removing the BOOST_TEST_MAIN from all but one of the files, when
using a single test program, is.

> The code above breaks when doing "bjam link=static" with 
> "in function main:/rmc/hunold/packages/boost/boost/test/unit_test.hpp:59: 
> error: undefined reference to 'boost::unit_test::unit_test_main(bool (*)(), 
> int, char**)'" because Boost.Test expects "main" to be linked into the static 
> library.

I'm fairly new to Boost.Test, but the documentation gave me the
impression that you had to make a decision on whether to use dynamic
linking or not when you wrote the code, because static and dynamic
linking need slightly different setups. That would suggest that you
can't easily switch between them, wouldn't it?

> And this issues 
> "
> test_main.cpp:95:1: warning: "BOOST_TEST_DYN_LINK" redefined
> <command-line>: warning: this is the location of the previous definition"
> 
> when compiling a shared library with all Boost.Build magic enabled.

Eh? The library code doesn't use Boost.Test, only the testing code does.
And even the testing code only includes that #define when
BOOST_XINT_SINGLE_TEST_PROGRAM is defined, which it should never be
except on my development machine, or that of someone else doing
modifications to the library.

> By the way: 
> You have deleted test/Jamroot. Should there be a "test/Jamfile" instead ?

Yes, there should. Did it not make it into Subversion? Checking... it
appears that it's there, so I'm not sure what to tell you. Maybe check
it out manually?

> build/Jamfile.v2 still contains "nothrow_random.cpp" which was deleted in 61752 
> as well.

Sorry, I didn't remember to update that. I would have caught it before a
final release though, I have an entire checklist of things to do for that.

> And I think you may also check in files for other build systems, too.
> Some libraries have Makefiles as well as visual studio solutions.

I didn't think anyone would care about my Code::Blocks or Visual Studio
projects. If there's some interest in them, I'll add them in the next
repository update.
- -- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvgPioACgkQp9x9jeZ9/wRHnwCgxRL/IbQz3hOCQiBX0s0l1kDL
djMAnAqYWM7f0DJqTHru4ce2aQ25dk5u
=L990
-----END PGP SIGNATURE-----