Skip to content

r-base 4.5.0#369

Closed
IsabelParedes wants to merge 28 commits into
conda-forge:mainfrom
IsabelParedes:4.5.0_h6106e3
Closed

r-base 4.5.0#369
IsabelParedes wants to merge 28 commits into
conda-forge:mainfrom
IsabelParedes:4.5.0_h6106e3

Conversation

@IsabelParedes

@IsabelParedes IsabelParedes commented Apr 22, 2025

Copy link
Copy Markdown
Member

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Fixes #368

@conda-forge-admin

conda-forge-admin commented Apr 22, 2025

Copy link
Copy Markdown
Contributor

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.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/15440730222. Examine the logs at this URL for more detail.

@IsabelParedes

Copy link
Copy Markdown
Member Author

@conda-forge-admin, please rerender

@IsabelParedes

Copy link
Copy Markdown
Member Author

Requires conda-forge/autoconf-feedstock#40

@IsabelParedes IsabelParedes marked this pull request as ready for review April 22, 2025 17:35
@xhochy

xhochy commented Apr 25, 2025

Copy link
Copy Markdown
Member
autoconf >=2.72 * does not exist

See conda-forge/autoconf-feedstock#40

@IsabelParedes

Copy link
Copy Markdown
Member Author

@conda-forge-admin, please rerender

@IsabelParedes

Copy link
Copy Markdown
Member Author

autoconf $v2.72$ now available 🎉

@IsabelParedes

IsabelParedes commented May 13, 2025

Copy link
Copy Markdown
Member Author

m2-autoconf >=2.72 * does not exist for win

Attempting to add it here conda-forge/m2-binary-packages-feedstock#17

@IsabelParedes

Copy link
Copy Markdown
Member Author

Running aclocal in a directory with an empty configure.ac triggers the same error

- autom4te: error: need GNU m4 1.4 or later: /home/conda/feedstock_root/build_artifacts/autoconf_1746873733042/_build_env/bin/m4

@IsabelParedes

IsabelParedes commented May 14, 2025

Copy link
Copy Markdown
Member Author

It seems that autoconf is the problem
conda-forge/autoconf-feedstock#41 $\leftarrow$ fixed 🎉

@IsabelParedes

Copy link
Copy Markdown
Member Author

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits May 20, 2025 10:25
@ZheFrench

Copy link
Copy Markdown

Hi @conda-forge/r-base-feedstock-maintainers,

I hope you’re all doing well! I noticed that PR #369 to bump r-base to 4.5.0 was opened on April 22 and has cleared most review tasks. Could you please let me know if there’s anything outstanding preventing its merge, and roughly when we might expect the new r-base 4.5.0 package to become available on conda-forge?

Thank you for your work on maintaining this feedstock—I’m looking forward to testing some of the new features in R 4.5.0!

@xhochy

xhochy commented May 27, 2025

Copy link
Copy Markdown
Member

We're now down to LAPACK related linkage errors:

D:/bld/r-base-split_1748339898149/_build_env/Library/bin/../lib/gcc/x86_64-w64-mingw32/13.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot export dgemmtr_: symbol not defined
D:/bld/r-base-split_1748339898149/_build_env/Library/bin/../lib/gcc/x86_64-w64-mingw32/13.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot export zgemmtr_: symbol not defined

@IsabelParedes

Copy link
Copy Markdown
Member Author

We're now down to LAPACK related linkage errors:

D:/bld/r-base-split_1748339898149/_build_env/Library/bin/../lib/gcc/x86_64-w64-mingw32/13.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot export dgemmtr_: symbol not defined
D:/bld/r-base-split_1748339898149/_build_env/Library/bin/../lib/gcc/x86_64-w64-mingw32/13.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot export zgemmtr_: symbol not defined

For the win build, would it be ok to use the internal BLAS/LAPACK implementation instead of the external libblas and liblapack packages?

@xhochy

xhochy commented May 27, 2025

Copy link
Copy Markdown
Member

The new mentioned routines are new: https://developer.r-project.org/blosxom.cgi/R-4-5-branch/NEWS/2025/04/02

@xhochy

xhochy commented May 27, 2025

Copy link
Copy Markdown
Member

The changelog as a bit more information: https://cran.r-project.org/doc/manuals/r-release/NEWS.html

Comment thread recipe/build-r-base.sh
export LIBRARY_PATH=${PREFIX}/Library/lib

# Support older BLAS implementations (pre-2025)
sed -i '/^LIBSOURCES =/ s/$/ dgemmtr.f zgemmtr.f/' src/modules/lapack/Makefile.win

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's going to be a lot slower than intended.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then we need to update the blas version used here. I guess this means we need to get in conda-forge/blas-feedstock#114 first, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to update to 3.12.1 or backport the dgemmtr routines to 3.9

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The other option is to remove these symbols altogether. They are only used in lapack.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is removing the symbols preferable to the included patch which adds the symbols and it's working?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, removing them is preferable. can you send a PR to remove them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @isuruf !

New PR here #379

@conda-forge-admin

Copy link
Copy Markdown
Contributor

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [27]

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/15441741720. Examine the logs at this URL for more detail.

@conda-forge-admin

conda-forge-admin commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

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.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/15445224626. Examine the logs at this URL for more detail.

@IsabelParedes

Copy link
Copy Markdown
Member Author

Hi @xhochy and @isuruf !

I added a patch following the instructions in src/extra/blas/Makefile.win to integrate the new symbols into Rblas.dll and it finally passes.

Rblas.dll imports xerbla_ from R.dll
OpenBLAS 0.3.29 includes [dz]gemmtr but ATLAS does not.
So to use latter, copy [dz]gemmtr.f from ../../modules/lapack
and add dgemmtr.o zgemmtr.o to the dependencies.

@IsabelParedes

Copy link
Copy Markdown
Member Author

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits June 4, 2025 14:24
@IsabelParedes

Copy link
Copy Markdown
Member Author

@conda-forge-admin, please rerender

@ZheFrench

Copy link
Copy Markdown

Hi @conda-forge/r-base-feedstock-maintainers,

I hope you’re all doing well! I noticed that PR #369 to bump r-base to 4.5.0 was opened on April 22 and has cleared most review tasks. Could you please let me know if there’s anything outstanding preventing its merge, and roughly when we might expect the new r-base 4.5.0 package to become available on conda-forge?

Thank you for your work on maintaining this feedstock—I’m looking forward to testing some of the new features in R 4.5.0!

May I ask the same question ?

@IsabelParedes

Copy link
Copy Markdown
Member Author

Newer version #378

@IsabelParedes

Copy link
Copy Markdown
Member Author

Newer version has been merged 🎉

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.

6 participants