- 
                Notifications
    
You must be signed in to change notification settings  - Fork 81
 
ENH: add tool.meson-python.wheel.exclude setting #766
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
Conversation
6b98023    to
    d68c896      
    Compare
  
    | 
           Another idea I was toying with is to extend the filtering by meson install tags to allow to specify different tags per subproject. meson-python does not rely on the meson implementation of the filtering anyway, thus extending the facility is possible.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dnicolodi! I like this. Agreed that these unmappable install locations/targets are a pain point, and this is a nice and ergonomic solution I think.
The wheel.exclude name is good, and it matches the name in scikit-build-core which is helpful.
One question about using introspection metadata locations. I can see that working, but there's also an alternative of using wheel locations - I'm not quite sure which is better.
          
 That may still require changes to the subproject itself (e.g., there are multiple shared libraries and you need only one of them), so is less general. I prefer   | 
    
| 
           We could also consider implementing both   | 
    
          
 I can see the appeal of that, yes. Especially on projects with lots of install targets, that seems nice. Although the cases where you can't make things concise with excluding install tags +  This could be done immediately, or later if there's demand. Given this PR is very nice and clean, may be okay to add it immediately?  | 
    
| 
           I've implemented  Working on this it came to my mind that we could also have a   | 
    
          
 Yeah potentially useful, but also potentially easy to misuse. This thread from yesterday is a good example: https://discuss.python.org/t/include-header-above-project-root/102221. Could potentially be a nicer way to include a C library that's one folder up in the repo, instead of symlinking it in - we've had examples of this, like  On the other hand, this seems quite bad: https://hatch.pypa.io/latest/config/build/#forced-inclusion. It gives an explicit example of pulling in a header file from outside the repo, which seems like a good way to create non-reproducible builds/issues.  | 
    
| 
           Hatchling has had some really quite inspirational bugs if I recall correctly, related to trying to match inclusion patterns against parent directories of the project root. Very fun, in the sense of not being fun at all, when it incorrectly operates on the standard system directory structure used for ALL package builds. 
 Not really sure I understand the problem. If you're building a C library one folder up in the repo, I'm guessing this is a C library and its optional bindings. You really want the entire project to be a single build including both C and python bits. Autotools / Meson / CMake can all do that already, with varying abilities to install the python code in the right site-packages directory, and with meson-python you can even support both building a system C library and its bindings, OR a PyPI wheel that statically links the C library for standalone distribution. You really don't want to require "one folder up" because it's not possible to produce an sdist like that, and you can't guarantee the C library is built when needed if it's two separate builds.  | 
    
          
 What I had in mind would not solve this issue. What I had in mind is a facility that maps a Meson installation path, for example to another, for example   | 
    
          
 The point is that you can't do it through a standard build frontend interface if the C library code isn't inside the part of the repo containing  
 Good point. Okay, leaving that problem for another day then:) The added   | 
    
          
 I assume you want to leave an update to those docs out of this PR?  | 
    
9bf9d07    to
    0b703da      
    Compare
  
    | 
           I've updated the documentation.  | 
    
0b703da    to
    4c0d817      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs LGTM, thanks. Two small suggestions only. Also there's a merge conflict to resolve. When that is resolved, this looks ready to go in.
4c0d817    to
    30a0d32      
    Compare
  
    30a0d32    to
    f3393d2      
    Compare
  
    
I am not entirely sure we want to include this facility: it may potentially be abused when better solutions exist. However, I think it is better than the solution we currently recommend https://mesonbuild.com/meson-python/how-to-guides/shared-libraries.html#shared-library-from-subproject of adding specific options to subprojects to exclude this or that parts from the installation.