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.

Sign up for GitHub

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

Jump to bottom

[SPARK-33105][INFRA] Change default R arch from i386 to x64 and parametrize BINPREF #29991

Closed
wants to merge 2 commits into from
Closed

[SPARK-33105][INFRA] Change default R arch from i386 to x64 and parametrize BINPREF #29991

wants to merge 2 commits into from

Conversation

zero323
Copy link
Member

@zero323 zero323 commented Oct 9, 2020 •
edited
Loading

What changes were proposed in this pull request?

Reported on dev:

http://apache-spark-developers-list.1001551.n3.nabble.com/Broken-rlang-installation-on-AppVeyor-td30294.html

Why are the changes needed?

It seems like update from rlang 0.4.7 to 0.4.8 exposed an issue, where build fails because of incompatible ddl

c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
skipping incompatible C:/R/bin/i386/R.dll when searching for -lR
[00:01:52]
c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
skipping incompatible C:/R/bin/i386/R.dll when searching for -lR
[00:01:52]
c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
cannot find -lR
[00:01:52] collect2.exe: error: ld returned 1 exit status

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34196/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34196/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34197/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34197/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Test build #129593 has finished for PR 29991 at commit cd8b621.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zero323 zero323 force-pushed the APPVEYOR-DEAFAULT-ARCH branch 2 times, most recently from b6475be to cf4af04 Compare October 9, 2020 21:12
@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34199/

@zero323 zero323 changed the title [WIP][INFRA] Change default R arch from i386 to x64 [WIP][INFRA] Change default R arch from i386 to x64 and parametrize BINPREF Oct 9, 2020
@zero323
Copy link
Member Author

zero323 commented Oct 9, 2020 •
edited
Loading

Notes:

I am not sure if we should put MinGW here. Ultimately we want to use one provided by Rtools, right?

spark/dev/appveyor-install-dependencies.ps1

Line 45 in 2e07ed3

$env:PATH = $RDrive + '\R\bin\' + $arch + ';' + 'C:\MinGW\msys\1.0\bin;' + $env:PATH

(thought we might have to add $RtoolsDrive + '\Rtools40\usr\bin to PATH, unless make is provided otherwise)

Here

spark/dev/appveyor-install-dependencies.ps1

Line 70 in 2e07ed3

$env:PATH = $RtoolsDrive + '\Rtools40\bin;' + $RtoolsDrive + '\Rtools40\mingw64\bin;' + $RtoolsDrive + '\Rtools40\' + $gccPath + '\bin;' + $env:PATH

$RtoolsDrive + '\Rtools40\' + $gccPath + '\bin;' doesn't seem to point to existing directory and it seem like it shouldn't be added in the first place. Shall we remove it?

@zero323
Copy link
Member Author

zero323 commented Oct 9, 2020

cc @HyukjinKwon

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34201/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34199/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34201/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Test build #129594 has finished for PR 29991 at commit 0d8f110.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

Should be good to go as long as AppVeyor build pass.

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34204/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34205/

@zero323 zero323 changed the title [WIP][INFRA] Change default R arch from i386 to x64 and parametrize BINPREF [SPARK-33105][INFRA] Change default R arch from i386 to x64 and parametrize BINPREF Oct 9, 2020
@zero323 zero323 marked this pull request as ready for review October 9, 2020 22:31
@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34204/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34205/

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Test build #129596 has finished for PR 29991 at commit 042ab55.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 9, 2020

Test build #129598 has finished for PR 29991 at commit 0d8f110.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 10, 2020

Test build #129600 has finished for PR 29991 at commit b6475be.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 10, 2020

Test build #129602 has finished for PR 29991 at commit cf4af04.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

srowen
srowen reviewed Oct 10, 2020
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know this well but seems plausible.

@HyukjinKwon
Copy link
Member

Merged to master.

@zero323
Copy link
Member Author

zero323 commented Oct 10, 2020

Thanks!

@zero323 zero323 deleted the APPVEYOR-DEAFAULT-ARCH branch October 10, 2020 07:03
zhengruifeng pushed a commit to zhengruifeng/spark that referenced this pull request Oct 12, 2020
@zero323 @zhengruifeng
[SPARK-33105][INFRA] Change default R arch from i386 to x64 and param…
f92c883 
…etrize BINPREF

### What changes were proposed in this pull request?

- Change default R `arch` from `i386` to `x64`, to match Rtools  version.
- Parameterize `BINPREF` with `WIN` (https://stackoverflow.com/a/44035904)

Reported on dev:

http://apache-spark-developers-list.1001551.n3.nabble.com/Broken-rlang-installation-on-AppVeyor-td30294.html

### Why are the changes needed?

It seems like update from rlang 0.4.7 to 0.4.8 exposed an issue, where build fails because of incompatible ddl

```
c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
skipping incompatible C:/R/bin/i386/R.dll when searching for -lR
[00:01:52]
c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
skipping incompatible C:/R/bin/i386/R.dll when searching for -lR
[00:01:52]
c:/Rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
cannot find -lR
[00:01:52] collect2.exe: error: ld returned 1 exit status
```

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Existing tests.

Closes apache#29991 from zero323/APPVEYOR-DEAFAULT-ARCH.

Authored-by: zero323 <mszymkiewicz@gmail.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srowen srowen srowen left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@zero323 @SparkQA @HyukjinKwon @srowen