Fix use of a .pxi Cython include file as source from configure_file

[rgommers]

Without adding .pxi as a known header suffix, the added test will fail with:

No specified compiler can handle file stuff.pxi

Technically only .pxd are header files, and .pxi are "include files" which are literally included in .pyx files. Adding them as headers seems to be fine though, since they're kinda similar and the point is to avoid treating them as source files.

[eli-schwartz]

Huh, I thought this was fixed in commit 9b999dd.

[rgommers]

Huh, I thought this was fixed in commit 9b999dd.

That fixed the problem for custom_target only. What that did was correctly adding order dependencies for .pxi/.pxd rather than silently dropping those files.

The problem we're hitting now happens earlier, and is not about dropping order dependencies but plain choking on configure_file(..., output: 'somefile.pxi'). This is the problematic code:

meson/mesonbuild/build.py

Lines 811 to 827 in b3b5734

	def process_sourcelist(self, sources: T.List['SourceOutputs']) -> None:
	"""Split sources into generated and static sources.
	Sources can be:
	1. Pre-existing source files in the source tree (static)
	2. Pre-existing sources generated by configure_file in the build tree.
	(static as they are only regenerated if meson itself is regenerated)
	3. Sources files generated by another target or a Generator (generated)
	"""
	added_sources: T.Set[File] = set() # If the same source is defined multiple times, use it only once.
	for s in sources:
	if isinstance(s, File):
	if s not in added_sources:
	self.sources.append(s)
	added_sources.add(s)
	elif isinstance(s, (CustomTarget, CustomTargetIndex, GeneratedList)):
	self.generated.append(s)

As you see from the comment, configure_file is treated as a source and has type File, so uses a different code path than CustomTarget even though they're both generated files. Hence later we hit the error path in

meson/mesonbuild/utils/universal.py

Lines 460 to 465 in b3b5734

	def get_compiler_for_source(compilers: T.Iterable['Compiler'], src: 'FileOrString') -> 'Compiler':
	"""Given a set of compilers and a source, find the compiler for that source type."""
	for comp in compilers:
	if comp.can_compile(src):
	return comp
	raise MesonException(f'No specified compiler can handle file {src!s}')

because .pxi is an unknown file type.

The assumption made that a configure_file output is a source file that the compiler must know about is, in general, not always going to be correct. I think the structural fix is your suggestion in gh-8515, adding depends/depend_file. I already know that .pxi is only an order dependency and my code in meson.build would actually be clearer if I could say:

py.extension_module(...
    'a_source.pyx',
    depends: generated_pxi,
)

rather than passing everything to sources.

[eli-schwartz]

I already know that .pxi is only an order dependency

In fact, you are wrong there. If it comes from a configure_file then it is created before calling ninja. What meson does in this case, at least for *.h files, is treat it like extra_files: "Not used for the build itself but are shown as source files in IDEs that group files by targets (such as Visual Studio)".

You don't need to depend on it as an order dependency, because it can't ever not exist for the first transpilation / compilation. It's still valuable to list as a depfile dependency for incremental rebuilds.

So the obvious immediate solution is to "just stop doing that" and the build will work. That means the nature of this issue shifts in my mind from "get cython transpile to work" to "figure out why this UX is the way it is, and why we distinguish between generated sources and static sources".

What is the general purpose of this error message? *.inc for example isn't listed as a type of header file, and we have a comment somewhere that talks about how we assume any unknown file extensions are header-style dependencies for exactly this general reason. People are very attached to the idea of their C/C++ files having "quirky" names that "indicate their purpose" when that purpose isn't strictly shared API function prototypes. Do we allow .inc files when they are generated but forbid them when they are static sources? If so, why, and was that an intentional difference?

[rgommers]

Thanks for pointing that out. Yes, if it's generated before meson is invoked and we're using Cython's depfile support for .pxi, then there should not be a race condition.

People are very attached to the idea of their C/C++ files having "quirky" names that "indicate their purpose" when that purpose isn't strictly shared API function prototypes. Do we allow .inc files when they are generated but forbid them when they are static sources? If so, why, and was that an intentional difference?

Yeah, the UX is confusing. The extra_files thing may make sense here also for .inc files? I had ignored extra_files because its description is confusing, while depends seemed right. depends is also still needed for anything that doesn't have depfile support I think.

Why the difference: just a guess, but maybe the only reason to pass non-sources as sources is if you have a generated target containing a mix of sources and headers? For static non-sources there should never be a need to pass them as sources. That said, I would not qualify configure_file output as static - it's generated, just earlier that custom_target.

[rgommers]

This fix is still helpful I think, and correct? Maybe extra_files is better, but this just removes a confusing source of error without a downside that I can think of.

[tristan957]

Is this worth getting into 1.2.0? Otherwise I can target it to 1.3.0. Seems like the previous conversation was never resolved

[rgommers]

Thanks for checking in on this @tristan957. I think this is mergeable as is and quite safe, but it's not that urgent to me since it's not blocking for anything. We're already at 1.2.0rc3 now, so targeting this for 1.3.0 seems fine.

[tristan957]

Cool. I think that is what I did already (on mobile right now). Maybe it is worth rebasing for the CI?