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

Allow external configuration of endianness in R package build #10642

Merged
merged 5 commits into from
Jul 30, 2024

Conversation

jakirkham
Copy link
Contributor

Rework R package's configure.ac to accept and use the USE_LITTLE_ENDIAN environment variable. If set, this allows overriding the endianness used for the R package built. If not set, then this falls back to autoconf's own endianness check. If it is still not clear what endianness to use, the build then errors and asks for it to be specified.

For more context conda-forge builds have needed to patch out the existing configuration logic for endianness and set their own value as the current check isn't always correct. This would allow the conda-forge builds to drop that patch and simply specify USE_LITTLE_ENDIAN when needed (a more maintainable solution).

@jakirkham jakirkham mentioned this pull request Jul 27, 2024
15 tasks
@jakirkham
Copy link
Contributor Author

jakirkham commented Jul 27, 2024

Noticed a couple failures here. In looking into them, am curious does this go back to dmlc-core. Do we need to update dmlc-core as well?

On a somewhat related note, it appears dmlc-core is using some deprecated endianness logic: dmlc/dmlc-core#688

@hcho3
Copy link
Collaborator

hcho3 commented Jul 29, 2024

Added AC_CONFIG_HEADERS() macro to avoid the warning

configure.ac:87: warning: AC_C_BIGENDIAN should be used with AC_CONFIG_HEADERS

This warning causes the R build to fail: https://github.com/dmlc/xgboost/actions/runs/10152770673/job/28074838045?pr=10642

@hcho3 hcho3 merged commit 757aafc into dmlc:master Jul 30, 2024
30 checks passed
hcho3 added a commit to hcho3/xgboost that referenced this pull request Jul 30, 2024
…0642)

* Allow users to set endianness in R build

* Run `autoreconf -vi`

* Don't use :BOOL suffix

* Use AC_CONFIG_HEADERS

---------

Co-authored-by: Hyunsu Cho <[email protected]>
Co-authored-by: Philip Hyunsu Cho <[email protected]>
hcho3 added a commit that referenced this pull request Jul 30, 2024
…ild (#10642) (#10645)

* Allow external configuration of endianness in R package build (#10642)

* Allow users to set endianness in R build

* Run `autoreconf -vi`

* Don't use :BOOL suffix

* Use AC_CONFIG_HEADERS

---------

Co-authored-by: Hyunsu Cho <[email protected]>
Co-authored-by: Philip Hyunsu Cho <[email protected]>

* Re-run roxygen2

---------

Co-authored-by: jakirkham <[email protected]>
@jakirkham jakirkham deleted the use_external_endian_flag branch July 30, 2024 19:08
@jakirkham
Copy link
Contributor Author

Thanks Hyunsu! 🙏

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.

2 participants