Cross compiling fixes, improvements and additions #128

eigendude · 2020-03-08T02:38:49Z

Description of the Change

This PR contains the build system fixes, improvements and additions broken out from #114.

Structure of the PR

The PR is organized as follows:

Fixes needed for cross-compiling
Improvements needed for cross-compiling

Fixes for cross-compiling

Commits:

~~CMake: Fix error including generated protobuf headers~~
~~Fix tinycbor include directory~~

These commits come first, because they address what could be considered defects. Inclusion would be valuable even if the rest of the PR is omitted. Further description is in the commit message.

Changes for cross-compiling

Commits:

These changes enable more control over the build process when embedded in another build system.

CMake is controlled by user variables; the first one we need is -DHUNTER_ENABLED=OFF so that we can have control over dependencies.

Another useful variable is -DTESTING=OFF, because embedding test infra drastically increases the up-front effort. The first two commits above allow this variable to exclude test infra from the dependency discovery in addition to the build.

Finally, I introduced a new variable: -DBUILD_INTERNAL_DEPS=OFF. This causes the in-tree dependencies to be skipped. I needed this for a build system that doesn't support git submodules. Instead, the dependencies are provided by the build sytsem.

Dependencies

Cross compiling fixes and improvements libp2p/cpp-libp2p#59 needed for libp2p dependency.

How has this been tested?

Temporarily disabled proofs and ran make from Kodi's build deps. Build succeeds.

Ran make install:

make[2]: *** No rule to make target 'install'.  Stop.

Next PR will address this.

igor-egorov

Looks good except the comment below

I am not approving while the review of libp2p/cpp-libp2p#59 is in progress

igor-egorov · 2020-03-10T12:06:23Z

cmake/dependencies.cmake


 # https://docs.hunter.sh/en/latest/packages/pkg/Boost.html
 hunter_add_package(Boost COMPONENTS date_time filesystem random system)
 find_package(Boost CONFIG REQUIRED date_time filesystem random system)

 # https://docs.hunter.sh/en/latest/packages/pkg/Microsoft.GSL.html
 hunter_add_package(Microsoft.GSL)
-find_package(Microsoft.GSL CONFIG REQUIRED)


According to the Hunter docs (or here), find_package is still required.

Could you please check if the code below works in your environment?
(let's keep find_package without CONFIG)

-find_package(Microsoft.GSL CONFIG REQUIRED) +find_package(Microsoft.GSL REQUIRED)

I remember trying this line with no luck. CMake has no FindMicrosoft.GSL.cmake, but if I add an empty one, it works. The same with cppcodec - we don't use the output CPPCODEC_FOUND anywhere, I just added the pkg-config check because I noticed cppcodec provided pkg-config files.

Does Hunter do any magic by providing found package info to CMake? If I removed all find_package() calls, I suspect fuhon would still build, does this sound weird?

According to the faq, hunter_add_package(foo) will download and install foo package while find_package(foo) should only look up for files in read-only mode.

find_package is a CMake builtin function, which loads package settings. Thus, find_package looks to be required while even when hunter is not used.

Dropped this commit

eigendude · 2020-03-10T22:40:36Z

libp2p/cpp-libp2p#59 is declared as a dependency in the PR description. I updated the bullet to a check box so we can see that the dependency is fulfilled directly from this PR.

eigendude · 2020-03-18T12:19:56Z

I updated libp2p/cpp-libp2p#59 and this PR. I removed all CONFIG commits. Also dropped a fix not needed after the rust BLS signature library was dropped in #129. Only 5 commits left in this PR now.

eigendude · 2020-03-24T12:09:17Z

Is there more to do on the PR?

codecov · 2020-04-05T02:41:31Z

Codecov Report

Merging #128 (b7db528) into master (ad026fd) will increase coverage by 0.02%.
The diff coverage is 61.53%.

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   46.00%   46.02%   +0.02%     
==========================================
  Files         713      715       +2     
  Lines       32293    32328      +35     
  Branches    17876    17885       +9     
==========================================
+ Hits        14855    14880      +25     
- Misses      13095    13102       +7     
- Partials     4343     4346       +3

Impacted Files	Coverage Δ
...re/markets/storage/provider/impl/provider_impl.hpp	`36.84% <ø> (ø)`
core/sector_storage/impl/manager_impl.hpp	`33.33% <ø> (ø)`
core/sector_storage/impl/remote_worker.cpp	`0.00% <0.00%> (ø)`
core/sector_storage/worker.hpp	`62.50% <ø> (ø)`
core/storage/piece/impl/piece_storage_impl.cpp	`47.94% <0.00%> (-2.06%)`	⬇️
...re/markets/storage/provider/impl/provider_impl.cpp	`62.00% <16.66%> (-0.53%)`	⬇️
core/miner/storage_fsm/impl/sealing_impl.cpp	`43.37% <50.00%> (+0.04%)`	⬆️
core/common/vector_cow.hpp	`72.72% <72.72%> (ø)`
core/sector_storage/impl/manager_impl.cpp	`61.04% <75.00%> (+0.40%)`	⬆️
core/sector_storage/impl/local_worker.cpp	`71.14% <87.50%> (+0.14%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f37400...b7db528. Read the comment docs.

igor-egorov · 2020-11-17T15:23:48Z

Hi @eigendude and sorry for the long waiting
It looks the time to merge the pr has come!
Could you please update the pr and resolve the merge conflicts?

eigendude · 2020-12-01T01:04:24Z

Upstream libp2p/cpp-libp2p#59 has been merged! I plan to rebase this sometime before the end of the year.

eigendude · 2022-02-16T16:28:07Z

Back from the dead! I rebased the PR and compile-tested. I dropped the tinycbor header change, it's no longer needed. I also added a commit removing the c-ares CMake declaration, as I can't find a use throughout the codebase.

This change shuffles the order of several CMake steps to expose CMake options to included files. By defining options before inclusion, included files are able to access the input provided by the build system.

When building without tests, GTest and GMock are not needed. This change allows build systems to continue with filecoin compilation without devoting resources to its testing infrastructure.

Not every build system can handle git submodules, so expose an option to exclude these dependencies from the build

When building without Hunter, it is desirable to pass the path to protoc and the protobuf include directory from the build system. Allow these variables to be overridden.

Signed-off-by: Garrett Brown <[email protected]>

eigendude mentioned this pull request Mar 8, 2020

Embedded Filecoin: Enable cross-compiling #114

Open

24 tasks

zuiris requested review from Alexey-N-Chernyshov and igor-egorov March 10, 2020 09:21

igor-egorov reviewed Mar 10, 2020

View reviewed changes

Alexey-N-Chernyshov approved these changes Mar 10, 2020

View reviewed changes

eigendude force-pushed the cross-compile-fixes branch 2 times, most recently from 155259e to aca6b56 Compare March 18, 2020 12:17

eigendude force-pushed the cross-compile-fixes branch from aca6b56 to 2654e41 Compare April 5, 2020 02:27

igor-egorov requested a review from turuslan November 17, 2020 15:26

eigendude force-pushed the cross-compile-fixes branch 2 times, most recently from 975417f to 67460ed Compare February 16, 2022 16:24

Garrett Brown added 5 commits February 16, 2022 08:33

CMake: Expose CMake options to includes

bdbdf7e

This change shuffles the order of several CMake steps to expose CMake options to included files. By defining options before inclusion, included files are able to access the input provided by the build system.

CMake: Exclude test-related depends when building tests

19e9b6f

When building without tests, GTest and GMock are not needed. This change allows build systems to continue with filecoin compilation without devoting resources to its testing infrastructure.

CMake: Allow excluding git submodule depends

53a168b

Not every build system can handle git submodules, so expose an option to exclude these dependencies from the build

CMake: Allow build system to provide protobuf compiler

de30c31

When building without Hunter, it is desirable to pass the path to protoc and the protobuf include directory from the build system. Allow these variables to be overridden.

CMake: Remove c-ares dependency

174b696

eigendude force-pushed the cross-compile-fixes branch from 67460ed to 174b696 Compare February 16, 2022 16:33

turuslan approved these changes Feb 16, 2022

View reviewed changes

Alexey-N-Chernyshov approved these changes Feb 17, 2022

View reviewed changes

Merge branch 'master' into cross-compile-fixes

b7db528

turuslan merged commit 03fd9bd into filecoin-project:master Feb 18, 2022

eigendude deleted the cross-compile-fixes branch February 18, 2022 15:08

eigendude mentioned this pull request Feb 18, 2022

CMake: Improve building without hunter #601

Merged

Markuu-s pushed a commit that referenced this pull request Mar 10, 2022

Cross compiling fixes, improvements and additions (#128)

48c9450

Signed-off-by: Garrett Brown <[email protected]>

Markuu-s pushed a commit that referenced this pull request Mar 10, 2022

Cross compiling fixes, improvements and additions (#128)

ebb14d3

Signed-off-by: Garrett Brown <[email protected]>

Markuu-s pushed a commit that referenced this pull request Mar 10, 2022

Cross compiling fixes, improvements and additions (#128)

913a10f

Signed-off-by: Garrett Brown <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cross compiling fixes, improvements and additions #128

Cross compiling fixes, improvements and additions #128

Uh oh!

eigendude commented Mar 8, 2020 •

edited

Loading

Uh oh!

igor-egorov left a comment

Uh oh!

igor-egorov Mar 10, 2020 •

edited

Loading

Uh oh!

eigendude Mar 10, 2020 •

edited

Loading

Uh oh!

igor-egorov Mar 11, 2020

Uh oh!

eigendude Mar 18, 2020

Uh oh!

eigendude commented Mar 10, 2020

Uh oh!

eigendude commented Mar 18, 2020

Uh oh!

eigendude commented Mar 24, 2020

Uh oh!

codecov bot commented Apr 5, 2020 •

edited

Loading

Uh oh!

igor-egorov commented Nov 17, 2020

Uh oh!

eigendude commented Dec 1, 2020

Uh oh!

eigendude commented Feb 16, 2022

Uh oh!

Uh oh!

Cross compiling fixes, improvements and additions #128

Cross compiling fixes, improvements and additions #128

Uh oh!

Conversation

eigendude commented Mar 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the Change

Structure of the PR

Fixes for cross-compiling

Changes for cross-compiling

Dependencies

How has this been tested?

Uh oh!

igor-egorov left a comment

Choose a reason for hiding this comment

Uh oh!

igor-egorov Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eigendude Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igor-egorov Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

eigendude Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

eigendude commented Mar 10, 2020

Uh oh!

eigendude commented Mar 18, 2020

Uh oh!

eigendude commented Mar 24, 2020

Uh oh!

codecov bot commented Apr 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

igor-egorov commented Nov 17, 2020

Uh oh!

eigendude commented Dec 1, 2020

Uh oh!

eigendude commented Feb 16, 2022

Uh oh!

Uh oh!

eigendude commented Mar 8, 2020 •

edited

Loading

igor-egorov Mar 10, 2020 •

edited

Loading

eigendude Mar 10, 2020 •

edited

Loading

codecov bot commented Apr 5, 2020 •

edited

Loading