Data.table: Stop "blowing away" user-supplied PKG_* flags

Created on 5 Aug 2020  路  7Comments  路  Source: Rdatatable/data.table

Since #4640 is closes for now, this issue is to follow up so we don't forget.
See https://github.com/Rdatatable/data.table/issues/4640#issuecomment-668877557

Most helpful comment

@mattdowle I bet you didn't mean to at handle me :)

All 7 comments


Simon,

Please could you reply. I am working on next release of data.table to CRAN.

The open issue is here: https://github.com/Rdatatable/data.table/issues/4664

Best, Matt



Matt,

Blowing away = removing all content without replacement

When you write configure tests, you have to first map all flags to the variables that configure uses. At that point you should honour (=accept and handle appropriately, from dictionary: fulfil (an obligation) or keep (an agreement)) flags that the user supplied so that any special setup on that machine can be communicated to configure by the user. There are two most common ways: the regular autoconf behavior is to honour CPPFLAGS, the R way is to honour PKG_CPPFLAGS*. It prudent fold PKG_CPPFLAGS (if used) into CPPFLAGS, otherwise they may not be used for configure tests, so the tests are not realistic (at least if configure is generated using autoconf). Once you have determined the flags are working you can substitute them. Note that you cannot use FOO=$FOO @bar@ in makefile, you want to set bar accordingly before substitution.

So in a nutshell regular autoconf setup:

  • populate flags (e.g., CPPFLAGS) from R CMD config and user variables (e.g., CPPFLAGS and/or PKG_CPPFLAGS)
  • perform tests (adding to flags like CPPFLAGS as needed)
  • substitute the result into PKG_* flags in Makevars (e.g., PKG_CPPFLAGS=@CPPFLAGS@)

Example (autoconf excerpt just focusing on CPPFLAGS):

R_CPPFLAGS=`"${R_HOME}/bin/R" CMD config CPPFLAGS`;
CPPFLAGS="${R_CPPFLAGS} ${CPPFLAGS} ${PKG_CPPFLAGS}"
AC_LANG(C)
# test here
# ...
# finalise
AC_CONFIG_FILES([src/Makevars])

src/Makevars:

PKG_CPPFLAGS=@CPPFLAGS@

Since you asked about PKG_* reference see R-exts 1.2.1 - note that the example you quoted assumes you're not adding any additional configure flags, it only shows an example of a setup using base R scenario with no other dependencies which is why there are no additional flags.

The bottom line is that you should always allow the user to override flags in cases a special setup is required. You should never assume that your configure can figure it out unconditionally since you can't cover all special cases for everyone in the world. A good way to do that in a transparent manner is to use autoconf which documents the variables used:

R CMD INSTALL --configure-args=鈥攈elp myPackage_0.1.tar.gz
[鈥

Some influential environment variables:

   CC          C compiler command
   CFLAGS      C compiler flags
   LDFLAGS     linker flags, e.g. -L<lib dir> if you have libraries in a
               nonstandard directory <lib dir>
   LIBS        libraries to pass to the linker, e.g. -l<library>
   CPPFLAGS    (Objective) C/C++ preprocessor flags, e.g. -I<include dir> if
               you have headers in a nonstandard directory <include dir>
   CPP         C preprocessor

Use these variables to override the choices made by `configure' or to help
it to find libraries and programs with nonstandard names/locations.

You can also use your own variables of flags and document it, .e.g:

R CMD INSTALL --configure-args=--help PKI_0.1-6.tar.gz
[鈥

Some influential environment variables:

               optional path to the include directory for OpenSSL headers
   PKG_CPPFLAGS
               additional pre-processor flags
   PKG_LIBS    additional linker library flags
   CC          C compiler command
   CFLAGS      C compiler flags
   LDFLAGS     linker flags, e.g. -L<lib dir> if you have libraries in a
               nonstandard directory <lib dir>
   LIBS        libraries to pass to the linker, e.g. -l<library>
   CPPFLAGS    (Objective) C/C++ preprocessor flags, e.g. -I<include dir> if
               you have headers in a nonstandard directory <include dir>
   CPP         C preprocessor

Use these variables to override the choices made by configure or to help
it to find libraries and programs with nonstandard names/locations.

However, I would recommend also supporting the standard approach or else it makes it harder for users to figure out a consistent setup.

I hope it helps.

Cheers,
Simon

[*] the rationale is very simple: without configure R CMD INSTALL uses PKG_CPPFLAGS/PKG_LIBS to allow the user to specify the package-specific flags beyond base R. So accepting them through configure simply preserves that behavior.



Simon,

Thanks for the extended detail. As usual, I have copied this to the
GitHub issue so that the information is shared :
https://github.com/Rdatatable/data.table/issues/4664.

Your article you pointed me to originally
(https://mac.r-project.org/openmp/) contains :

but if the package adheres to the R rules, you can use ... Unfortuantely [sic], some packages don't

It seems that no such rule exists then. You pointed me to R-exts
1.2.1, but I don't see any rule there about not blowing away
environment variables, or similar wording. Not only does no such R
rule exist then, section 1.2.1.1 contains the instruction which I
followed.

Since you asked about PKG_* reference see R-exts 1.2.1 - note that the example you quoted assumes you're not adding any additional configure flags, it only shows an example of a setup using base R scenario with no other dependencies which is why there are no additional flags.

Then please improve that part of the manual to state that. The correct
method to set those variables should be used there.

Please do not accuse me, or anyone else in this community, of not
adhering to R rules, when not only doesn't any such rule exists, but
an instruction to blow away environment variables is contained in the
manual, which I followed. Please either correct that wording in your
article, or point me to the line where that rule is stated, as I
asked.

Best, Matt


@mattdowle I bet you didn't mean to at handle me :)

From Simon


Matt, as usual you're missing the point and are attacking the person helping you instead of trying to understand. Since it confirms that all my efforts are wasted on you, I'll reserve those for people that are able to extract value from it.
All the best in your future endeavours,
Simon



Simon,
No you are missing the point.
I won't stand to be bullied by you.
Don't accuse me of breaking an R rule, that doesn't exist.
Your attempts to twist and distract from that don't work on me.
Good bye Simon.
Matt


I spoke to Simon and he has now changed the wording in the article, https://mac.r-project.org/openmp/.
The wording about breaking R rules is gone, and it now reads much better:

How you do the latter depends on the package, but if the package does not set these environment variables itself, you can try
PKG_CPPFLAGS='-Xclang -fopenmp' PKG_LIBS=-lomp R CMD INSTALL myPackage
If that doesn't work, please consult the package's documentation, and liaise with its maintainer.

Was this page helpful?
0 / 5 - 0 ratings