-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Linking issue on osx-arm64 #28
Comments
@conda-forge/r Any ideas about what might be the issue here? Help would be much appreciated. |
I can recreate (both new and old versions) on macOS 14.1.2. Checking linking with r-changeforest=1.1.2 osx-64 $ otool -L lib/R/library/changeforest/libs/changeforest.dylib
lib/R/library/changeforest/libs/changeforest.dylib:
@rpath/R/library/changeforest/libs/changeforest.dylib (compatibility version 0.0.0, current version 0.0.0)
@rpath/R/lib/libR.dylib (compatibility version 4.3.0, current version 4.3.2)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1197.1.1) r-changeforest=1.1.2 osx-arm64 $ otool -L lib/R/library/changeforest/libs/changeforest.dylib
lib/R/library/changeforest/libs/changeforest.dylib:
@rpath/R/library/changeforest/libs/changeforest.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.0.0) r-changeforest=0.6.0 osx-arm64 $ otool -L lib/R/library/changeforest/libs/changeforest.dylib
lib/R/library/changeforest/libs/changeforest.dylib:
@rpath/R/library/changeforest/libs/changeforest.dylib (compatibility version 0.0.0, current version 0.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.0.0) I can't look more right now, but this issue seems novel. |
Latest logs (v1.1.2) had: WARNING: The install/build script(s) for r-changeforest deleted the following files (from dependencies) from the prefix:
['lib/R/lib/libRblas.dylib', 'lib/R/lib/libRlapack.dylib'] Never seen that before and only happens in the osx-arm64 builds. The location is notably also where the Still at a loss for ideas. If something overwrote the |
Thanks @mfansler for looking into this! In 1.1.1 to 1.1.2 I upgraded |
Hi! Thanks for pinging. I believe upgrading extendr-api (which is still under development and introduces breaking changes) might be the reason. Internally, it depends on libR-sys, which generates bindings and links to R. I have no apple silicon at hand to test this, and we do not have arm build on CI yet, so I will try to check what has been changed in the past couple of versions manually. |
@mlondschien |
The PR I introduced into your original repo should resolve this issue mlondschien/changeforest#162 |
Thanks all!
No, both fail. I updated as I hoped this might fix the issue. Thanks @JosiahParry. This is super helpful! I merged and prepared a new release. Let's see if this did the trick.
It does (did) so as well on my machine. The error occurs only when installing the package with conda.
This is the input after implementing @JosiahParry's changes. What am I looking for here. Do you maybe have a reference to read into @mfansler?
@JosiahParry, if I understand correctly you released your rust/R package |
I don't know how conda forge works at all but it looks like the "recipe" might be used and that it injects the version of libR-sys to use https://github.com/conda-forge/r-changeforest-feedstock/blob/main/recipe/build.sh. I don't think that is necessary. R CMD install should be sufficient. Yes, publishing to packages to CRAN is not too difficult anymore thanks to the work of @yutannihilation among others! There is a vignette that describes the process but its not actually rendered at the moment. I'm happy to help walk through the way that I've been able to accomplish this. If you're in the extendr discord that might be a good place to get informal feedback. https://github.com/extendr/rextendr/blob/main/vignettes/articles/cran-compliance.Rmd |
Unfortunately, no reference AFAIK - just my observation that compiled R packages on Conda Forge typically link against Might also be worth noting here that we use macOS SDK 10.9 target (still!) for osx-64 and SDK 11 for osx-arm64. There are linker warnings in the osx-64 builds that the build targets 10.12, but links against 10.9. That could indicate the build is not respecting the flags we set. For now though, that part does seem to at least link correctly. |
Is it possible that in conda-arm environment we fail here? We fail to resolve library paths and omit this from linking step? https://github.com/extendr/libR-sys/blob/6fc2b5f2371fe20ec53d2cdf845a6e13dfdd3cfe/build.rs#L649-L651 |
This is necessary to allow cross-compiling. xref extendr/libR-sys#85
Thanks! I'll have a look.
How would I do this? |
@mlondschien , were there any specific steps you undertook to install R & this package from conda-froge? I am considering renting an M1 server to debug this issue & I need step-by-step guide to reproduce it. |
I used |
It looks like its passing? https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=853942&view=results |
@JosiahParry it's a false positive. Conda Forge doesn't run tests on |
In my understanding, as the code comment says, |
To my eyes, this looks the core of the problem. Maybe you simply need to specify
|
This comment was marked as off-topic.
This comment was marked as off-topic.
After investigating a lot of things and moving in a wrong direction, I am now convinced this might be the case -- the library produced for arm64 by your pipeline is much smaller than that build locally on arm64 machine. I suspect it does not link at all and hence cannot find a symbol (which is defined in rust library). @yutannihilation is likely correct void R_init_changeforest_extendr(void *dll);
void R_init_changeforest(void *dll) {
R_init_changeforest_extendr(dll);
} |
A fellow project that uses cross-compilation on mac os (successfully, I think), seems to set |
Thanks @Ilia-Kosenkov for looking into this! I'm confused. Wouldn't this line
set Another note: The R error mentions |
Yes but this happens only if Yep I noticed that it has an underscore prefix, but this name is tied to a function we generate for package |
Here they set target for cross-compilation, which overrides the default value https://github.com/pola-rs/r-polars/blob/89fe7196422a5477307db1015dbd139ad323158c/.github/workflows/release-lib.yaml#L44C23-L44C23 |
Thanks. I had missed this. mlondschien/changeforest#165 should do the trick if we set |
Neither |
Is this a case of these variables needing to be |
I added
Can I add the target |
@mlondschien From which PR is the referenced output? |
Also, you should not be using |
|
Pushed a fix to the feedstock 695513b and made an upstream PR: mlondschien/changeforest#168 I have verified locally beforehand that it works. |
Thanks! Will |
It's set by the compiler package that I have added in the change. |
Thanks @xhochy and everyone who looked into this! This did the trick. As suspected by @yutannihilation and @Ilia-Kosenkov, this was missing a |
Hey @mlondschien , thanks for the update! I was not following the issue the past day or two, if it resolved now, or is it still broken for aarch64? |
Yes. I just checked installing directly from conda-forge. |
Solution to issue cannot be found in the documentation.
Issue
I received a bug report that installing
changeforest
with conda results in linkage issues. I was able to reproduce the error on an osx-arm64 (M1 Macbook) machine.Suprisingly, this used to work when I last tested (~1 year ago). Downgrading did not solve the issue. Could this be an OS compatability issue? I upgraded to OSX 13 (Ventura) since last testing this. Building the package locally (
R CMD INSTALL --build changeforest-r
) works, as long as I add-F/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/
to thePGK_LIBS
flags in theMakevars
.@xhochy, is there anything that immediately comes to mind?
The text was updated successfully, but these errors were encountered: