R installation failing on Cygwin

[embray]

When running a make of Sage on Cygwin the build fails upon installation of R. I will attach the full log, but the relevant error is:

make[4]: Entering directory '/home/embray/src/sagemath/sage/local/var/tmp/sage/build/r-3.2.4-revised.p0/R-revised/src/library/Recommended'
begin installing recommended package MASS
INSTALL: unknown option -- pkglock
Try 'INSTALL --help' for more information.
Makefile:51: recipe for target 'MASS.ts' failed
make[4]: *** [MASS.ts] Error 1
make[4]: Leaving directory '/home/embray/src/sagemath/sage/local/var/tmp/sage/build/r-3.2.4-revised.p0/R-revised/src/library/Recommended'
Makefile:39: recipe for target 'recommended-packages' failed

--pkglock is an argument to the INSTALL script for installing R packages. But it seems it's been passed in as literally -- pkglock (with a space). Not sure why.

This issue also raises the question again about whether to make R an optional package (as well as whether to allow Sage to use a system install of R).

Upstream: None of the above - read trac for reasoning.

Component: porting: Cygwin

Keywords: windows cygwin R

Author: Erik Bray

Branch/Commit: b2ebe0b

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/20655

[embray]

Attachment: r-3.2.4-revised.p0.log

[embray]

comment:1

Ahahahahah I at least slightly understand better. I was losing my mind because I could not for the life of me find where this error message could be coming from.

Turns out it's coming from the GNU install command, which gets confused for R's INSTALL command on Windows because case-insensitivity.

[embray]

comment:2

Okay, I think this is a bug in R IMO.

R installs a bunch of "scripts" particular to R with names like BUILD and INSTALL. R's install mistakenly marks these "scripts" as executable even though they do not contain a shebang line, so they are technically not executable in that sense. Instead it runs those scripts from a main script (called Rcmd) using exec.

The problem here is that Rcmd only run these R-specific scripts if they are executable, and exist in $R_HOME/bin. Realistically just whether they exist and are files in $R_HOME/bin should be good enough, since there shouldn't be any non-executable files in there. If nothing else it should try to exec and if that fails it will return a permission error. On Cygwin exec doesn't care if the file is marked executable or not, for this reason. It will just try to execute it.

[embray]

Upstream: Not yet reported upstream; Will do shortly.

[embray]

comment:4

I guess on *nix it still has to mark those scripts as executable in order for them to work with exec, even though they are not strictly executable on their own. Without a shebang line the shell will still try to execute them in the current shell process so I guess I rescind my complaint about them being marked executable.

Nevertheless the test -x used before executing them won't work sometimes on Cygwin :/

[embray]

comment:5

This adds a patch that is the bare minimum to work around the issue and is essentially harmless.

---

New commits:

04d57eb

Add patch for R needed for the 'R CMD' command to work on Cygwin.

[embray]

Author: Erik Bray

[embray]

Commit: 04d57eb

[embray]

Branch: u/embray/cygwin-r-issue

[embray]

Changed upstream from Not yet reported upstream; Will do shortly. to None of the above - read trac for reasoning.

[embray]

comment:6

Changed my mind about reporting upstream. According to this issue, and a few others it seems they don't want to officially support Cygwin builds and will close Cygwin issues as wontfix. This is also environment specific. It only affects you if you're building sage on a Cygwin filesystem that is mounted with noacl. So maybe this doesn't affect most users.

I still think the patch is worth keeping for the sake of least resistance on trying to build Sage on Cygwin. This was a very mysterious and hard to track down problem.

[vbraun]

comment:7

needs some documentation (in the patch header)

[embray]

comment:9

Done!

[tscrim]

comment:10

I was able to build R on cygwin32 using 7.3 without this. Still waiting on my cygwin64.

[tscrim]

comment:11

Also built okay without this patch on cygwin64. Should we close this as wont-fix?

[embray]

comment:12

Replying to @tscrim:

Also built okay without this patch on cygwin64. Should we close this as wont-fix?

This issue only impacts if you're building in a directory mounted with noacl or with a messed up ACL which happens not infrequently if you have non-Cygwin processes messing with the permissions on files.

The end result is that setting executable flags may not always work.

[tscrim]

comment:13

So it's something more with the configuration then? I will try rebuilding R with this branch later tonight.

[jdemeyer]

comment:14

It worries me when executable permissions don't work correctly. I think that false positives (test -x returning true on a non-executable file) are not so bad, but false negatives (test -x returning false on an executable file) seem more problematic. I would not be surprised if there would be more programs (even just the shell) behaving badly for this reason.

[embray]

comment:15

It's not really a problem in the shell so much. Cygwin relies on a lot more than the x flag to determine whether or not a file is executable, precisely because of the unreliability of emulating UNIX permissions on top of Windows ACL. At most, Cygwin just relies on the x permission as a hint.

[tscrim]

comment:16

So then should we consider merging this ticket Jeroen? I'm not sure what changing -x to -f really does with regards to how things are built.

[jdemeyer]

comment:17

I having nothing against adding this patch to R. I am only saying that it's an ugly workaround and not really a fix.

Just one thing: the patch is missing documentation. At the very least, it should refer to this Trac ticket.

[jdemeyer]

Work Issues: patch description

[embray]

comment:19

Replying to @jdemeyer:

I having nothing against adding this patch to R. I am only saying that it's an ugly workaround and not really a fix.

No disagreement there. And really what it's working around is a shortcoming in Cygwin. If you prefer, it could be a Cygwin-only patch. It's just there's no real reason that the patched line strictly needs to test -x (and the code being patched is an ugly mess to begin with).

[sagetrac-git]

Changed commit from 04d57eb to b2ebe0b

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b2ebe0b

Added patch description

[embray]

New commits:

b2ebe0b

Added patch description

[jdemeyer]

comment:22

If this works for you...

[jdemeyer]

Changed work issues from patch description to none

[jdemeyer]

Reviewer: Jeroen Demeyer

[jdemeyer]

comment:23

because they do no contain a shebang line

I guess this is the real bug.

[embray]

comment:24

Replying to @jdemeyer:

because they do no contain a shebang line

I guess this is the real bug.

Yes and no. I think the real bug is that it requires them to be executable, because the way those particular scripts are invoked, IIRC, is by sourcing them from an actual executable. In other words, they shouldn't be considered executables in their own right in the first place.

Otherwise yes, I agree they should have a shebang line.

[vbraun]

Changed branch from u/embray/cygwin-r-issue to b2ebe0b