sage-env: identify the version of command-line tools (OS X) and set LDFLAGS accordingly by jhpalmieri · Pull Request #36599 · sagemath/sage

jhpalmieri · 2023-10-30T22:59:31Z

Identify the version of command-line tools in use (OS X), and use the version to set LDFLAGS.

In sage-env, save the version of command-line tools being used in XCLT_VERSION. If it's too recent, we set LDFLAGS to solve some problems:

Fixes In doctesting, ignore two ld warnings from recent OS X: #36337 (doctest failures)
Fixes Build failures with Xcode CLT 15.0: scipy (via meson), ecm #36342 (scipy building, replacing scipy: Fix build on macOS with Xcode CLT 15.0 #36523).

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

jhpalmieri · 2023-10-30T23:01:10Z

In a perfect world, we would document variables like this somewhere in the developer's guide...

jhpalmieri · 2023-11-01T18:42:10Z

We could also set LDFLAGS depending on XCLT_VERSION, either here or on another PR. (See #36337 (comment).)

jhpalmieri · 2023-11-02T05:02:44Z

With this change, scipy builds and I don't get the "ignoring duplicate libraries" messages in the sagelib log or in doctests.

jhpalmieri · 2023-11-02T05:03:36Z

I only have access to OS X machines with the latest command-line tools, so I can't test as thoroughly as I would like.

mkoeppe · 2023-11-04T22:07:29Z

src/bin/sage-env

    export MACOSX_VERSION=`uname -r | awk -F. '{print $1}'`
+    # Try to identify command-line tools version.
+    # See https://apple.stackexchange.com/q/180957/351985.
+    XCLT_VERSION=`pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | awk '/version: / { print $NF }' | cut -d. -f-1`


I'm not sure if querying with pkgutil is the right approach.
I think on a system several versions of CLT (and separately several versions of Xcode) can be installed.

mkoeppe · 2023-11-04T22:08:37Z

I think it may be better to just check whether $(xcode-select -p)/usr/bin/ld-classic exists.

jhpalmieri · 2023-11-04T22:34:57Z

I think it may be better to just check whether $(xcode-select -p)/usr/bin/ld-classic exists.

Like this?

    if [ -x "$(xcode-select -p)/usr/bin/ld-classic" ] ; then
        CLT_LD_CLASSIC=yes
    else
        CLT_LD_CLASSIC=no
    fi
    export CLT_LD_CLASSIC

(Might as well test whether it's executable, right?)

jhpalmieri · 2023-11-04T22:39:11Z

I think it may be better to just check whether $(xcode-select -p)/usr/bin/ld-classic exists.

Like this?
    if [ -x "$(xcode-select -p)/usr/bin/ld-classic" ] ; then
        CLT_LD_CLASSIC=yes
    else
        CLT_LD_CLASSIC=no
    fi
    export CLT_LD_CLASSIC
(Might as well test whether it's executable, right?)

(Or move this check later when setting LD_FLAGS, and don't define a variable to track it — that's a little cleaner, if we're just using this to set LD_FLAGS.)

mkoeppe · 2023-11-04T22:41:02Z

Yes, looks good to me

and if so, set LDFLAGS to use it.

jhpalmieri · 2023-11-05T00:07:30Z

Okay, here's a new version.

github-actions · 2023-11-05T00:53:11Z

Documentation preview for this PR (built with commit 543cd2d; changes) is ready! 🎉

jhpalmieri · 2023-11-05T01:44:30Z

Thanks!

sagemathgh-36599: sage-env: identify the version of command-line tools (OS X) and set LDFLAGS accordingly Identify the version of command-line tools in use (OS X), and use the version to set LDFLAGS.   In sage-env, save the version of command-line tools being used in XCLT_VERSION. If it's too recent, we set LDFLAGS to solve some problems: - Fixes sagemath#36337 (doctest failures) - Fixes sagemath#36342 (scipy building, replacing sagemath#36523).    ### 📝 Checklist     - [X] The title is concise, informative, and self-explanatory. - [X] The description explains in detail what this PR is about. - [X] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36599 Reported by: John H. Palmieri Reviewer(s): Matthias Köppe

…ests on M1 runners, add timeouts   - Fixing the failure with `macos-13-homebrew`, as seen in https://github.com/sagemath/sage/actions/runs/7761817710/job/21171032513 and upstream uses of macos.yml dimpase/primecountpy#11 (comment), sagemath/cypari2#152 - Generalizing the `ld_classic` workaround from sagemath#36599 for the case of full XCode. - This is to fix the failure seen on `homebrew-macos-13-usrlocal- standard-xcode_15.0`, e.g. in https://github.com/scipopt/SCIP- SDP/actions/runs/7793548242/job/21253452882?pr=9#step:10:2766 - Deactivating the `ld_classic` workaround when `LD` is set, as is in a conda environment; as seen e.g. in https://groups.google.com/g/sage- devel/c/1viBzw-ZaoQ/m/S_u9K20xAAAJ - [Apple Silicon runners recently became available](https://github.blog/changelog/2024-01-30-github-actions- introducing-the-new-m1-macos-runner-available-to-open-source/); we add some tests (example run: https://github.com/mkoeppe/sage/actions/runs/7772086965/job/21194110510) - On Apple Silicon, `tox -e local-conda-forge` now uses the correct installer - Intel runners were recently upgraded too, increase parallelism. - Add self-destruct sequence to cancel workflows before the 6h limit (catches linbox-related build hang) - Actually handle the input `extra_sage_packages` (for scipopt/PySCIPOpt#630)    Follow-ups, not addressed here: - conda-forge-macos-latest-minimal tachyon build failure - needs sagemath#36969 - adding a config that tests arm64 conda-forge-minimal - would need config.guess updates in numerous packages - arm64 conda-forge-standard - matplotlib build failure (https://github. com/mkoeppe/sage/actions/runs/7810111886/job/21313997438#step:10:2749) - hangs or excessive build time that lead to timeouts, probably from linbox ### :memo: Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### :hourglass: Dependencies   URL: sagemath#37237 Reported by: Matthias Köppe Reviewer(s): Matthias Köppe, Tobias Diez

sagemathgh-37237: CI macOS: Fix failure with macos-13-homebrew, add tests on M1 runners, add timeouts   - Fixing the failure with `macos-13-homebrew`, as seen in https://github.com/sagemath/sage/actions/runs/7761817710/job/21171032513 and upstream uses of macos.yml dimpase/primecountpy#11 (comment), sagemath/cypari2#152 - Generalizing the `ld_classic` workaround from sagemath#36599 for the case of full XCode. - This is to fix the failure seen on `homebrew-macos-13-usrlocal- standard-xcode_15.0`, e.g. in https://github.com/scipopt/SCIP- SDP/actions/runs/7793548242/job/21253452882?pr=9#step:10:2766 - Deactivating the `ld_classic` workaround when `LD` is set, as is in a conda environment; as seen e.g. in https://groups.google.com/g/sage- devel/c/1viBzw-ZaoQ/m/S_u9K20xAAAJ - [Apple Silicon runners recently became available](https://github.blog/changelog/2024-01-30-github-actions- introducing-the-new-m1-macos-runner-available-to-open-source/); we add some tests (example run: https://github.com/mkoeppe/sage/actions/runs/7772086965/job/21194110510) - On Apple Silicon, `tox -e local-conda-forge` now uses the correct installer - Intel runners were recently upgraded too, increase parallelism. - Add self-destruct sequence to cancel workflows before the 6h limit (catches linbox-related build hang) - Actually handle the input `extra_sage_packages` (for scipopt/PySCIPOpt#630)    Follow-ups, not addressed here: - conda-forge-macos-latest-minimal tachyon build failure - needs sagemath#36969 - adding a config that tests arm64 conda-forge-minimal - would need config.guess updates in numerous packages - arm64 conda-forge-standard - matplotlib build failure (https://github. com/mkoeppe/sage/actions/runs/7810111886/job/21313997438#step:10:2749) - hangs or excessive build time that lead to timeouts, probably from linbox ### :memo: Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### :hourglass: Dependencies   URL: sagemath#37237 Reported by: Matthias Köppe Reviewer(s): Matthias Köppe, Tobias Diez

sage-env: identify the version of command-line tools (OS X)

5251c2f

jhpalmieri added t: enhancement p: minor / 4 s: needs review c: build labels Oct 30, 2023

jhpalmieri mentioned this pull request Oct 30, 2023

scipy: Fix build on macOS with Xcode CLT 15.0 #36523

Closed

5 tasks

Merge branch 'develop' into identify-clt-version

097e704

Set LDFLAGS depending on version of OS X command-line tools

2341fb4

jhpalmieri added p: critical / 2 and removed p: minor / 4 labels Nov 2, 2023

jhpalmieri changed the title ~~sage-env: identify the version of command-line tools (OS X)~~ sage-env: identify the version of command-line tools (OS X) and set LDFLAGS accordingly Nov 2, 2023

mkoeppe reviewed Nov 4, 2023

View reviewed changes

Check whether command-line tools includes the executable ld-classic,

543cd2d

and if so, set LDFLAGS to use it.

mkoeppe approved these changes Nov 5, 2023

View reviewed changes

vbraun merged commit 8898473 into sagemath:develop Nov 10, 2023

github-actions bot removed the s: positive review label Nov 10, 2023

Uh oh!

Comments

Conversation

jhpalmieri commented Oct 30, 2023 • edited by mkoeppe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

jhpalmieri commented Oct 30, 2023

Uh oh!

jhpalmieri commented Nov 1, 2023

Uh oh!

jhpalmieri commented Nov 2, 2023

Uh oh!

jhpalmieri commented Nov 2, 2023

Uh oh!

mkoeppe Nov 4, 2023

Choose a reason for hiding this comment

Uh oh!

mkoeppe commented Nov 4, 2023

Uh oh!

jhpalmieri commented Nov 4, 2023

Uh oh!

jhpalmieri commented Nov 4, 2023

Uh oh!

mkoeppe commented Nov 4, 2023

Uh oh!

jhpalmieri commented Nov 5, 2023

Uh oh!

github-actions bot commented Nov 5, 2023

Uh oh!

jhpalmieri commented Nov 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jhpalmieri commented Oct 30, 2023 •

edited by mkoeppe

Loading