Skip to content

Add install-tags filtering for wheel generation #267

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Add install-tags filtering for wheel generation #267

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a7ad70c
add unofficial 'install-tags' option
peter-urban Nov 25, 2022
dc77812
Merge branch 'origin-main' into fork-main
peter-urban Nov 28, 2022
85e7092
Merge remote-tracking branch 'origin/main' into fork-main
peter-urban Dec 1, 2022
d2d9ee2
Merge remote-tracking branch 'origin/main' into fork-main
peter-urban Dec 2, 2022
1acfde3
Merge remote-tracking branch 'original/main'
Jan 17, 2023
794bf78
Update formatting in __init__.py
Jan 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions mesonpy/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ def build_editable(self, directory: Path, verbose: bool = False) -> pathlib.Path
return wheel_file


MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install']
MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install', 'install-tags']
Copy link
Member

Choose a reason for hiding this comment

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

I wonder it wouldn't just be better to parse this from the install args.

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, I'll implement this

Suggested change
MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install', 'install-tags']
MesonArgsKeys = Literal['dist', 'setup', 'compile', 'install']

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! This can be done with argparse and parse_known_args.

MesonArgs = Mapping[MesonArgsKeys, List[str]]


Expand Down Expand Up @@ -876,8 +876,12 @@ def build_commands(self, install_dir: Optional[pathlib.Path] = None) -> Sequence
@functools.lru_cache(maxsize=None)
def build(self) -> None:
"""Trigger the Meson build."""
for cmd in self.build_commands():
self._meson(*cmd[1:])
self._meson('compile', *self._meson_args['compile'],)
if self._meson_args['install-tags']:
install_tags = '--tags=' + ','.join(self._meson_args['install-tags'])
self._meson('install', '--destdir', os.fspath(self._install_dir), install_tags, *self._meson_args['install'],)
else:
self._meson('install', '--destdir', os.fspath(self._install_dir), *self._meson_args['install'],)
Comment on lines -879 to +884
Copy link
Member

Choose a reason for hiding this comment

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

I think this may have happened because of the merge, but this code now needs to go into build_commands.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, this is a merge error, my code was still based on a very old version of meson-python

Copy link
Contributor

@peter-urban peter-urban Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

if install-tags is not separated from the "install" args, this whole block can be reverted to
for cmd in self.build_commands(): self._meson(*cmd[1:])


@classmethod
@contextlib.contextmanager
Expand All @@ -903,6 +907,12 @@ def _info(self, name: str) -> Dict[str, Any]:

@property
def _install_plan(self) -> Dict[str, Dict[str, Dict[str, str]]]:
install_plan = self._info('intro-install_plan').copy()

for files in install_plan.values():
for file, details in list(files.items()):
if details['tag'] not in self._meson_args['install-tags']:
del files[file]
Comment on lines 909 to +915
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we should do it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

i updated this block to parse the "install" args instead of using an extra 'install-tags' field

Suggested change
def _install_plan(self) -> Dict[str, Dict[str, Dict[str, str]]]:
install_plan = self._info('intro-install_plan').copy()
for files in install_plan.values():
for file, details in list(files.items()):
if details['tag'] not in self._meson_args['install-tags']:
del files[file]
def _install_plan(self) -> Dict[str, Dict[str, Dict[str, str]]]:
"""Meson install_plan metadata."""
# copy the install plan so we can modify it
install_plan = self._info('intro-install_plan').copy()
# parse install args for install tags (--tags)
install_tags = []
for arg in self._meson_args['install']:
if arg.strip().startswith('--tags='):
install_tags = arg.split('=', 1)[1].split(',')
break
else:
return install_plan
# filter out files that do not fit the install tags
for files in install_plan.values():
for file, details in list(files.items()):
if details['tag'].strip() not in install_tags:
del files[file]
return install_plan

Copy link
Contributor

Choose a reason for hiding this comment

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

@FFY00 This is just a suggestion (it was easy to implement it here).
If you give me some hints or thoughts on your doubts I could look for a better place to put this filter.

Copy link
Member

Choose a reason for hiding this comment

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

This is a bit fragile, and I'd prefer not having to maintain it. I think it should be pretty easy to use argparse, as I mentioned in #267 (comment). What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh ja, I didn't see the comment 🙂 I'll update the suggestion tomorrow

Copy link
Member

Choose a reason for hiding this comment

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

No worries. Thank you 😊

Copy link
Contributor

@peter-urban peter-urban Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Updated the suggestion to use argparse

Suggested change
def _install_plan(self) -> Dict[str, Dict[str, Dict[str, str]]]:
install_plan = self._info('intro-install_plan').copy()
for files in install_plan.values():
for file, details in list(files.items()):
if details['tag'] not in self._meson_args['install-tags']:
del files[file]
def _install_plan(self) -> Dict[str, Dict[str, Dict[str, str]]]:
"""Meson install_plan metadata."""
# copy the install plan so we can modify it
install_plan = self._info('intro-install_plan').copy()
# parse install args for install tags (--tags)
parser = argparse.ArgumentParser()
parser.add_argument('--tags')
args, _ = parser.parse_known_args(self._meson_args['install'])
# filter the install_plan for files that do not fit the install tags
if args.tags:
install_tags = args.tags.split(',')
for files in install_plan.values():
for file, details in list(files.items()):
if details['tag'].strip() not in install_tags:
del files[file]
return install_plan

naturally this needs import argparse at the beginning of the file

"""Meson install_plan metadata."""
return self._info('intro-install_plan').copy()

Expand Down