Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#22627 closed defect (fixed)

R build broken on Cygwin

Reported by: embray Owned by:
Priority: major Milestone: sage-8.0
Component: porting: Cygwin Keywords: days85
Cc: Merged in:
Authors: Erik Bray Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 1cf2883 (Commits, GitHub, GitLab) Commit:
Dependencies: #20523 Stopgaps:

Status badges

Description (last modified by embray)

For a bunch of R's shared library modules it does not explicitly include -l flags for libraries they link to, as it is not necessary to do so, for the most part, on Linux.

However, on Windows it is necessary to resolve all symbols at link time. This patch fixes the issue, and is otherwise harmless.

This issue was already reported upstream some years ago, and rejected as invalid since the R developers do not support Cygwin.

Attachments (1)

r-3.3.3.p1.log (117.2 KB) - added by gouezel 4 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 4 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/cygwin/r-build
  • Commit set to 6c24ea3079afe47f8ff8fe9324027d2feb08b0b7
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

6c24ea3Adds a patch to fix building R on Cygwin

comment:2 Changed 4 years ago by tscrim

This will conflict with #20523. Yet instead of basing this on #20523, which is potentially still controversial, I would create a new separate branch that is based on #20523.

comment:3 Changed 4 years ago by embray

I should test #20523 on Cygwin anyways....once I get the rest fixed.

comment:4 Changed 4 years ago by jdemeyer

  • Dependencies set to #20523
  • Keywords days85 added
  • Status changed from needs_review to needs_work

Setting to needs_work because it should be rebased on #20523.

comment:5 Changed 4 years ago by git

  • Commit changed from 6c24ea3079afe47f8ff8fe9324027d2feb08b0b7 to 1cf2883712f4838a30aa7779fe81f269800c2f52

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

00fcc86Depend on PCRE
23f9471Update spkg-install to conform to #20692.
22974b8R-shlib is needed in configurefor the (standard) rpy2 package.
d873670Added curl to dependencies.
3b11753Merge branch 'public/r311' of git://trac.sagemath.org/sage into newrrr
8e07611Reinsert Cygwin-specific patches (lost in the mist of battle).
516f029Upgraded to R 3.3.3
cbe9134Merge branch 'public/r311' of git://trac.sagemath.org/sage into finalr
f029f66This simple patch removes an overkill check in R's configure.
1cf2883Adds a patch to fix building R on Cygwin

comment:6 Changed 4 years ago by embray

  • Status changed from needs_work to needs_review

Rebased on #20523. This simple patch this adds could probably be directly tacked on to #20523, but since that issue has been under contention for a long time and is finally close to ready I'd rather not touch it.

Last edited 4 years ago by embray (previous) (diff)

comment:7 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

Changed 4 years ago by gouezel

comment:8 Changed 4 years ago by gouezel

Doesn't work for me (i.e., I think the patch fixes some build issue of R, but not all of them for me), on cygwin64 on x86_64, windows 10 pro. Short version: a file libR.sois correctly produced, but next I get

gcc -fopenmp  -L../../lib -L/home/Sebastien/sage33/sage/local/lib -Wl,-rpath,/home/Sebastien/sage33/sage/local/lib  -o R.bin Rmain.o  -lR 
/usr/lib/gcc/x86_64-pc-cygwin/5.4.0/../../../../x86_64-pc-cygwin/bin/ld: cannot find -lR
collect2: error: ld returned 1 exit status

Is it looking for libR.dll instead?

Log file attached.

comment:9 Changed 4 years ago by embray

I'm not building R 3.3.3. I'm still on 3.2.4. I've been meaning to test the 3.3.3 build on Cygwin for comment on #20523, but have kept getting side-tracked by other things. It's entirely likely there are other, unrelated build issues of 3.3.x on Cygwin.

(on cygwin -lR should find the libR.dll.a import library).

comment:10 Changed 4 years ago by embray

  • Status changed from positive_review to needs_work

I confirmed, finally, that with the recent upgrade to R 3.3.3 building is broken on Cygwin again; this patch alone doesn't fix it. I got the same error as 8. I will investigate and update the patch as needed.

comment:11 Changed 4 years ago by vbraun

  • Branch changed from u/embray/cygwin/r-build to 1cf2883712f4838a30aa7779fe81f269800c2f52
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:12 Changed 4 years ago by vbraun

  • Commit 1cf2883712f4838a30aa7779fe81f269800c2f52 deleted

Do it on a new ticket..

comment:13 Changed 4 years ago by embray

  • Branch changed from 1cf2883712f4838a30aa7779fe81f269800c2f52 to u/embray/cygwin/r-build
  • Commit set to 1cf2883712f4838a30aa7779fe81f269800c2f52

Turns out we already had the patch I attached in the this ticket here, along with some other Cygwin-specific patches: https://git.sagemath.org/sage.git/tree/build/pkgs/r/patches/cygwin?id=718f365788938d539483a3ec714c7b42172cd237

These patches just aren't being applied at all (the spkg-install moves them but never applies them, since patches are now applied before spkg-install is run).

comment:14 Changed 4 years ago by embray

Why? R build is still broken on Cygwin. It is not "fixed".

comment:15 Changed 4 years ago by vbraun

But the ticket is merged and didn't cause the buildbot to break. If you don't think its fixed you shouldn't have set it to needs_review.

comment:16 Changed 4 years ago by embray

If you don't think its fixed you shouldn't have set it to needs_review.

What are you talking about? Why shouldn't I have set needs_review? I realized weeks after it was set to positive_review that this needed more work and set it to needs_work. Why was it merged even after I set it to needs_work?

comment:17 Changed 4 years ago by vbraun

It was merged and tested before you switched it away from positive review. It does take quite a bit of time to run though all buildbots.

comment:18 Changed 4 years ago by embray

I don't understand. Does it get merged automatically or manually? What this ticket shows is that it was set to "needs_work" by me first, and then closed by you and I have no idea if you're doing this manually or if there are some scripts doing it.

comment:19 Changed 4 years ago by vbraun

There are scripts testing and merging positively reviewed tickets. Backing that out because somebody changes it back from positive review is a manual process.

comment:20 Changed 4 years ago by embray

But do you manually update the ticket, or does the script do that (i.e. 11)? What I don't understand is why it would merge the branch even after the ticket's status has been changed--unless that's happening before it changed, but there's no obvious way to see that that's the case.

comment:21 Changed 4 years ago by embray

  • Branch changed from u/embray/cygwin/r-build to 1cf2883712f4838a30aa7779fe81f269800c2f52
  • Commit 1cf2883712f4838a30aa7779fe81f269800c2f52 deleted

I think I changed this by mistake.

comment:22 Changed 4 years ago by vbraun

The branch is merged before testing and closed after testing on the buildbot. If you switch it back from positive review during testing then I get notified but nothing is done automatically.

Note: See TracTickets for help on using tickets.