Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggested small fix to account for main architecture being a sub architecture #116

Closed
BerwinTurlach opened this issue Mar 10, 2024 · 13 comments

Comments

@BerwinTurlach
Copy link
Contributor

G'day Dirk,

from a johnny-come-lately to littler, and probably really late as sub-architectures for R seem to be on the way out[^1].

I compile R on my machines myself on Linux, installing a 32 bit and a 64 bit version, both as sub architectures[^2]. So when I installed littler and attached the package, I was greeted with an empty string as the location of the 'r' binary.

The fix seems to be simple, one just has to include .Platform$r_arch in the call to system.file() in R/init.R.

I have submitted a pull request for this change.

Cheers,
Berwin

[^1] It seems that Windows and MacOSX have only 64 bit versions, no multi architectures. I wonder whether they will soon disappear on Linux too?
[^2] I know, a very unique set-up. :-)

@eddelbuettel
Copy link
Owner

Hi Berwin I saw #117 first and replied there. I am a little puzzled as to what is going on -- are you in fact on windows? An existence proof for it building would be very welcome and I would love to extend it for the official build if that now works automagicallly. But I do not think I want to support the 32 and 64 bit switch that R used to support but no longer does. (I am happy to accomodate you on your self-selected two-build-sizes-on-Windows journey but I think the priority should first be all windows users and not just "you", no?)

@BerwinTurlach
Copy link
Contributor Author

Hi Dirk, thank you for looking at this.
I am very sure that I am working on Linux, specifically Xubuntu (Ubuntu 22.04.4 LTS).
I compile my R versions myself, first specifying "r_arch=32" to configure and configuring the compilers to compile 32 bit binaries, then specifying "r_arch=64" to configure and compilers configured for 64 bit binaries.
So my main/default architecture is 64 bit, but it is installed as a sub-architecture, so for all R packages I have /32 and /64 subdirectores (in /libs etc).
In case of littler, the binary is installed to littler/bin/64/r, thus it is not found by the command 'system.file("bin", "r", package="littler")' but needs 'system.file("bin", .Platform$r_arch, "r", package="littler")'.
As far as I can tell, this should not be a problem of other machines with a "traditional" set up as on those machines .Platform$r_arch evaluates to "".
But I am aware that the support for multiple architectures seems to be running out for R and since more and more packages cannot be compiled to 32 bit anymore (also because Ubuntu stopped providing a lot of 32 bit libraries), I am actually wondering whether I should abandon my current system and go for only a 64 bit compile.
Cheers,
Berwin

@eddelbuettel
Copy link
Owner

I see. So you are using the opportunity Linux offers you to create both 32 bit and 64 bit binaries. That is admirable. I also appreciate that in the 3rd message on this issue you are clarifying where and how you buid this. This may have been helpful at the start, so if could please consider somewhat augmented messaging next time.

The build is also fairly non-standard. Linux allows a large number of different cross-compilations and we do attempt to accomodate all of those. But as the suggested modication is non-intrusive and 100% cosmetic so we can probably work with it.

I just checked and injecting the (in a large majority of use cases empty and hence pointless) .Platform$r_arch does not seem to do harm:

> .Platform$r_arch
[1] ""
> message("You could link to the 'r' binary installed in '", system.file("bin", .Platform$r_arch, "r", package="littler"), "' from '/usr/local/bin' in order to use 'r' for scripting.", "See the 'vignette(\"littler-faq\")' for more details.")
You could link to the 'r' binary installed in '/usr/local/lib/R/site-library/littler/bin//r' from '/usr/local/bin' in order to use 'r' for scripting.See the 'vignette("littler-faq")' for more details.
> 

It just looks silly with the added /. So maybe I'll branch the message into a prior path printing with/without the added entry.

@BerwinTurlach
Copy link
Contributor Author

Hi Dirk,
Your comment popped up while I was preparing this. So it might be pointless. :)

Not sure what an existence proof is, but here is what happens if I install littler from CRAN:

R version 4.3.3 (2024-02-29) -- "Angel Food Cake"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu/64 (64-bit)
[...]
> install.packages("littler")
--- Please select a CRAN mirror for use in this session ---
trying URL 'https://cloud.r-project.org/src/contrib/littler_0.3.19.tar.gz'
Content type 'application/x-gzip' length 122659 bytes (119 KB)
==================================================
downloaded 119 KB
* installing *source* package ‘littler’ ...
** package ‘littler’ successfully unpacked and MD5 sums checked
** using staged installation
checking for gcc... gcc
checking whether the C compiler works... yes
[,...]
** installing vignettes
** testing if installed package can be loaded from temporary location
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (littler)

The downloaded source packages are in
	‘/tmp/RtmpfWUULe/downloaded_packages’

Then, when attaching the library I see:

> library(littler)
The littler package provides 'r' as a binary.
See 'vignette("littler-examples") for several usage illustrations,
and see 'vignette("littler-faq") for some basic questions.
You could link to the 'r' binary installed in
''
from '/usr/local/bin' in order to use 'r' for scripting.See the 'vignette("littler-faq")' for more details.

Note how the string that should give the location of the binary is empty.
If I install the version from my cloned repository with the change, the output is:

> library(littler)
The littler package provides 'r' as a binary.
See 'vignette("littler-examples") for several usage illustrations,
and see 'vignette("littler-faq") for some basic questions.
You could link to the 'r' binary installed in
'/home/opt/R/R-4.3.3/lib/R/library/littler/bin/64/r'
from '/usr/local/bin' in order to use 'r' for scripting.See the 'vignette("littler-faq")' for more details

Cheers,
Berwin

@eddelbuettel
Copy link
Owner

eddelbuettel commented Mar 10, 2024

Please see my additional commit in your 'smallfix' branch.

Your second comment it a reflection of the issue. You do set on R_arch on a platform where it is not commonly set. system.file() returns paths for existing files but by your choosing your setup differs. "Changes in behavior may be observed." No more no less.

Please try to added commit. It should help.

@BerwinTurlach
Copy link
Contributor Author

Hi Dirk,

Yes, your solution would work. Installing the littler version with your commit shows:

> library(littler)
The littler package provides 'r' as a binary.
See 'vignette("littler-examples") for several usage illustrations,
and see 'vignette("littler-faq") for some basic questions.
You could link to the 'r' binary installed in
'/home/opt/R/R-4.3.3/lib/R/library/littler/bin/64/r'
from '/usr/local/bin' in order to use 'r' for scripting.See the 'vignette("littler-faq")' for more details.

But I am wondering whether it would not be easier, and cleaner, to get rid of the second / by wrapping normalizePath() around the call to system.file(), i.e. just normalizePath(system.file("bin", .Platform$r_arch, "r", package="littler"))?

Cheers,
Berwin

@eddelbuettel
Copy link
Owner

Lovely suggestion on normalizePath, will do. I am currently (by testing at [mac-builder])https://mac.r-project.org/macbuilder)) another refinement just for that architecture were not displaying R_arch actually is a disservice to users. So this is all good.

@eddelbuettel
Copy link
Owner

Sent another commit. The macOS issue is a non-issue; $R_ARCH is empty there but sr/Makevars.in already covered it where it mattered (which is how you could install as you did !). So catching up with the usage message is good.

@BerwinTurlach
Copy link
Contributor Author

I was initially trying to modify the code similarly to your first suggestion, but then remembered normalizePath(). Probably a function that is not as prominent in people's mind as it should be. :)

Your last commit looks fine and works on my machine once file.path() is replaced by system.file(). I made this change in my latest commit.

The reason my machine installed r into bin/64 is probably due to the setting of src/Makevars.in. On my machine the lib/R/bin/R shell wrapper contains:

# Since this script can be called recursively, we allow R_ARCH to
# be overridden from the environment.
# This script is shared by parallel installs, so nothing in it should
# depend on the sub-architecture except the default here.
: ${R_ARCH=/64}

If I call R --arch=32, then R_ARCH is overwritten by that argument, but the :${R_ARCH=/64} is presumably present because I install my main architecture as a sub-architecture (unusual but allowed :-) ).

@eddelbuettel
Copy link
Owner

Your last commit looks fine and works on my machine once file.path() is replaced by system.file(). I made this change in my latest commit.

Thanls. That may have been absent-mindedness on my part.

On my machine the lib/R/bin/R shell wrapper contains:

Could you share more details? Are you running a standard R from a distribution such as Fedora or is this a configuration choice you made locally?

@BerwinTurlach
Copy link
Contributor Author

BerwinTurlach commented Mar 10, 2024

I am compiling from source. My bash script essentially does:

VERSION=4.3.3
cd /opt/src/R-$VERSION
cp ../R-config.site-32 config.site
./configure --prefix=/opt/R/R-$VERSION --enable-R-shlib r_arch=32 | tee CONFIGURE_32
make -j6
make check
make install 
make distclean
cp ../R-config.site-64 config.site
./configure --prefix=/opt/R/R-$VERSION --with-blas="-lopenblas" --with-lapack --enable-R-shlib r_arch=64 | tee CONFIGURE_64
make -j6
make check
make install 
make pdf info
make install-pdf install-info

The files R-config.site-32 and R-config.site-64 contain the necessary configuration options for respectively 32 bit and 64 build.
The fact that r_arch=32 and r_arch=64 is specified to configure results in both architectures being installed as sub-architectures.
There is more to the script, namely installing all those packages on which packages that I maintain depend, ensuring that, as far as possible, 32 bit and 64 bit of all those packages are installed. This point is getting more and more painful, so I really think about stop using two architectures.
And when I really do, I might stop compiling my R installation from source and just use your Debian packages. :-)

@eddelbuettel
Copy link
Owner

Right. And as R supports it, littler should too.

And I thought I was stubborn hanging on to 32 bit on Linux for too long. I do recall that for my very very first Docker uses on my laptop at the time I had to patch Docker to create a 32 bit binary. I saw the light when that laptop died and it has been x86_64 only ever since for maybe eight years. It's not my role to tell you what to do but .... 32 bit is gone.

Anyway, I will fold this in. Have been working on another package the last few hours.

@eddelbuettel
Copy link
Owner

Taken care of now in the merged #117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants