dmlc / xgboost

Scalable, Portable and Distributed Gradient Boosting (GBDT, GBRT or GBM) Library, for Python, R, Java, Scala, C++ and more. Runs on single machine, Hadoop, Spark, Dask, Flink and DataFlow
https://xgboost.readthedocs.io/en/stable/
Apache License 2.0
25.79k stars 8.69k forks source link

CHECK_* macro in logging.h collides with CHECK_* macro from absl #10400

Open hcho3 opened 1 month ago

hcho3 commented 1 month ago

https://buildkite.com/xgboost/xgboost-ci-multi-gpu/builds/5220#018ff0f5-6610-44e5-adac-0b3e9bcf298f/99-444:

/workspace/dmlc-core/include/dmlc/logging.h:211: error: "CHECK" redefined [-Werror]
  211 | #define CHECK(x)                                           \
      |
In file included from /opt/grpc/include/grpcpp/impl/server_callback_handlers.h:21,
                 from /opt/grpc/include/grpcpp/generic/async_generic_service.h:23,
                 from /workspace/build/plugin/federated/federated.grpc.pb.h:13,
                 from /workspace/tests/cpp/plugin/federated/../../../../plugin/federated/federated_comm.h:6,
                 from /workspace/tests/cpp/plugin/federated/test_federated_comm.cc:10:
/opt/grpc/include/absl/log/check.h:57: note: this is the location of the previous definition
   57 | #define CHECK(condition) ABSL_LOG_INTERNAL_CHECK_IMPL((condition), #condition)
      |

gRPC brings in Abseil C++, which defines its own CHECK_* macros. To avoid name collision, XGBoost's logging should use prefixed macros: XGB_CHECK, XGB_CHECK_LT etc.

trivialfis commented 2 weeks ago

Took a brief look into this for restoring the warning as error cmake option. It seems we need to build a part of the logger in xgboost to avoid symbols from dmlc.

hcho3 commented 1 week ago

@trivialfis Let's consider removing DMLC loggers and implementing the whole loggers inside XGBoost. And also: use prefixed macros like XGB_CHECK and XGB_LOG.

trivialfis commented 1 week ago

It's going to be a lot of work, we have to eliminate all dmlc headers that have logging in it. For example, we need to remove the dmlc parameter as well.