Skip to content

interpreter: Move validation of BuildTarget(extra_files) to Interpreter #15189

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

interpreter: Move validation of BuildTarget(extra_files) to Interpreter #15189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 1 addition & 11 deletions mesonbuild/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1285,17 +1285,7 @@ def process_kwargs(self, kwargs: BuildTargetKeywordArguments) -> None:
(str, bool))
self.install_mode = kwargs.get('install_mode', None)
self.install_tag = stringlistify(kwargs.get('install_tag', [None]))
extra_files = kwargs.get('extra_files', [])
for i in extra_files:
# TODO: use an OrderedSet instead of a list?
if i in self.extra_files:
continue
# TODO: this prevents built `File` objects from being used as
# extra_files.
trial = os.path.join(self.environment.get_source_dir(), i.subdir, i.fname)
Copy link
Member

@eli-schwartz eli-schwartz Nov 3, 2025

Choose a reason for hiding this comment

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

Is this changing? Are we adding support for new things here?

Copy link
Member Author

@dcbaker dcbaker Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is checking that we don't use generated files, which we could also check with File.is_built.

Copy link
Member Author

@dcbaker dcbaker Nov 3, 2025

Choose a reason for hiding this comment

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

I updated the commit message to have that.

I originally had several patches in this series that made the transformation one change at a time, but I feel like I always get asked to squash that, so I didn't send it as a split series.

Copy link
Member

@eli-schwartz eli-schwartz Nov 3, 2025

Choose a reason for hiding this comment

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

Thanks. I was mostly confused since I wasn't sure how to read the TODO. It seemed to be implying that we want to change the status eventually and start allowing generated files.

Copy link
Member Author

@dcbaker dcbaker Nov 3, 2025

Choose a reason for hiding this comment

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

I wrote the TODO, and originally I thought this was buggy behavior, but then realized this field is for files being grouped in an IDE, which I would think generally people don't want to edit generated files so this actually makes sense.

if not os.path.isfile(trial):
raise InvalidArguments(f'Tried to add non-existing extra file {i}.')
self.extra_files.append(i)
self.extra_files = kwargs.get('extra_files', [])
self.install_rpath: str = kwargs.get('install_rpath', '')
self.build_rpath = kwargs.get('build_rpath', '')
self.resources = kwargs.get('resources', [])
Expand Down
3 changes: 2 additions & 1 deletion mesonbuild/interpreter/interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3384,7 +3384,8 @@ def build_target(self, node: mparser.BaseNode, args: T.Tuple[str, SourcesVarargs
sources = self.source_strings_to_files(sources)
objs = kwargs['objects']
kwargs['dependencies'] = extract_as_list(kwargs, 'dependencies')
kwargs['extra_files'] = self.source_strings_to_files(kwargs['extra_files'])
# TODO: When we can do strings -> Files in the typed_kwargs validator, do this there too
kwargs['extra_files'] = mesonlib.unique_list(self.source_strings_to_files(kwargs['extra_files']))
self.check_sources_exist(os.path.join(self.source_root, self.subdir), sources)
if targetclass not in {build.Executable, build.SharedLibrary, build.SharedModule, build.StaticLibrary, build.Jar}:
mlog.debug('Unknown target type:', str(targetclass))
Expand Down
15 changes: 14 additions & 1 deletion mesonbuild/interpreter/type_checking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,24 @@ def _target_install_convertor(val: object) -> bool:
return bool(val)


def _extra_files_validator(args: T.List[T.Union[File, str]]) -> T.Optional[str]:
generated = [a for a in args if isinstance(a, File) and a.is_built]
if generated:
return 'extra_files contains generated files: {}'.format(', '.join(f"{f.fname}" for f in generated))
return None


# Applies to all build_target like classes
_ALL_TARGET_KWS: T.List[KwargInfo] = [
OVERRIDE_OPTIONS_KW,
KwargInfo('build_by_default', bool, default=True, since='0.38.0'),
KwargInfo('extra_files', ContainerTypeInfo(list, (str, File)), default=[], listify=True),
KwargInfo(
'extra_files',
ContainerTypeInfo(list, (str, File)),
default=[],
listify=True,
validator=_extra_files_validator,
),
KwargInfo(
'install',
object,
Expand Down
Loading