-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Filesystem significant languages and build directory implementation details #8725
Comments
Copied from IRC:
|
I did run into this with Cython earlier. Was not happy. |
In Rust+Cargo, they usually work around this by having Cargo set the mod generated {
include!(concat!(env!("OUT_DIR"), "/generated.rs"));
} (or even #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
mod generated; |
Ah, yes. There's another problem with that. Ninja doesn't support environment variables by design, so to handle that we would have to either implement a command line switch for rustc to override the |
That's just |
Not on Windows it isn't... |
How do existing systems handle this then? Do they mandate that all generated source files must be put in the source directory instead of a build directory? |
Cargo, AFAICT, doesn't handle this case at all. They rely on tricks in the source code with env vars (either using the |
Wouldn't it be cleanest to add a flag to rustc that would cause it to search for modules under multiple directories, much like include directories work in C-based languages? Perhaps like this:
Then upon seeing #[path = "foo/bar.rs"]
mod meh; rustc would search both at |
@dcbaker the term you are looking for in Java-land is https://docs.oracle.com/javase/7/docs/technotes/tools/windows/classpath.html Unfortunately I think this can be solved by maybe being able to teach |
OR maybe classpath isn't even the right approach to solving this problem in Java. The amount of hassle that I just described seems to tell me it isn't the best approach or would even work to begin with. If |
@bugaevc For Rust that is definitely an option, though we'd need to be able to pass it multiple times, |
Ok I just got done doing some testing with javac. We should get this feature for free in Java.
package com.micron.hse;
import com.micron.hse.MyGenerated;
public class MyStatic {
public static void main(String[] args) {
System.out.println(MyGenerated.hello());
}
}
package com.micron.hse;
public class MyGenerated {
public static String hello() { return "hello"; }
} Looks like javac -d . com/micron/hse/MyStatic.java build/com/micron/hse/MyGenerated.java ^ That works fine. If done correctly you should see 2 |
So it's really just Cython and Rust that this affects. |
and possibly zig |
I think for Cython the situation is a little worse than I expected. It relies not only on files in the same directory, but walks all the way up the package hierarchy. An example taken from SciPy:
The most relevant part of the _decomp_update_pyx = custom_target('_decomp_update',
output : '_decomp_update.pyx',
input : '_decomp_update.pyx.in')
# Extension relies on the generated cython_blas.pxd too
py3.extension_module('_decomp_update', _decomp_update_pyx, dependencies : py3_dep,
install : true, subdir : 'scipy/linalg') So there's a As a hack, I tried copying all the files that are needed to the build dir, like so: # Needed to trick Cython, it won't do a relative import outside a package
_dummy_init = custom_target('_dummy_init',
output : '__init__.py',
input : '__init__.py',
command : ['cp', '@INPUT@', '@OUTDIR@']) Then the build succeeds. If I do not copy #define __PYX_HAVE__scipy__linalg___decomp_update silently changes to #define __PYX_HAVE__linalg___decomp_update and we get an incomprehensible error at import time (it'll look like So it can be made to work, but it's pretty hacky even with the "structured sources" idea in gh-8775. |
Out of curiousity, why would something like: _decomp_update_pyx = custom_target('_decomp_update',
output : '_decomp_update.pyx',
input : '_decomp_update.pyx.in')
py.extension_module(
'foo',
{
'': ['__init__.py', 'lina1g.pyx',],
'lina1g': ['__init__.py', '_generate_cy.py', _decomp_update_pyx],
}
) Work? For Cython I suspect we need to set the --workdir= flag to point to the builddir, but otherwise that seems not too bad, unless I'm missing something? |
Maybe that's not too bad indeed. The first part should then be: _decomp_update_pyx = custom_target('_decomp_update',
output : 'linalg/_decomp_update.pyx',
input : 'linalg/_decomp_update.pyx.in') # or some variant (is `/` portable?) It seems like this code is then in the wrong The alternative I thought of was to have one |
Yeah, the idea behind the structured inputs was that you could just describe the layout to Meson and it would ensure that it exists. It actually uses (many) custom targets behind the scene to do the copies if you need them. Also, yes, |
@dcbaker can this be closed since we now have |
I think so. I guess we never got this implemented for cython though |
Do envision structured source for pymod.extension_module()? |
Yeah. If you’re generating pyx files you need to structure the sources appropriately |
There's two reasons for having to copy files to the build dir for Cython:
To get rid of (2) there's now For (1), |
|
Feels like this should be solved by something like |
You'd hope, but I don't think that that will work - there is such a thing as |
It's not ideal to restrict which valid code you're allowed to use "because build systems", but I guess using absolute rather than relative cimports could be a low-effort way to solve that for situations where out of tree builds are done? That would have the benefit of avoiding otherwise unneeded file copying. The alternative would be... I dunno, a special version of |
Perhaps - it would need testing whether absolute imports work in both situations (generated |
Meson has a strict rule that the layout of the build directory as an implementation detail, this makes a lot of sense as it gives us (Meson) flexibility to change the layout when it makes sense (we've done this a number of times, including with things like shortening the names of generated directories, or adding the private dir). This is also completely fine for languages like C in which the filesystem layout is an implementation detail of the language, and can easily be fixed with
include_directories()
. There are however, a growing number of languages supported by Meson (Rust, Python), or with pending support in Meson (Cython, Zig), in which the filesystem layout is the import layout. This introduces real problems when trying to mix generated and static sources, as the compilers generally expect a single, unified tree.I'd like to propose then a way to provide sources as a tree, but in an abstract way such that Meson can make configure time decisions about what it wants to do to fulfill these requirements. Specifically, I'm proposing:
This specifically would tell Meson that when compiling a file main.rs, it must be in a directory such that there is a subdir foo, which contains the output of
generated_rs
(whatever that happens to be), and foo must have a subdir bar, withother.rs
. Meson could at configure time decide that all of the inputs are static (non-generated) files, and do nothing, or it could decide that sincegenerated_rs
is acustom_target
, to copy/link all of the files into a new directory and compile them there.This solves the problem that Rust, Zig, and Cython have of needing a single tree of filres, but also creates an abstraction such that Meson can continue to hide the actual layout of the build dir, as this tree could exist anywhere within the build dir.
The text was updated successfully, but these errors were encountered: