Patching in ExternalProject_Add can lead to CI issues
We (in ATLAS) have been seeing a recurring issue in our CI system since a while, which I think I understand to some degree by now. And which seems to come from a not-entirely-perfect behaviour in ExternalProject_Add
.
One of our projects is used to build a whole bunch of external software for us, which our downstream projects can then use.
https://gitlab.cern.ch/atlas/atlasexternals
Some of the externals we need to patch for one reason or another. For instance we patch the FastJet source code as well, to make it build with the latest version of Clang.
https://gitlab.cern.ch/atlas/atlasexternals/blob/master/External/FastJet/CMakeLists.txt
The simplified version of this is the following:
# Set the minimum required CMake version:
cmake_minimum_required( VERSION 3.2 )
# Set the project's name:
project( PatchBugTest )
# Build FastJet, with a patch on top of it:
include( ExternalProject )
ExternalProject_Add( FastJet
PREFIX ${CMAKE_BINARY_DIR}
URL http://fastjet.fr/repo/fastjet-3.2.2.tar.gz
URL_MD5 058367d96052f99d6347027a2c39ab4a
INSTALL_DIR ${CMAKE_BINARY_DIR}/fastjet
PATCH_COMMAND patch -p1 < ${CMAKE_CURRENT_SOURCE_DIR}/fastjet_clang.patch
CONFIGURE_COMMAND <SOURCE_DIR>/configure --prefix=<INSTALL_DIR>
--enable-shared --disable-static --disable-debug
BUILD_IN_SOURCE 1
BUILD_COMMAND make
INSTALL_COMMAND make install )
diff -ur fastjet-3.2.2-orig/plugins/SISCone/SISConeBasePlugin.cc fastjet-3.2.2-fixed/plugins/SISCone/SISConeBasePlugin.cc
--- fastjet-3.2.2-orig/plugins/SISCone/SISConeBasePlugin.cc 2017-05-22 14:03:28.000000000 +0200
+++ fastjet-3.2.2-fixed/plugins/SISCone/SISConeBasePlugin.cc 2017-08-31 13:39:20.743720919 +0200
@@ -9,8 +9,8 @@
// By default this does a simple direct comparison but it can be
// overloaded for higher precision [recommended if possible]
bool SISConeBasePlugin::UserScaleBase::is_larger(const PseudoJet & a, const PseudoJet & b) const{
- return a.structure_of<UserScaleBase::StructureType>().ordering_var2()
- > b.structure_of<UserScaleBase::StructureType>().ordering_var2();
+ return a.structure_of<UserScaleBase>().ordering_var2()
+ > b.structure_of<UserScaleBase>().ordering_var2();
}
FASTJET_END_NAMESPACE
Now, this build as-is just fine. But in our CI system, where we re-use the the build area (since building our entire software takes a long time, we don't want to do it from scratch all the time), it can happen that in a new build the patch command gets removed, and then added back again. You can try this by:
- Building the example configuration above as-is;
- Comment out the
PATCH_COMMAND
line, and building it again (just executingmake
for instance); - Enable the
PATCH_COMMAND
line again, and executemake
once again.
In the last step one gets an error from the patching.
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/krasznaa/Development/cmake/build
[ 12%] Performing patch step for 'FastJet'
patching file plugins/SISCone/SISConeBasePlugin.cc
Reversed (or previously applied) patch detected! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file plugins/SISCone/SISConeBasePlugin.cc.rej
make[2]: *** [src/FastJet-stamp/FastJet-patch] Error 1
make[1]: *** [CMakeFiles/FastJet.dir/all] Error 2
make: *** [all] Error 2
Because the source area is kept as-is between the builds. And between the first and third builds the build area forgets that this patching was already done.
I imagine that the correct logic here should be that whenever the patching command changes, even if the sources were already downloaded/uncompressed earlier, they would have to be uncompressed from scratch once more. Otherwise how can one expect to for instance extend a patching command, and still keep the incremental build functional?
Am I wrong on this? Are we using the patching feature of ExternalProject_Add
incorrectly?
Cheers, Attila