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

Patch meson.build for wheel build #54

Closed
wants to merge 2 commits into from
Closed

Conversation

kazarmy
Copy link
Member

@kazarmy kazarmy commented Mar 14, 2024

Since Meson doesn't allow direct reading of environment variables, this pr uses sed to patch meson.build for the pypa wheel builds. The patch result should be:

diff --git a/meson.build b/meson.build
index 3d6214c..6beb707 100644
--- a/meson.build
+++ b/meson.build
@@ -117,7 +117,7 @@ if target_swig
       '-python', '-c++',
       '-outdir', '@OUTDIR@', '@INPUT@'
     ],
-    install: host_machine.system() == 'windows',
+    install: true,
     install_dir: [py.get_install_dir(), false]
   )
   swig_py = swig_output[0]
@@ -134,7 +134,7 @@ if target_swig
       py.dependency(),
       rz_core,
     ],
-    install: host_machine.system() == 'windows',
+    install: true,
   )
   if host_machine.system() != 'windows'
     meson.add_install_script('py_install.py', swig_py.full_path(), ext_mod.full_path())

The sed suffix trick is from here, here and here.

@wargio
Copy link
Member

wargio commented Mar 14, 2024

can't you add some options and use the arguments with the env vars?

option('enable_install', type: 'combo', choices: ['enabled', 'disabled', 'auto'], value: 'auto', description: 'enables or disables the install option.')

and in the CI script we just add -Denable_install=$MESON_INSTALL or even better the variables from the matrix ?

@kazarmy kazarmy marked this pull request as ready for review March 14, 2024 11:08
@kazarmy
Copy link
Member Author

kazarmy commented Mar 14, 2024

The CI build script has this:

    - name: Build rz-bindgen
      uses: pypa/[email protected]

and I don't see how one can add -Denable_install=$MESON_INSTALL to that.

@kazarmy
Copy link
Member Author

kazarmy commented Mar 14, 2024

But maybe I should look further

@kazarmy kazarmy marked this pull request as draft March 14, 2024 11:12
@kazarmy
Copy link
Member Author

kazarmy commented Mar 15, 2024

Superseded by #55

@kazarmy kazarmy closed this Mar 15, 2024
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