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

Load correct dependencies in RProvider.fsx (fix #166) #167

Merged
merged 5 commits into from Apr 5, 2016

Conversation

tpetricek
Copy link
Member

Our build.fsx script was not replacing the versions properly - it only did that for the RProvider package (which is not needed anymore, because we reference files from there directly!) but it did not generate correct references for dependencies.

This does not affect FsLab or Paket users, but it would be nice to fix this for those using R provider directly via NuGet.

@tpetricek
Copy link
Member Author

After the changes, it still did not run on Mac - it seems that RDotNet fails to initialize REngine when we set the environment variables and then pass it null as a DLL path.

The second checkin changes how this is handled so that we find the DLL path ourselves (we did this already to check whether it exists) and then passes the DLL path to R.NET.

I released an alpha version on NuGet so that @evelinag can test it on Mac, but it certainly needs more testing.

@evelinag
Copy link
Contributor

evelinag commented Feb 8, 2016

After the changes, the alpha version works on a Mac (I tested it on two machines running El Capitan).

The only problem is that it works only when the editor is launched from the command line because otherwise RProvider doesn't have access to $PATH and because of this it can't find R. This seems to be a "feature" introduced in El Capitan (for example see similar issues in https://github.com/ionide/ionide-fsharp/issues/155 or steelbrain/linter#726).
Would it be possible to add R_HOME into ~/.rprovider.conf along with MONO64?

@tpetricek
Copy link
Member Author

@hmansell @dcharbon @adamklein This seems to be causing issues to other people too. Can you please review the changes?

@adamklein
Copy link

Seems fine to me but maybe @hmansell wants to have a quick look?

@hmansell
Copy link
Contributor

Looks OK to me (but I haven't tested it)

@tpetricek
Copy link
Member Author

Merging this - I also updated R.NET to address #169.

@tpetricek tpetricek merged commit 585a47c into fslaborg:master Apr 5, 2016
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.

None yet

4 participants