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

[NO MRG] R noarch packages #1

Closed
wants to merge 7 commits into from
Closed

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Dec 2, 2017

For pure R packages, we can make the package noarch: generic. There'll be a warning however that the package was compiled for r-3.4.1 if it's imported in r-3.3.2.

I tested building this package in linux and importing in windows and it works. (conda install r-regsitry -c isuruf if you want to try)

cc @conda-forge/core, @bgruening, @johanneskoester, @jdblischak, @daler, @mingwandroid

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • noarch packages can't have selectors. If the selectors are necessary, please remove noarch: generic.

mkdir -p $PREFIX/R/library
mv $PREFIX/lib/R/library/$R_PACKAGE_NAME $PREFIX/R/library/$R_PACKAGE_NAME

echo "ln -s \${PREFIX}/R/library/$R_PACKAGE_NAME \${PREFIX}/lib/R/library/$R_PACKAGE_NAME" > $PREFIX/bin/.${PKG_NAME}-post-link.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

The above hack is because in windows the library is installed into R/library while in unix it's installed into lib/R/library

@bgruening
Copy link
Contributor

If this works, it would be awesome! Thanks @isuruf!

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

@conda-forge-admin, please lint

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@jdblischak
Copy link
Member

@isuruf This is really great! I was able to install and use the r-registry package from your channel with both R 3.3.2 and R 3.4.1 on Linux, macOS, and Windows. I ran the following in R to test:

library("registry")
r <- registry()
r

I received no warnings or errors.

@bgruening
Copy link
Contributor

It works for me as well. I must admit I'm not a fan to pollute the recipe and the directory that much with the postlink magic but the advantages outweighing my concerns!

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

There are several other ways though.

  1. Teach conda about a new noarch: r
  2. Set R_LIBS_USER in activate.sh
  3. Edit Rprofile file (4 is the same)
  4. Creating a file in R_HOME/etc/Renviron.site and setting R_LIBS_USER

1 requires changing conda and conda-build.
2 requires activating the environment.
3 and 4 require changing r-base.

@mingwandroid
Copy link

We should not accept post links scripts here. We should unify the windows layout with the others instead.

@mingwandroid
Copy link

Post linking everything nullifies the advantages of hardlinks. Doing this for the odd packages when necessary is ok but doing it wholesale is not.

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

@mingwandroid, what do you think about the 4 alternatives I mentioned above? none of them require post link scripts

@jakirkham
Copy link
Member

  1. Teach conda about a new noarch: r

xref: conda/conda#5974

@mingwandroid
Copy link

  1. Is functionally no different from a post link.
  2. You are overriding the users libs folder here.
    3 and 4 are just 2 again.

Why would you want to do anything other than making the layout the same on Windows as the others? Then discussion of hacks doesn't need to happen.

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

Why would you want to do anything other than making the layout the same on Windows as the others? Then discussion of hacks doesn't need to happen.

Backwards compatibility for R 3.3.2 and R 3.4.1

@mingwandroid
Copy link

mingwandroid commented Dec 3, 2017

There's no such backwards compatibility nor concerns about that.

Each r package pins to the exact r-base version and it needs to stay this way.

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

Each r package pins to the exact r-base version and it needs to stay this way.

That's not what this PR intends to do. noarch: python doesn't pin to the python version either.

@mingwandroid
Copy link

mingwandroid commented Dec 3, 2017

You are attempting compatibility across platforms here for packages that do not need compilation. There is no compatibility across R versions.

The bytecode changes between versions in incompatible ways.

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

What I mean is if we change the layout in R 3.5, the noarch generic packages won't work in 3.4 and 3.3

@mingwandroid
Copy link

The R packages that do not need compilation contain bytecode specific to that version of R. You cannot expect bytecode for R 3.5 to run on 3.4 or from 3.4 to run on 3.3. It does not work.

@isuruf
Copy link
Member Author

isuruf commented Dec 3, 2017

Does bytecode for R 3.3 run on 3.4 and 3.5? Or is that incompatible as well?

@mingwandroid
Copy link

The R people evolve the language and at the same time the bytecode format. They do not guarantee any compatibility between different minor versions. You'll probably get away with patch versions but I'm not sure. I do know that they themselves do not risk anything here and do full build-outs at each patch version for the systems they provide binaries for (Windows for ever and macOS since 3.4).

If you really want full noarch: r so that it does the bytecode compilation at install time then that might be workable but IMHO, having cross-platform noarch is a good enough change and you shouldn't try to over-reach for now.

When I last checked 1/2 the R packages needed compilation and 1/2 did not.

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

6 participants