Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

update meson and meson-python to the latest versions, 1.7.0 and 0.7.1 #39492

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Feb 11, 2025

Meson update is needed to deal with the issue found on #38749
A straightforward update. This will fix #39490

In particular, building of the latest cysignals on macOS might need this.

📝 Checklist

  • The title is concise and informative.
  • 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 and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Feb 11, 2025

Documentation preview for this PR (built with commit 4b82be4; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

LGTM, but I would prefer if someone actually using sage-the-distro tests this.

@dimpase dimpase requested a review from kwankyu February 11, 2025 20:01
@dimpase dimpase mentioned this pull request Feb 13, 2025
5 tasks
@dimpase
Copy link
Member Author

dimpase commented Feb 24, 2025

ping?

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

It seems that we need CI macOS incremental / minimal | standard...

@dimpase
Copy link
Member Author

dimpase commented Feb 26, 2025

It seems that we need CI macOS incremental / minimal | standard...

what? this PR unblocks cysignals update on your favourite OS!

@dimpase
Copy link
Member Author

dimpase commented Feb 26, 2025

It seems that we need CI macOS incremental / minimal | standard...

@roed314 @saraedum - kwankyu is engaging in plain sabotage of Sage development process here. Please talk him or I'll ping sage-abuse

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

It seems that we need CI macOS incremental / minimal | standard...

what?

@tobiasdiez prefers that "someone actually using sage-the-distro tests this."

As you say, macOS is the main platform, for which this PR should be tested.

I noticed that among the Checks, there is no "CI macOS incremental" workflow, which would test the PR branch for sage-the-distribution on macOS, instead of "someone".

As we don't have such workflow, I am running the test on my own machine.

Meanwhile I wrote the comment.

@user202729
Copy link
Contributor

user202729 commented Feb 26, 2025

It seems that we need CI macOS incremental / minimal | standard...

@roed314 @saraedum - kwankyu is engaging in plain sabotage of Sage development process here. Please talk him or I'll ping sage-abuse

Uh, I see the comment as a disjoint thought at most i.e. "hm, maybe this thing would be useful to prevent issues like this?" instead of anything specifically have to do with this pull request i.e. it's not "we need this or I'll dispute this pull request". Let's let it rest.


Anyway, on the technical issue side: Isn't that pretty much what vbraun is doing, except it's done before a release instead of after? And aren't the minimal/standard/whatever thing only run after a release instead of on each pull request anyway?

(though I don't really understand what exactly is being done apart from "a lot of testing before release")

[edit: okay looks like sometimes the checks for CI Linux is ran e.g. on this pull request. I guess there's a button somewhere to trigger the run.]

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

Off-topic:

(though I don't really understand what exactly is being done apart from "a lot of testing before release")

Me too. On the other hand, portability CI (CI Linux, etc.) implemented by github actions is documented and used by developers.

[edit: okay looks like sometimes the checks for CI Linux is ran e.g. on this pull request. I guess there's a button somewhere to trigger the run.]

There is no "button". It is automatically triggered when there are changes in build/pkgs, that is, sage packages.

In #39009, I added a "button" (workflow-dispatch) to run "CI Linux" for selected platforms.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

On the present PR branch, sage built successfully on my macOS machine. There are two doctest failures

sage -t --warn-long 5.0 --random-seed=79799359098827494157399476478347523676 src/sage/repl/ipython_extension.py  # 1 doctest failed
sage -t --warn-long 5.0 --random-seed=79799359098827494157399476478347523676 src/sage/libs/giac/__init__.py  # Timed out

Maybe unrelated.

On the other hand, "./configure" said

meson:                          using system package; SPKG will not be installed
meson_python:                   no; standard, SPKG version 0.17.1 will be installed

Hence I did not actually build sage with the upgraded "meson" package!

So I ran make again, this time after

$ ./configure --with-system-meson=no
...
meson:                          no; standard, SPKG version 1.7.0 will be installed
meson_python:                   no; standard, SPKG version 0.17.1 is already installed
...

We could save developer time if there were "CI macOS incremental / minimal | standard"...

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

Sage built well again, and the same doctests failed.

I now leave review to @tobiasdiez.

@tobiasdiez
Copy link
Contributor

I now leave review to @tobiasdiez.

Seems like bad times for sage-the-distro if people using it are not even willing to review package upgrade PRs.

@dimpase
Copy link
Member Author

dimpase commented Feb 26, 2025

We could save developer time if there were "CI macOS incremental / minimal | standard"...

even more time would be saved if meson were to be required to come from the system, full stop, no gazzillion cases to check.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 26, 2025

I now leave review to @tobiasdiez.

Off-topic:

... people using it are not even willing to review package upgrade PRs.

That is true if the "people" is only me, and I refused to spend time to check the PR branch on my own machine after you paused reviewing this PR hoping someone to check the sage-the-distro part.

@dcoudert
Copy link
Contributor

I tested on top of #39571 and using ./configure --enable-system-site-packages --with-python=`which python3.12`.
It builds and seems ok to me.

@vbraun vbraun merged commit d6c136c into sagemath:develop Feb 28, 2025
20 of 47 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrade meson to 1.7.0
6 participants