В Tue, 29 Apr 2025 14:43:40 +0200
Nils Lüschow <nils.luesc...@uni-duesseldorf.de> пишет:

> While we generally got this feature to work when Gecode is
> pre-installed on the system, we would like to spare the users this
> step and include it with the package

It's probably a good idea to try to achieve both. If a system copy of
the package is available, link with it; otherwise build the bundled
copy. Unfortunately, this kind of user convenience involves a lot of
testing to make sure that both configurations work as intended.
Bundling dependencies also creates headaches for distro packagers.

> However, we run into a problem when it comes to compiling Gecode
> during the packages build-process, which I describe in this SO post

One problem is that when using $(MAKE) to compile gecode, various
variables used by gecode's Makefile are overridden through the
MAKEFLAGS variable. For example, $(CXX) ends up being defined as
$(CXX20) $(CXX20STD) without the variables CXX20 and CXX20STD being
available to the downstream Make process. What's worse is CXXFLAGS are
also overridden, losing the include paths in the process.

In order to build gecode from Makevars, you'll have to disable these
overrides by emptying MAKEFLAGS:

build_gecode:
        cd $(GECODE_DIR) && \
        ./configure CC="$(CC)" CXX="$(CXX20) $(CXX20STD)" \
                --disable-examples --enable-static --disable-shared
        $(MAKE) -C $(GECODE_DIR) MAKEFLAGS=

(Alternatively, you can ./configure and build the library from the
./configure script of your package.)

Other changes are also needed to make the package build in a portable
manner:

 - Recipe lines in Makefiles must start with tabs, not groups of
   four spaces.
 - Don't use GNU Make append syntax (+=) without declaring
   SystemRequirements: GNU Make.
 - Don't set compiler-specific flags such as -mtune=native without at
   least testing that the compiler understands them.
 - With the current directory structure, GECODE_DIR must be just
   gecode, not $(CURDIR)/gecode-release-6.3.0.
 - Since the C++ source code already uses #include <gecode/...>, the
   include path must be -I$(GECODE_DIR), not -I$(GECODE_DIR)/gecode.
 - Don't hard-code compiler names. Instead of ./configure CC=gcc ...,
   use ./configure CC="$(CC)" CXX="$(CXX20) $(CXX20STD)".
 - Build gecode as a static library and link it into the package
   shared library. Otherwise you'll need to arrange for the gecode
   shared libraries to be installed together with the R package and
   properly loaded before your package shared library.
 - Provide a 'clean' target so that R CMD build doesn't include the
   binaries from the gecode subdirectory into the source package.

With that, we get the following Makevars:

all: $(SHLIB)

GECODE_DIR = gecode
PKG_CPPFLAGS = -I$(GECODE_DIR)
PKG_LIBS = -L$(GECODE_DIR) -lgecodesearch -lgecodeminimodel \
           -lgecodeint -lgecodekernel -lgecodesupport
CXX_STD = CXX20

$(SHLIB): gecode_built
# package code relies on header files only available after gecode is
# configured and probably built
$(OBJECTS): gecode_built

gecode_built:
        cd $(GECODE_DIR) && \
                ./configure CC="$(CC)" CXX="$(CXX20) $(CXX20STD)" \
                --disable-examples --enable-static --disable-shared \
                --disable-flatzinc --disable-qt
        $(MAKE) -C $(GECODE_DIR) MAKEFLAGS=
        touch $@

clean:
        -cd $(GECODE_DIR) && make clean
        -rm -f gecode_built

Hopefully, the Makevars.win with same contents but
--with-host-os=windows added to the configure flags will work on
Windows. (It needs to be .win, not .ucrt, because your package declares
support for R > 3.6, which predates use of UCRT in R. To think of it,
R-3.6.0 also predates C++20...) You can probably give some more
--disable-... arguments to ./configure to remove some more components
that the R package doesn't use.

However, this is only the beginning of the troubles.

R packages are not allowed file path lengths above 100 bytes, and your
package has seven that are longer, e.g.,

>> RcppTestPackage/src/gecode/gecode/third-party/boost/numeric/interval/detail/x86gcc_rounding_control.hpp

Maybe you can unbundle the bits of Boost and rely on the 'BH' package
instead. Maybe there's a way to rename the files and patch the includes
in the source code.

The gecode library produces a lot of warnings, and R CMD check
considers some of them serious:

>> Found the following significant warnings:
>> ./gecode/kernel/core.hpp:4102:24: warning: array subscript -1 is
>> below array bounds of ‘unsigned int [1]’ [-Warray-bounds]
>> ./gecode/kernel/core.hpp:4109:17: warning: array subscript -1 is
>> below array bounds of ‘unsigned int [1]’ [-Warray-bounds]
>> ./gecode/support/allocator.hpp:88:11: warning: ‘void free(void*)’
>> called on pointer returned from a mismatched allocation function
>> [-Wmismatched-new-delete]                 

The -Warray-bounds warnings might be false positives (could pc_max ever
be 0?), but -Wmismatched-new-delete is probably real; the compiler
shows a place where an object allocated using new(...) can be
deallocated using free(...).

Some parts of the gecode library contain references to std::cerr, and
the R package code calls rand(). These are not recommended in R
packages, because the standard error stream isn't necessarily connected
to the terminal and because rand() is disconnected from R's random
number generator.

Once you build the whole gecode yourself, be ready for it being tested
with AddressSanitizer and UndefinedBehaviorSanitizer. Things like
free(new(...)) that were previously invisible to them (due to the
third-party library being compiled without sanitizers) will now be
revealed (and become yours to fix instead of just something happening
upstream).

There are other problems noted by R CMD check, but they should be
easier to fix.

-- 
Best regards,
Ivan

______________________________________________
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel

Reply via email to