Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild for abseil_cpp20211102 #7

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This PR has been triggered in an effort to update abseil_cpp20211102.

Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/1847610558, please use this URL for debugging.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@BastianZim
Copy link
Member

@conda-forge-admin, please rerender

number: 0
skip: true # [not linux]
number: 1
skip: true # [win]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BastianZim BastianZim added the automerge Merge the PR when CI passes label Feb 15, 2022
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: failed

Thus the PR was not passing and not merged.

@BastianZim BastianZim removed the automerge Merge the PR when CI passes label Feb 15, 2022
@BastianZim

This comment was marked as outdated.

@wolfv
Copy link
Member

wolfv commented Feb 17, 2022

Ah, cool, I just wanted to link to the recent conda-forge discussion but it was you who asked on gitter :)

@BastianZim
Copy link
Member

😄 I'm just waiting on upstream and will otherwise just increment and test it.

@BastianZim
Copy link
Member

@conda-forge-admin, please rerender

@BastianZim BastianZim force-pushed the rebuild-abseil_cpp20211102-0-1_h0e16de branch from 91ff606 to 6ce5eaa Compare February 17, 2022 19:50
@BastianZim
Copy link
Member

@wolfv Would you mind having a look again? It's now the correct SDK but it's hitting an Undefined symbols for architecture x86_64 error which I'm not sure how to solve.

@BastianZim
Copy link
Member

Also, I'm going to bot-rerun this before merging, just to get a clean git history.

@BastianZim
Copy link
Member

@wolfv Gentle ping here, if you have a sec. :)

@BastianZim BastianZim mentioned this pull request Feb 21, 2022
@BastianZim
Copy link
Member

@wolfv Quick ping here :)

@wolfv
Copy link
Member

wolfv commented Mar 9, 2022

maybe @Mizux has an idea?

I think what changed is that we switched to C++ 17 on macOS? And another Abseil version?

The error is

[ 97%] Linking CXX shared library lib/libortools.dylib
ld: warning: -pie being ignored. It is only used when linking a main executable
Undefined symbols for architecture x86_64:
  "CoinMessageHandler::setLogLevel(int)", referenced from:
      operations_research::CLPInterface::Solve(operations_research::MPSolverParameters const&) in clp_interface.cc.o
  "CoinMessageHandler::setLogLevel(int, int)", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
      operations_research::CLPInterface::Solve(operations_research::MPSolverParameters const&) in clp_interface.cc.o
  "CoinMessageHandler::CoinMessageHandler()", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
      operations_research::CLPInterface::Solve(operations_research::MPSolverParameters const&) in clp_interface.cc.o
  "CoinMessageHandler::~CoinMessageHandler()", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
      operations_research::CLPInterface::Solve(operations_research::MPSolverParameters const&) in clp_interface.cc.o
  "CoinBuild::addRow(int, int const*, double const*, double, double)", referenced from:
      operations_research::CLPInterface::ExtractNewConstraints() in clp_interface.cc.o
  "CoinBuild::CoinBuild()", referenced from:
      operations_research::CLPInterface::ExtractNewConstraints() in clp_interface.cc.o
  "CoinBuild::~CoinBuild()", referenced from:
      operations_research::CLPInterface::ExtractNewConstraints() in clp_interface.cc.o
  "CoinModel::addRow(int, int const*, double const*, double, double, char const*)", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
  "CoinModel::addColumn(int, int const*, double const*, double, double, double, char const*, bool)", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
  "CoinModel::CoinModel()", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
  "CoinModel::~CoinModel()", referenced from:
      operations_research::CBCInterface::Solve(operations_research::MPSolverParameters const&) in cbc_interface.cc.o
ld: symbol(s) not found for architecture x86_64

@Mizux
Copy link

Mizux commented Mar 9, 2022

https://github.com/google/or-tools/blob/dd28bdf6e56f1d71dfc33a77657a99c69dc32efb/cmake/dependencies/CMakeLists.txt#L179 ?
this may explain the bad build.

@wolfv did you try an otool -l on ortools.dylib to check if we effectively depends on system install of CoinOR (Cbc, Clp, Cgl, Osi, CoinUtils) ?

BTW in or-tools we use my CoinOR forks which are CMake based build (when building coinor as static lib to ship a standalone ortools library)

devNote:

%grepc -RIin "CoinModel::addRow(int" *-src
coinutils-src/CoinUtils/src/CoinModel.cpp:717:void CoinModel::addRow(int numberInRow, const int *columns,

EDIT: Maybe found the error with my last commit in FindCoinUtils.cmake
https://github.com/google/or-tools/blame/1b1cbc4fa5de40be9c7bf8180d5e8a1ed1f6b9a0/cmake/FindCoinUtils.cmake#L40-L42
s/UTILS/Utils/g -_-

@h-vetinari
Copy link
Member

The new abseil builds in conda-forge use C++17 on unix. Unfortunately, abseil is ABI-sensitive to the C++ standard version used to compile, and each consuming library needs to match that. See conda-forge/abseil-cpp-feedstock#29

@BastianZim
Copy link
Member

BastianZim commented Mar 10, 2022

Yeah this comes from @Mizux comment and the resulting discussion in: conda-forge/staged-recipes#16147 (comment)

@h-vetinari
Copy link
Member

Have you tried setting -DCMAKE_CXX_STANDARD=17 in build-cpp.sh?

@BastianZim
Copy link
Member

Have you tried setting -DCMAKE_CXX_STANDARD=17 in build-cpp.sh?

No, but I am slightly hesitant to hard-code these things. Let me try though and see.

@h-vetinari
Copy link
Member

h-vetinari commented Mar 10, 2022

No, but I am slightly hesitant to hard-code these things.

Global C++ standard flags change very rarely. Or at the very least, the next jump to C++20 will probably still take a loooooong time, not least because clang is still far behind conformance (much less through the CWG/LWG issues) even on main (and all the compiler diffusion timelines that implies). GCC also hasn't buttoned up modules yet.

And in this case, you should IMO be explicit rather than relying on CMake/compiler defaults, because the abseil-ABI forces you to match.

@Mizux
Copy link

Mizux commented Mar 10, 2022

FYI: For or-tools we are moving to c++20 because we have few designated initializers in struct so compilation should works with a gcc 8.
note: for make based build we require gcc 10+ because we hard code -std=c++20 while it is call c++2a on gcc <= 9 (CMake know it and do it nicely)
ref: https://en.cppreference.com/w/cpp/language/aggregate_initialization

IIRC gcc/g++ with -std=c++17 still manage to compile or-tools (with this C++20 feature) while Visual Studio is more pedantic and fail...

@h-vetinari
Copy link
Member

OK, evidently this didn't work. 😑

The symbol collisions still seem to come from else-where (coin-or-clp & coin-or-cbc); maybe we'd need to compile those in C++17 too...? 🤔

@BastianZim
Copy link
Member

BastianZim commented Mar 10, 2022

The symbol collisions still seem to come from else-where (coin-or-clp & coin-or-cbc); maybe we'd need to compile those in C++17 too...? 🤔

That could make sense. Are you working on them anyways? Otherwise, I'd just rerender them and add that (Not a member there though)

Edit: Just remembered that they're not CMake based so well need to adapt that.

@BastianZim
Copy link
Member

IIRC gcc/g++ with -std=c++17 still manage to compile or-tools (with this C++20 feature) while Visual Studio is more pedantic and fail...

Well, it's working for Linux but I'm not sure if something is hardcoded somewhere in the conda-forge infrastructure. 😓

@h-vetinari
Copy link
Member

but I'm not sure if something is hardcoded somewhere in the conda-forge infrastructure.

No it's not; there was some discussion in conda-forge/clang-compiler-activation-feedstock#17, but it's left to the feedstocks. Abseil is a bit special because the ABI changes based on the standard version, which is not usually what happens.

@BastianZim
Copy link
Member

Ahh ok, thanks for the pointers. From what I can see you're doing some stuff in the other feedstocks. I'll wait then for that ok?

@h-vetinari
Copy link
Member

FYI: For or-tools we are moving to c++20 because we have few designated initializers

Just FYI in turn: using abseil with C++20 mode is subject to as-yet undetermined breakage: abseil/abseil-cpp#1127

@Mizux
Copy link

Mizux commented Mar 14, 2022

FYI: For or-tools we are moving to c++20 because we have few designated initializers

Just FYI in turn: using abseil with C++20 mode is subject to as-yet undetermined breakage: abseil/abseil-cpp#1127

Well in turn out that bazel is not compatible is any project that don't use the same C++ dialect for all of its sub-components (aka a project in C++14 and an other in C++17 may also break when trying to merge both in your super project build...), for Homebrew I manage to move "every packages" to C++17, it seems that or-tools is compilable with c++17 when using gcc (11?) or clang (13?) since they are not picky when using "designated initializers" so would suggest to replace lines
https://github.com/google/or-tools/blob/525162feaadaeef640783b2eaea38cf4b623877f/CMakeLists.txt#L15 and
https://github.com/google/or-tools/blob/525162feaadaeef640783b2eaea38cf4b623877f/cmake/dependencies/CMakeLists.txt#L307 to C++17 and see if it works with your build system (i.e.compiler) stack....

@BastianZim
Copy link
Member

Ok, thanks for investigating @Mizux. Just to be clear: Do you just want the two lines changed or something else as well?

@BastianZim
Copy link
Member

As we now have the new release, I'm going to re-run this so that we can get the new release out first and then try to enable OSX later.

@BastianZim BastianZim added the bot-rerun Instruct the bot to retry the PR label Mar 17, 2022
@regro-cf-autotick-bot
Copy link
Contributor Author

Due to the bot-rerun label I'm closing this PR. I will make another one as appropriate. This was generated by https://github.com/regro/autotick-bot/actions/runs/1999485720

@regro-cf-autotick-bot regro-cf-autotick-bot deleted the rebuild-abseil_cpp20211102-0-1_h0e16de branch March 17, 2022 16:22
@Mizux
Copy link

Mizux commented Mar 17, 2022

Ok, thanks for investigating @Mizux. Just to be clear: Do you just want the two lines changed or something else as well?

Well, currently we revert to C++17 the CMAKE_CXX_STANDARD otherwise it would imply to also bump abseil-cpp and all related projects (aka big bang). According to our CI it seems to pass even if there are some c++20 features in use 🤷

note: only VS2022 spot it and complained so if CMake detect MSVC generator it will set CXX_STANDARD to 20 otherwise it will stick to 17 like before.
TLDR: nothing to done on your side, should works as in v9.2...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot-rerun Instruct the bot to retry the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants