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

Rebuild for r-base 4.4 and UCRT mingw toolchain #87

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This PR has been triggered in an effort to update r-base44_and_m2w64-ucrt.

Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.


If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/10604935422 - please use this URL for debugging.

@conda-forge-webservices
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/meta.yaml) and found it was in an excellent condition.

@eddelbuettel
Copy link
Contributor

I am confused. How did this get triggered now? R 4.4.0 came out ~ four months ago, and R 4.4.1 followed ~ 2 months ago.

@jdblischak
Copy link
Member

I am confused. How did this get triggered now? R 4.4.0 came out ~ four months ago, and R 4.4.1 followed ~ 2 months ago.

conda-forge lags behind the official R releases. First the new R release has to be packaged with the conda-forge toolchain. Then it has to migrate throughout the dependency network. The R 4.4 migration is almost finished: https://conda-forge.org/status/migration/?name=r-base44_and_m2w64-ucrt

@jdblischak
Copy link
Member

The osx-64 failure is with the new r-base 4.4:

** testing if installed package can be loaded from temporary location
ERROR: loading failed
* removing ‘/Users/runner/miniforge3/conda-bld/r-tiledb_1724888611418/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/lib/R/library/tiledb’
Error: package or namespace load failed for ‘tiledb’:
 .onLoad failed in loadNamespace() for 'tiledb', details:
  call: dyn.load(lib_path)
  error: unable to load shared object '/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/':
  dlopen(/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/, 0x0006): tried: '/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/' (not a file)
Error: loading failed

@mfansler
Copy link
Member

mfansler commented Sep 3, 2024

This doesn't immediately explain why v4.4 and not v4.3 fails, but probably shouldn't be passing both -mmacosx-version-min=11 (from the configure script) and -mmacosx-version-min=10.13 (from CF) flags. That part is at least consistent (both 11) in the osx-arm64 builds.

@jdblischak
Copy link
Member

probably shouldn't be passing both -mmacosx-version-min=11 (from the configure script) and -mmacosx-version-min=10.13 (from CF) flags

@mfansler do you know how we can override the -mmacosx-version-min=10.13 flag from conda-forge? I would like to try -mmacosx-version-min=11 to match the configure script to see if that makes any difference.

I ask because we have previously tried to set -mmacosx-version-min=10.14, but that didn't remove the instances of -mmacosx-version-min=10.13.

We already set c_stdlib_version to 10.14 in the CBC file, as instructed in the docs:

c_stdlib_version: # [osx and x86_64]
- 10.14 # [osx and x86_64]

which results in MACOSX_DEPLOYMENT_TARGET and MACOSX_SDK_VERSION being set to 10.14:

MACOSX_DEPLOYMENT_TARGET:
- '10.14'
MACOSX_SDK_VERSION:
- '10.14'
c_stdlib:
- macosx_deployment_target
c_stdlib_version:
- '10.14'

Looking at our last successful osx-64 build for R 4.3 in main, this resulted in the macOS 10.14 SDK being downloaded and -mmacosx-version-min=10.14 being added to the env var CPPFLAGS. However, the compiler lines still include -mmacosx-version-min=10.13 -mmacosx-version-min=10.13. I assume the 10.13 is from the conda-forge global pinning. I'm confused why it is repeated twice though.

Note that we also have some non-standard wrapper scripts for the compiler on osx-64 in this repository:

args="${@##-mmacosx-version-min=10.9*}"
$NN_CXX_ORIG $args -mmacosx-version-min=10.14

I did some experiments in a branch on my fork. I tried updating the wrapper scripts to replace 10.13 since that is now the default. This had no effect.

args="${@##-mmacosx-version-min=10.13*}"
$NN_CC_ORIG $args -mmacosx-version-min=10.14

I also tried not using the wrapper scripts at all. This similarly had zero effect. The R 4.3 job passed, and the R 4.4 failed with the same error. Furthermore, both continued to inject the double -mmacosx-version-min=10.13. I looked at a few other feedstocks with compiled R code, and the double -mmacosx-version-min=10.13 is a common feature.

@danielnachun
Copy link

This is an oddly specific error to macOS only on R 4.4:

 .onLoad failed in loadNamespace() for 'tiledb', details:
  call: dyn.load(lib_path)
  error: unable to load shared object '/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/':
  dlopen(/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/, 0x0006): tried: '/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/' (not a file)

For some reason it's trying to load a shared library from a directory instead of a proper dylib. I'm not sure if that's because the dylib isn't being build or the wrong path is being passed to the loader.

@eddelbuettel
Copy link
Contributor

@danielnachun That is a very standard part of each and every R package build: to test whether the build is truly relocatable it runs it from the temporary directory, and if successful, moves it and tests it again.

@danielnachun
Copy link

That test makes sense but the issue here is that this error (I should have formatted the code block better):

tried: '/private/tmp/Rtmpb9lrhR/R.INSTALL245a26dc05a4/tiledb/' (not a file)

seems to imply that it's trying to use dlopen on a directory instead of libtiledb.dylib inside that directory. I don't know if that's because libtiledb.dylib simple wasn't built and it's falling back to an incorrect path or if there is some other cause of this that's less obvious.

@mfansler
Copy link
Member

mfansler commented Sep 4, 2024

@jdblischak looks like it was captured in the lib/R/etc/Makeconf files of the osx-64 r-base builds. For example, in r-base-4.4.1-h4074611_15 (breaks \ added for readability):

CPPFLAGS =  -D_FORTIFY_SOURCE=2 \
  -isystem /Users/runner/miniforge3/conda-bld/r-base-split_1723488738748/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeh/include \
  -mmacosx-version-min=10.13 \
  -mmacosx-version-min=10.13 \
  -I/Users/runner/miniforge3/conda-bld/r-base-split_1723488738748/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeh/include

I'm unsure if that should be the case, so I'll post an Issue. However, I won't have bandwidth today to look into it further. I think the question of whether this actually is the problem here hinges on whether g++ will consider all the flags and take the max minimum (11 + 10.13 => 11) or if it only considers the last one (11 + 10.13 => 10.13). I wouldn't rule out this is a red herring for this particular build, but feedstocks should be able to use CBC settings to raise the minimum, and that functionality seems broken from what you've shown.

@jdblischak
Copy link
Member

I have an idea of what could be causing the problem with R 4.4.

So I think we need the extension .dylib for the R 4.3 build and .so for the R 4.4 build

@danielnachun
Copy link

@jdblischak That makes a lot of sense! I guess we just need to know how to set the file extension and then this will hopefully work?

@jdblischak
Copy link
Member

I guess we just need to know how to set the file extension and then this will hopefully work?

I am going to work on a patch for configure

Copy link
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

🚀

@jdblischak jdblischak merged commit c699223 into conda-forge:main Sep 10, 2024
12 checks passed
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the rebuild-r-base44_and_m2w64-ucrt-0-1_hfe771b branch September 10, 2024 17:20
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.

5 participants