-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Extend native files to store install path information #4743
Conversation
7d48599
to
ed917e3
Compare
I agree this should go in the file, but I am hesitant to say that should be the native file. This is not a property of arbitrary machines (build, host, and target); it is as best a property that tends to vary with the host machine. |
As I mentioned, I'd like to allow these paths to be specified in both the native file (the build machine config files, if you will) as well as the cross file (the host and target machine config files). for cross files though I need to tinker with some assumptions the backend has, namely that there will only be one prefix. |
ed917e3
to
61fa325
Compare
And I do think it really is a property of the build machine. Take for example libdir (assume x86 and linux), Even if you're doing a "native" build for a different distro you might need a different libdir. I think that the cross file version is probably more interesting from an end user point of view, but not having 3 different files that a user needs to copy to replicate a build is also a feature. |
How is there multiple prefixes with a cross build? There's DESTDIR-like functionality---I'm "installing" it on this machine but I'll actually have to copy the files elsewhere before I use it---but that's different. In general, the build machine is purely an implementation detail, all interface details should stem from the host machine and target machine. [A prefix in the target machine would be where the newly-build compiler would install things by default, but that doesn't really make much sense.] |
In short, the host machine is the one on which software runs, the install directory is where the problem runs. Because of the common thread of "program runs", I deem the install dir a host machine property. |
Take this super trivial case: executable('foo', 'foo.cpp', native : true, install : true)
executable('cross_foo', 'foo.cpp', native : false, install : true) Currently foo and cross_foo would be installed into the same prefix, even though one is compiled for the build machine and the other for the host machine. This is broken IMHO, which leaves us with two choices:
Thinking about it more 2 is probably a better solution, which actually means I can just add my patches to add this to the cross file as well. |
If we want to limit ourselves 2-3 files ("native and cross" or "file per machine") and not more, I'd permit the |
Yeah, here's how I'm leaning: such that in a cross build the paths section from the native file is completely ignored |
Yeah I'm not sure that should be true for all CLI flags, but that the right way for this. |
61fa325
to
46bf539
Compare
The unix tradition is that command line > config, and I've been going with that. I think that will surprise people less. |
Oh yes I agree with the direction of the |
Right, what I'm suggesting is in a case like: [paths]
prefix = '/usr/x86root'
libdir = 'lib32' and the command line that prefix would be /usr/x86root, and the libdir would be lib, command line arguments in unix tradition override config file variables. |
083b582
to
6398ab6
Compare
ping |
Instead of "paths" should this be "options" so you could provide a value for any option, not just paths? |
Well, I'm not sure what the right thing to do is here. In cross files we store some options as properties, like c_args. But you're probably right, it probably makes sense to store these in an |
#4626 makes
|
@jpakkane, preferences here, I'd like to get this merged and I'm happy to change the name to whatever you like. |
adba5bf
to
a3b78b3
Compare
The file must be called nativefile.txt
Let's go with the current |
mesonbuild/environment.py
Outdated
# Store paths for native and cross build files. There is no target | ||
# machine information here because nothing is installed for the target | ||
# architecture, just the build and host architectures | ||
self.paths = PerMachine(Directories(), Directories(), None) |
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.
Well by that logic this shouldn't be a PerMachine
because nothing is ever installed for the build platform by definition. (It is the host platform, and when build == host, it is because of the host platform, not because of the build platform aliasing it.)
All things target-platform related are dead code in Meson today, btw, but if/when Meson gets used for some compiler, it may be nice to specify per-target paths separately. (Consider multilib GCC or LLVM and their /lib/compiler-config
directories.)
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.
If you wish to keep PerMachine
for future target support, use PerMachineDefaultable
so that you can default paths.host
when no cross file is used, to reflect that the host and build configurations are the same.
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.
Sure, but the nice thing about storing it in a PerMachine is that we don't have to special case the cross/native reading machinery. We just load the cross and the native files and then use the right one.
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.
Also, that's not necessarily true. LLVM can build llvm-config for the build system, even when libLLVM.so is compiled for the host system. So it is possible that you could need both.
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.
Hehe, *-config
and friends are indeed an exception to my rule. Fair point, but I hope people don't do use this for that and instead switch to pkg-config. xxx-config
has caused us using my distro a ton of pain (NixOS/nixpkgs#51176)!
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.
Sure -config has caused me no end of pain. Unfortunately no one asks me :)
060afce
to
970f7d7
Compare
970f7d7
to
9c48201
Compare
@Ericson2314 For the moment I've just used the PerMachineDefaultable as-is. I'm not entirely happy with it, but that is life. |
Oh a last thing for this PR, which otherwise looks great. Instead of trying to detect the default and override conditionally, can we always set the options based on the user config before we slurp in CLI flags? See https://github.com/mesonbuild/meson/pull/4626/files#diff-246a2f6d498375c5981f0ca48d99cae2R585 for where I did this with the compiler options |
9c48201
to
4c057ce
Compare
This allows the person running configure (either a developer, user, or distro maintainer) to keep a configuration of where various kinds of files should end up.
Just like the previous patch, but for cross files Fixes mesonbuild#1433
Mypy struggles with the imperative form of Enum declaration, and upstream doesn't consider it a bug, they recomend using the class form for enums that are going to be externally exposed.
4c057ce
to
0eccce7
Compare
@Ericson2314, that was no small ask :) I do think that the result is nicer (no more tracking of default arguments), but it was basically a complete rewrite of the series. Hopefully this version is ready. |
@dcbaker All the more thanks then! I when through it one last time, commit by commit, and it looks good. |
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.
Great work!
@jpakkane how do you feel about this? |
This series is the start of fixing #4637, though it's not complete. Currently this allows setting builtin path options in the native and cross files. These values will be replaced by command line arguments.
When doing a native (non-cross) build, then the values from the native file will be used, when doing a cross build the values from the cross file will be used. values from the native file will never be used in a cross build, and vice versa. In keeping with the unix tradition, command line arguments overwrite any config file values.