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

In CI, use the Conda-provided R #459

Closed
wants to merge 3 commits into from
Closed

Conversation

DilumAluthge
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Base: 78.01% // Head: 72.72% // Decreases project coverage by -5.29% ⚠️

Coverage data is based on head (53539b2) compared to base (edcdf4a).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #459      +/-   ##
==========================================
- Coverage   78.01%   72.72%   -5.30%     
==========================================
  Files          23       23              
  Lines        1610     1569      -41     
==========================================
- Hits         1256     1141     -115     
- Misses        354      428      +74     
Impacted Files Coverage Δ
src/namespaces.jl 37.03% <0.00%> (-55.56%) ⬇️
src/Const.jl 51.16% <0.00%> (-48.84%) ⬇️
src/io.jl 53.57% <0.00%> (-18.20%) ⬇️
src/eventloop.jl 54.71% <0.00%> (-13.81%) ⬇️
src/setup.jl 71.42% <0.00%> (-4.71%) ⬇️
src/render.jl 87.14% <0.00%> (-1.43%) ⬇️
src/methods.jl 67.47% <0.00%> (-0.35%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ViralBShah
Copy link
Contributor

Failing later now with similar error.

@DilumAluthge
Copy link
Member Author

Can you reproduce the error on a local Windows machine?

@DilumAluthge
Copy link
Member Author

We'll likely need to recruit a Windows user to help.

They don't need to set their machine up in a particular way - because we can reproduce this error in CI when R_HOME is set to *, the user can just set the R_HOME environment variable to * when they run the tests locally. No need for them to install R on their local machine.

@ViralBShah
Copy link
Contributor

ViralBShah commented Oct 19, 2022

I might be able to try that out on an underpowered windows machine I do have access to.

Although, I thought that removing Conda and just using the CI provided R would be better in terms of fewer dependencies.

@ViralBShah
Copy link
Contributor

Do you know why it has issues adding General registry? https://github.com/JuliaInterop/RCall.jl/actions/runs/3277858196/jobs/5395606774#step:8:39

@DilumAluthge
Copy link
Member Author

Do you know why it has issues adding General registry? https://github.com/JuliaInterop/RCall.jl/actions/runs/3277858196/jobs/5395606774#step:8:39

That looks like a bug in the https://github.com/julia-actions/julia-buildpkg action - I'll look into it.

@ViralBShah
Copy link
Contributor

So, 64-bit windows is only failing on the conda specific test, which we may be able to disable (or update). The 32-bit windows is failing on finding the right location of the R shared library. If we can get win64 passing, then we can disable win32 for now, and at least get to green CI.

@ViralBShah ViralBShah closed this Oct 19, 2022
@ViralBShah
Copy link
Contributor

#462 is an alternate solution.

@DilumAluthge DilumAluthge deleted the DilumAluthge-patch-1 branch October 19, 2022 20:31
@ViralBShah
Copy link
Contributor

On windows 64-bit, I set R_HOME to * and Conda failed at some point because it couldn't find C:\Users\viral\.julia\conda\3\Library\bin\R\bin\x64\R.dll.

[ Info: Installing R via Conda.  To use a different R installation, set the "R_HOME" environment variable and re-run Pkg.build("RCall").
[ Info: Downloading miniconda installer ...
[ Info: Installing miniconda ...
[ Info: Running `conda config --add channels r --file 'C:\Users\viral\.julia\conda\3\condarc-julia.yml' --force` in root environment
[ Info: Running `conda install -y 'r-base>=3.4.0,<4'` in root environment
ERROR: LoadError: Could not find library C:\Users\viral\.julia\conda\3\Library\bin\R\bin\x64\R.dll. Make sure that R shared library exists.
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:35
 [2] validate_libR(libR::String)
   @ Main C:\Users\viral\.julia\packages\RCall\6kphM\deps\setup.jl:11
 [3] locate_libR(Rhome::String)

However, the correct location is /c/Users/viral/.julia/conda/3/R/bin/x64

@simonbyrne
Copy link
Member

Does that mean Conda.LIBDIR == "C:\Users\viral\.julia\conda\3\Library\bin"?

@simonbyrne
Copy link
Member

Looks like it:

https://github.com/JuliaPy/Conda.jl/blob/8f7133206f3efb6308dff5a2b09393d10e6cc122/src/Conda.jl#L57

I guess we should use

Rhome = joinpath(Conda.ROOTENV, "R")

@ViralBShah ViralBShah restored the DilumAluthge-patch-1 branch October 20, 2022 02:28
@ViralBShah
Copy link
Contributor

@simonbyrne Do we necessarily need to use R <= 4?

@ViralBShah ViralBShah reopened this Oct 20, 2022
@ViralBShah ViralBShah marked this pull request as ready for review October 20, 2022 02:40
@ViralBShah ViralBShah closed this Oct 20, 2022
@ViralBShah ViralBShah deleted the DilumAluthge-patch-1 branch October 20, 2022 02:50
@ViralBShah
Copy link
Contributor

ViralBShah commented Oct 20, 2022

Seems like this would need more work to figure out and we already have a working CI. Just closing it again.

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

Successfully merging this pull request may close these issues.

3 participants