Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Port build system to Meson#26621

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
oscargus merged 8 commits intomatplotlib:mainfromQuLogic:meson
Oct 4, 2023
Merged

Port build system to Meson#26621

oscargus merged 8 commits intomatplotlib:mainfromQuLogic:meson
Oct 4, 2023

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedAug 29, 2023
edited
Loading

PR summary

Now that NumPy and SciPy have spent some time on Meson, it's not too bad to move our build system over to Meson as well. This PR also implements the move topyproject.toml from#23829. I have added some documentation with some key points for migration, though most are edge cases that most developers shouldn't have to worry about.

The main change is that editable installs should disable build isolation:

$ pip install --no-build-isolation --editable .

This is becausepyproject.toml-based install would normally isolate builds and extensions are built in the isolated environment, which goes against the idea of an editable install. (You will also need to install build requirements in your target environment, but I think that's not new anyway.)

The other downside is that all files need to be listed manually in mostmeson.build.However, this also has a side benefit of not accidentally installing extra files (e.g., I get several temporary*.gcno files and the wholenode_modules directory if they happen to exist onmain.)

The upside is that compilation is written in a very standard way. Both FreeType and Qhull are subprojects using manually writtenmeson.build but those are just the same as the files in the main source. Qhull's build is a simple port of what we did insetupext.py tomeson.build. FreeType's build is based onthe 2.9.1 version in Meson's wrapdb, but dropping some of the extra install bits we won't use and rolled back to 2.6.1. For 2.11.1, we use the Meson build system already part of FreeType. When we move forward on FreeType versions, we can drop the patched-in build and use the upstream build system.

The other upside of Meson+meson-python builds is that compilation is a) faster with better parallelism since the entire set of files can be seen by the build system, and b) can be doneat import time in an editable install. The latter means that you will no longer need to re-install when editing C(++) files (unless you switch to an older commit.)

Closes#23829

PR checklist

thesamesam and the-pawel-wojcik reacted with hooray emoji
@QuLogic
Copy link
MemberAuthor

And looking back at#23829, I forgot about adding thedev extra.

@QuLogicQuLogicforce-pushed themeson branch 9 times, most recently from1ba714f toee25941CompareAugust 31, 2023 10:38


if len(sys.argv) < 4:
raise SystemExit('usage: {sys.argv[0]} <input> <output> <backend>')
Copy link
Member

Choose a reason for hiding this comment

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

This should have anif __name__ == '__main__': guard

@QuLogicQuLogic added the CI: Run cygwinRun cygwin tests on a PR labelAug 31, 2023
@QuLogicQuLogicforce-pushed themeson branch 2 times, most recently from86b2d7b to7f5d3a9CompareSeptember 1, 2023 07:04
@QuLogicQuLogic added the CI: Run cibuildwheelRun wheel building tests on a PR labelSep 1, 2023
@QuLogicQuLogicforce-pushed themeson branch 2 times, most recently from9eaf930 tocb71290CompareSeptember 1, 2023 08:57
@QuLogicQuLogic mentioned this pull requestSep 2, 2023
1 task
@QuLogic
Copy link
MemberAuthor

OK, I think we're at a good point now. A couple of issues had to be cleared up:

  • I accidentally forgot to install themacosx backend, which made it unavailable in tests, so auto-picking in subprocesses chose TkAgg, which is still broken on Azure. I have fixed the backend here, but also made the subprocess explicitly use Agg where possible inFix flaky CI tests #26680. Unfortunately, I don't think we can fixtest_lazy_auto_backend_selection as its point is to not have a backend set.
  • In some CI, we use the backend override frommplsetup.cfg; I've replicated that using the Meson equivalent, but I wonder if we should do that everywhere.
  • Azure and AppVeyor have a ton of compilers preinstalled in addition to MSVC. Meson picks the non-MSVC option by default, so I have explicitly passed the option to use MSVC in CI. Since we don't specifically hard-require MSVC, I have not set that option in our config, and I expect most developers do not have multiple compilers on Windows, but it is a gotcha on CI.

There are also a couple followups that need to be considered eventually:

There isn't a huge change for developers, and you might be able to glean that from the CI changes (though most changes are extraneous for local work.) There may be a bit more change for distributions, but they might be used to that from NumPy/SciPy's existing change. I will write up some docs for that next week.

thesamesam reacted with eyes emoji

@QuLogic
Copy link
MemberAuthor

OK, there is actually one stall on the macOS arm64 builds, which I've reported upstream atmesonbuild/meson-python#468

@ksunden
Copy link
Member

I have found a solution that appears towork regarding getting stubtest running with editable install, though not sure its actually agood solution...

If you add the$REPO_ROOT/lib to the mesonpy-generatedmatplotlib-editable.pth file insite-packages, stubtest is able to run and give accurate results. (One slight change from setuptools editable install is that we had unused allowlist entries for untyped subpackages, but that is pretty trivial)

As far as I can tell it does not affect the special handling set up by mesonpy in_matplotlib_editable_loader, but does allow mypy to connect the stubs to the runtime.

All that said, while it may be good enough for a controlled CI environment, I don't like that as a solution for developers who may wish to run stubtest on their machine (note thatmypy alone will work fine, just not with-p or-m, you must give it thelib/matplotlib path... stubtest requires runtime, so is more restricted) In my testing my editor integration still worked fine without any changes.

Perhaps this is a safe enough change and we should lobby/PR upstream to include it to support the mypy usecase.

The other option which seems to also be broken right now, but I don't think should be a problem, is to do a non-editable install... this is not great for the developer machine use case either, but at least does not involve editing generated files...

Currently I'm getting:

error: matplotlib failed to import, InvalidVersion: Invalid version: '"3.9.0.dev0"'

Which is a weird error on a couple fronts, firstly being why is version needed for importing in the first place?, secondly being why is that an invalid version?

Investigation has revealed that stubtest is doing (the__import__ equivalent of)from matplotlib import *, and that is what is triggering the "InvalidVersion". Testing on a REPL confirms that whileimport matplotlib is fine, trying to get__version_info__ (including a* import) breaks. The problem appears to be that there are quotes on__version__.

In summary:

  • I think after sorting out__version_info__ problems probably the best short term path for CI is to use non-editable install for the stubtest job
  • If that doesn't work, editing thepth file should
  • May be worth talking to upstream about this usecase and seeing if the addition to the pth file can be included there, which would help for developer machines without having to edit a generated file.

Copy link
Member

@oscargusoscargus left a comment

Choose a reason for hiding this comment

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

This seems to be OK now? To some extent it may be better to try and get this in so that there is ample time before the 3.9-release to find any issues, but I am still a bit worried to merge it myself...

Well, what is the worst that can happen?

@oscargusoscargus merged commit5ef56df intomatplotlib:mainOct 4, 2023
@greglucas
Copy link
Contributor

To some extent it may be better to try and get this in so that there is ample time before the 3.9-release to find any issues, but I am still a bit worried to merge it myself...

Well, what is the worst that can happen?

I would not be worried. You had another approving review and merged it as the maintainer's workflow says. As you say, this will give a good opportunity to see if anything does crop up rather than waiting until the last minute.

@QuLogicQuLogic deleted the meson branchOctober 4, 2023 19:15
@QuLogic
Copy link
MemberAuthor

This removes special casing for AIX and IBM i OS, but the FreeType build is using an included Meson system instead of their own old autotools-based build.

@ayappanec@zheddie can you confirm whether themain branch still builds for you?

@ayappanec
Copy link
Contributor

@KamathForAIX Can you confirm whether the main branch builds fine in AIX ?

@KamathForAIX
Copy link
Contributor

Hi@QuLogic ,

So in AIX gcc with LTO option is not supported. So in meson.build file we should not use it as a default option if the target is AIX. If we can write a logic like that it would be great.

Secondly, in the matplotlib/tools/generate_matplotlibrc.py I had to change !/usr/bin/env python to /usr/bin/env python3.9..

With the two changes mentioned above we are able to build in AIX. Kindly let me know what we can do regarding the same.

@tacaswell
Copy link
Member

Does!/usr/bin/env python3 work (rather than python3.9)?

@KamathForAIX
Copy link
Contributor

@tacaswell Yes it works

@QuLogic
Copy link
MemberAuthor

So in AIX gcc with LTO option is not supported. So in meson.build file we should not use it as a default option if the target is AIX. If we can write a logic like that it would be great.

If that is not supported, then this should be handled upstream in Meson. If they silently ignoreb_lto=true, then we will have to do nothing. If they error out, then we should see if we can conditionalize it.

@KamathForAIX
Copy link
Contributor

KamathForAIX commentedOct 18, 2023
edited
Loading

Hi@QuLogic ,

Sure. We will work this out with the meson community members. No worries. Thank you for connecting with AIX for the meson build system test for Matplotlib. If there is anything update or help needed we will get back to you.

Is there anything else we can do for Matplotlib community from AIX? Kindly let us know.

Have a nice day ahead.

Regards,
Aditya Kamath.

@tacaswell
Copy link
Member

@KamathForAIX I do not know of any CI service that offer AIX workers so we are a bit blind to if we accidentally break anything. If you could have a monthly cron-job that tries to build Matplotlib and run the tests and report any failures up to us that would be super helpful!

@KamathForAIX
Copy link
Contributor

Hi@tacaswell

Sure, we will set a CI for matplotlib and run the tests.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@WeatherGodWeatherGodWeatherGod left review comments

@greglucasgreglucasgreglucas approved these changes

@oscargusoscargusoscargus approved these changes

Assignees
No one assigned
Labels
BuildCI: Run cibuildwheelRun wheel building tests on a PRCI: Run cygwinRun cygwin tests on a PRDocumentation: buildbuilding the docs
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

8 participants
@QuLogic@ksunden@oscargus@greglucas@ayappanec@KamathForAIX@tacaswell@WeatherGod

[8]ページ先頭

©2009-2025 Movatter.jp