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

Move to Meson Build System #1245

Draft
wants to merge 14 commits into
base: feature-new-build
Choose a base branch
from

Conversation

AesaraB
Copy link
Contributor

@AesaraB AesaraB commented Feb 24, 2024

Completes #1229

Two arguments from setup.py catch our interest, they contain other commands and do everything needed to create a running instance of MyPaint:

  1. build
  2. managed_install

Build(build)

managed_install

TBD

Other setuptools/distutils Features

@AesaraB AesaraB linked an issue Feb 24, 2024 that may be closed by this pull request
@AesaraB
Copy link
Contributor Author

AesaraB commented Feb 25, 2024

Back to top

Dependencies

Meson can check for dependencies, this is what we've checked so far:

  • python
  • gtk3
  • pygobject
  • lcms
  • json-c
  • gettext
  • librsvg2
  • numpy
  • pycairo

@AesaraB
Copy link
Contributor Author

AesaraB commented Feb 25, 2024

Back to top

build_py

Inherits distutils' Command

This is probably the simplest step in the build process. It copies files from two directories, gui/ and lib/

List of files copied by build_py: build_py.txt

gui/

There are four "file types" in this directory. From what I gather, build_py just copies it all

  • *.glade
  • *.py
  • *.xml
  • directories

lib/

There are four "files types" in this directory. build_py only copies all *.py files, including ones in subdirectores.

  • *.hpp
  • *.cpp
  • *.py
  • directories

The pattern?

There are two potential matching strategies for the pattern we see: additive, or subtractive.

  1. Additive: *.glade, *.py, *.xml are copied to the build directory
  2. Subtractive: All files that aren't *.hpp or *.cpp are copied to the build directory

Translating to Meson

You can't glob files in meson like you can in setuptools/distutils. Obviously this has implications for how we compile C/C++ files, but it also poses an issue for build_py.

We can either:

  1. Individually specify all the lib/*.py files in meson.build,
    • This may create an arduously long meson.build file.
  2. Separate C/C++ files from python files in lib/, or
  3. Create a script to glob these files and modify meson.build for us.
    • I'm not sure how different this is in practice to option 1.

@AesaraB
Copy link
Contributor Author

AesaraB commented Feb 25, 2024

Back to top

BuildConfig(Command)

Inherits distutils' Command

Concept

  • lib/config.py is a file created from a template.
  • The template (which will get filled in with information we provide) is config.py.in
  • The substituted information comes in the form of a dict
  • instantiate_template() takes a template and substitution dictionary as arguments to create to create an output file (lib/config.py)

Translating to Meson

Meson does out of source builds, meaning that lib/config.py will not exist in the the source directory, but rather the build directory.

@AesaraB
Copy link
Contributor Author

AesaraB commented Feb 25, 2024

Back to top

build_ext

The gist of this class is that it basically takes in a list of C++ files alongside a list of flags and uses swig to compile them to the build directory.

Output of build_ext: build_ext.txt

@jtojnar
Copy link
Contributor

jtojnar commented Feb 25, 2024

Yeah, I would expect something like https://github.com/linux-nvme/libnvme/blob/bff7dda7e2a0f883d0b89e23fed725c916de3e61/libnvme/meson.build#L29-L45 to work.

@AesaraB AesaraB added this to the v2.1.0 milestone Feb 25, 2024
@AesaraB AesaraB self-assigned this Feb 25, 2024
'-g', # always include symbols, for profiling
]

if system_os == "darwin"

Choose a reason for hiding this comment

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

Lexer error:

meson.build:47:16: ERROR: Double quotes are not supported. Use single quotes.
if system_os == "darwin"
                ^

meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
Comment on lines +63 to +68
mypaintlib_swig = custom_target(
'mypaintlib.py',
input: ['lib/mypaintlib.i',
'lib/gdkpixbuf2numpy.cpp',
'lib/pixops.cpp',
'lib/fastpng.cpp',

Choose a reason for hiding this comment

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

I think this has to go in src/meson.build (or src/lib/meson.build), since all the paths are in src/lib/* and it will be painful as-is.

'lib/fill/blur.cpp',
'lib/fill/morphology.cpp'
],
output: ['mypaintlib.py', 'lib/mypaintlib_wrap.cpp'],

Choose a reason for hiding this comment

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

Just output it to mypaintlib_wrapper.cpp, exact paths don't really matter for a .cpp file anyway.

@AesaraB
Copy link
Contributor Author

AesaraB commented Mar 8, 2024

@eli-schwartz I appreciate your review, but the latest commit was half copied and pasted code that I was yet to fully translate. Nonetheless, you have made some useful suggestions, which I'll add in when I can sit down and look at them more closely.

@eli-schwartz
Copy link

Ahh, gotcha. :)

(I have selfish reasons for having a small interest in seeing mypaint be compiled by meson, as it's currently a pain to handle diverging expectations of PEP 517 vs traditional setup.py usage and packagers are being strongly encouraged to only use PEP 517, which doesn't work here. So I figured any small help I could provide might make that transition happen faster. Please do feel free to ask me for help.)

@AesaraB
Copy link
Contributor Author

AesaraB commented Mar 8, 2024

@eli-schwartz I'm tremendously grateful for your interest in improving MyPaint -- thank you for your offer of help.

Would you mind if I call you in to be a reviewer at the final stages of the draft?

@eli-schwartz
Copy link

I'd be thrilled to. :)

Co-authored-by: Eli Schwartz <eschwartz93@gmail.com>
@AesaraB AesaraB removed this from the v2.1.0 milestone Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Building: Move to Meson
3 participants