tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: math/R Makeconf fix



> On Dec 17, 2024, at 04:35, Jonathan Perkin <jperkin%mnx.io@localhost> wrote:
> 
> * On 2024-12-16 at 18:53 GMT, Brook Milligan wrote:
> 
>> I would like to commit the following patch to fix a bug in Makeconf.in for the math/R package.
>> 
>> Makeconf (made from Makeconf.in) is a Makefile fragment that R uses to build packages or other software from within R.  Currently, it passes rpath directories to the linker using -Wl,-R.  This fails on Darwin and should be replaced by -Wl,-rpath.  While at it, the patch also uses the same argument to -rpath as used to -L.
>> 
>> This should not affect building R itself or R packages within pkgsrc.
>> 
>> Please confirm that this is OK during the freeze.
> 
> Not ok. This is in common code and will just end up breaking OS that
> support -R but not -rpath.
> 
> We have COMPILER_RPATH_FLAG for this to be done properly, but I would
> prefer this just wait until after the freeze as there is far too much
> potential for breakage.

So why don’t we replace the hardcoded -Wl,-R with ${COMPILER_RPATH_FLAG}?  It seems that this is the correct fix and would be very low risk; there are 725 uses of COMPILER_RPATH_FLAG in pkgsrc, so I expect it is always set correctly.  I have verified that R builds fine on Darwin with COMPILER_RPATH_FLAG in Makeconf and that the resulting R builds programs correctly (which it does not without that change).

Also, just to be clear, the Makeconf file is not used by pkgsrc, neither to build R nor to build R packages.  As far as I can tell, it is only used within R when some R code is trying to compile third-party code.  For example, the R-nimble package creates files and then compiles them from within R; that process uses Makeconf (and fails on Darwin without this change).

Given this, I feel that the risk here is tiny and that we have a bug to fix.  Your call, though.

Cheers,
Brook



Home | Main Index | Thread Index | Old Index