dmlc / dmlc-core

A common bricks library for building scalable and portable distributed machine learning.
Apache License 2.0
864 stars 520 forks source link

Running CMake should not leave unclean git trace #551

Closed hcho3 closed 5 years ago

hcho3 commented 5 years ago

Currently, running a build with CMake will overwrite the pre-existing header include/dmlc/build_config.h, leaving a dirty git trace. The reason for this behavior is described in https://github.com/dmlc/dmlc-core/pull/432#pullrequestreview-230400171

Unclean git trace is inconvenient. For example, let's say you are working with the XGBoost project contains dmlc-core as a Git submodule. At the root of the XGBoost directory, you run

git checkout feature_branch

to switch to feature_branch. This won't work, because the dmlc-core submodule had been modified by the last run of CMake. You'd then have to manually clean out dmlc-core by running

cd dmlc-core
git checkout -- include/dmlc/build_config.h
cd ..

Related: apache/incubator-mxnet#15575

Fix: CMake should create a brand new header that's not under version control. The header under version control is now called include/dmlc/build_config_default.h, and CMake will generate include/dmlc/build_config.h. The generated header include/dmlc/build_config.h is also registered in .gitignore, so running CMake will not leave dirty git trace.

Detail: CMakeLists.txt will define the symbol DMLC_CORE_USE_CMAKE to indicate that dmlc-core is being built with CMake, not GNU Make. This symbol won't appear when the user opts for GNU Make.

cc @larroy @apeforest

hcho3 commented 5 years ago

@sriramch @rongou See if this would improve your development routine.

hcho3 commented 5 years ago

@apeforest After this is merged, I will file a pull request to MXNet to upgrade 3rdparty/dmlc-core submodule. No change in MXNet codebase will be necessary.

hcho3 commented 5 years ago

@larroy GNU Make uses dmlc/build_config_default.h.

sriramch commented 5 years ago

@hcho3 thanks for informing. i haven't face this issue (yet) as i have been using gnu make thus far...

rongou commented 5 years ago

@hcho3 Thanks for fixing this! It's been so annoying. :)