boostorg / boost_install

8 stars 32 forks source link

Derive dependencies from the dependency graph automatically. #1

Closed swatanabe closed 6 years ago

swatanabe commented 6 years ago

Note that the dependency on /boost/headers is currently hard-coded, as it is the odd man out. The dependencies injected into the cmake files are now in the variant specific file, as that matches Boost.Build's model more accurately.

This somewhat reduces the recorded dependencies as it only tracks library dependencies. The original dependency list also includes header dependencies.

swatanabe commented 6 years ago

AMDG

On 10/23/2018 10:42 AM, Peter Dimov wrote:

I wasn't sure about this because in general it will find_dependency more than once (for multiconfig generators.)

Is that wrong or is it just unnecessary work?

In Christ, Steven Watanabe

swatanabe commented 6 years ago

AMDG

On 10/23/2018 10:45 AM, Peter Dimov wrote:

pdimov commented on this pull request.

 # Target install
  • generate install-dependencies : $(libraries) : @boost-install%generate-dependencies install ;

I suppose that if we use the boost-install%rulename syntax in the other places we can avoid exporting the generating rules in Jamroot?

Correct. This syntax is how Boost.Build tracks the module internally. If you just have @rulename, it will be converted to @$(current-project-module)%rulename.

In Christ, Steven Watanabe

pdimov commented 6 years ago

Is that wrong or is it just unnecessary work?

I don't know offhand; we'll see when you fix the include as now Appveyor fails because of it. :-)

pdimov commented 6 years ago

Hmm.

-- Found boost_system
--   libboost_system-vc140-mt-gd-x32-1_69.lib
-- Found boost_headers
--   libboost_system-vc140-mt-x32-1_69.lib

Something's not right here but I'm not sure what it is.

https://ci.appveyor.com/project/pdimov/boost-install/build/job/ehm3sy38h2g37hh6#L133

pdimov commented 6 years ago

Here too:

-- Found boost_filesystem
--   libboost_filesystem-vc140-mt-gd-x32-1_69.lib
-- Found boost_system
--   libboost_system-vc140-mt-gd-x32-1_69.lib
-- Found boost_headers
--   libboost_system-vc140-mt-x32-1_69.lib
--   libboost_filesystem-vc140-mt-x32-1_69.lib

and then

LINK : fatal error LNK1181: cannot open input file '\lib\libboost_filesystem-vc140-mt-x32-1_69.lib' [C:\projects\boost-root\tools\boost_install\test\filesystem\__build__\main.vcxproj]

https://ci.appveyor.com/project/pdimov/boost-install/build/job/imq0apy7t56u7ynq

swatanabe commented 6 years ago

AMDG

On 10/23/2018 12:12 PM, Peter Dimov wrote:

Here too:

-- Found boost_filesystem
--   libboost_filesystem-vc140-mt-gd-x32-1_69.lib
-- Found boost_system
--   libboost_system-vc140-mt-gd-x32-1_69.lib
-- Found boost_headers
--   libboost_system-vc140-mt-x32-1_69.lib
--   libboost_filesystem-vc140-mt-x32-1_69.lib

This appears to be a pre-existing problem:

https://ci.appveyor.com/project/pdimov/boost-install/builds/19654835/job/cl0lnhbj3fub97sp#L132

Both the debug and release configurations are matching, because neither Boost_USE_DEBUG_LIBS nor Boost_USE_RELEASE_LIBS is set.

"Found boost_headers" is just getting inserted in the middle, because the order of execution changed.

and then

LINK : fatal error LNK1181: cannot open input file '\lib\libboost_filesystem-vc140-mt-x32-1_69.lib' [C:\projects\boost-root\tools\boost_install\test\filesystem\__build__\main.vcxproj]

https://ci.appveyor.com/project/pdimov/boost-install/build/job/imq0apy7t56u7ynq

_IMPORT_PREFIX is clearly getting clobbered by find_dependency.

In Christ, Steven Watanabe

pdimov commented 6 years ago

Both debug and release matching is not a problem, it's by design. The question is why they aren't at their proper places. Being listed after -- Found boost_headers means that their variants are being included by the config for boost_headers, which isn't supposed to happen.

swatanabe commented 6 years ago

AMDG

Oh, now I see how debug/release is supposed to work. The output is a bit strange, but not a problem by itself. Does CMake have any way to handle variable scoping that will protect _IMPORT_PREFIX?

In Christ, Steven Watanabe

pdimov commented 6 years ago

_IMPORT_PREFIX is clearly getting clobbered by find_dependency.

What fun. Let's make it _BOOST_IMPORT_PREFIX then.

swatanabe commented 6 years ago

AMDG

On 10/23/2018 01:01 PM, Peter Dimov wrote:

_IMPORT_PREFIX is clearly getting clobbered by find_dependency.

What fun. Let's make it _BOOST_IMPORT_PREFIX then.

That may or may not help depending on whether the problem is find_dependency itself or the fact that boost_headers-config.cmake /also/ uses the name _IMPORT_PREFIX. I don't really understand CMake scoping rules.

In Christ, Steven Watanabe

pdimov commented 6 years ago

Ah. Right. find_dependency in the foreach includes the other config file which clobbers everything, not just _IMPORT_PREFIX. It's even funnier than I thought, then.

We have to collect the dependencies in a variable in the variant files, instead of using find_dependency there, then do the dependency loop outside.

I wonder how it worked before.

pdimov commented 6 years ago

Ah, it worked because I use a b2 loop and inline the find_dependency calls so there's nothing they can do at this point.

swatanabe commented 6 years ago

AMDG

On 10/23/2018 01:15 PM, Peter Dimov wrote:

We have to collect the dependencies in a variable in the variant files, instead of using find_dependency there, then do the dependency loop outside.

And make sure that we don't clobber the loop variables...

I wonder how it worked before.

It worked before because the dependencies were handled at the very end, outside the loop.

In Christ, Steven Watanabe

pdimov commented 6 years ago

And make sure that we don't clobber the loop variables...

Yes, we have to scope everything manually with BOOST$LIB, both the dependency list and the loop variable. :-/

swatanabe commented 6 years ago

AMDG

On 10/23/2018 01:22 PM, Peter Dimov wrote:

And make sure that we don't clobber the loop variables...

Yes, we have to scope everything manually with BOOST$LIB, both the dependency list and the loop variable. :-/

Can we get away with wrapping find_dependency in a function. We don't need to export any variables, right?

In Christ, Steven Watanabe

pdimov commented 6 years ago

Good idea, perhaps we could. With CMake, you never know until you try it. (On all possible versions.) It should in principle work according to the docs, but I'm not positive that we won't hit bugs in CMake because of the recursive call.

We could name the function boost_$lib_find_dependency though, that would avoid the recursive function redefinition.

pdimov commented 6 years ago

This almost worked, which is rather an indication of the inadequacy of my tests, considering that it doesn't seem to pass any dependent libraries to the linker. :-)

pdimov commented 6 years ago

(Since I made System header-only, it works without it being linked.)

swatanabe commented 6 years ago

AMDG

It looks like I got confused by LINK_INTERFACELIBRARIES${CONFIG} vs INTERFACE_LINK_LIBRARIES.

In Christ, Steven Watanabe

pdimov commented 6 years ago

Nice work, thanks.

It seems that we have a problem at the moment with buildable -> headers -> buildable dependencies, which I knew we had in theory, but it seems that we also have it in practice, in the form of serialization -> spirit -> thread. But that's to be fixed in Serialization's Jamfile I suppose, and you'll pick it up from there.