mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.59k stars 1.63k forks source link

foo.vapi files generated by Vala library() should be installed #891

Closed nirbheek closed 7 years ago

nirbheek commented 8 years ago

Not sure if we want to put this behind an option, but I vote that we just install it unconditionally for now. Can add an option later if people ask for it.

Found while investigating #890

arteymix commented 8 years ago

The best way to install them is via install_data.

I just do:

library('foo', 'foo.vala')
install_data(meson.current_build_dir() + '/foo.vapi', install_dir: 'share/vala/vapi')

I also think we should install GIRs, headers and other files this way.

We should update the Wiki about this.

arteymix commented 8 years ago

I think it's dangerous to start adding install_<output> options when we can reuse existing keywords. The generate_gir is a good example of that: it should have been two separate functions instead.

arteymix commented 8 years ago

Here what I suggest for the Wiki (section Installing VAPI and GIR files):


To install generated VAPI and GIR files, one can use install_data and refer to the files relative to the build directory:

install_data(meson.current_build_dir() + '/foo.vapi', 
             install_dir: 'share/vala/vapi')
install_data(meson.current_build_dir() + '/foo@sha/Foo-1.0.gir', 
             install_dir: 'share/gir-1.0')

The foo@sha is only a temporary hack until we get proper options to make them land along in the build tree.

nirbheek commented 8 years ago

No, this install_data thing is a hack and you're not supposed to do it like this at all. install_data is only for pre-existing source files, which is why it only takes string arguments. Targets are supposed to install everything themselves.

install_data should actually be checking that the files listed are available at configure time. The fact that it does not is a bug. We won't change this because all Vala projects seem to rely on this, but we should not make it the official way of installing generated files.

arteymix commented 8 years ago

I still think that install_data is a far more superior way of installing things than doing everything in the target. Maybe we should have a install_build_data instead? or a from_build option?

nirbheek commented 8 years ago

How would you add a dependency between the target that builds the file and the install rule that installs it?

arteymix commented 8 years ago

Two target cannot build the same file, so there's a unique target that produce the file we want to install.

It's either implicit (given that we've stated all the output already) or via a depends parameter.

arteymix commented 8 years ago

I don't think installing VAPI by default is a good idea because there's many situation where you don't wait it installed:

I would agree to have this behind install_vala_vapi and install_vala_vapi_dir options, although it's a bit verbose and redundant when we could simply use install_data for anything that goes into share/*.

nirbheek commented 8 years ago

Sure, let's not install vapi by default. That makes sense.

install_data is not for generated files, and adding that would lead to an inconsistency between how all other generated files are installed. It's also a bad idea to add two different ways of installing generated files since then we have to check if the user is using both methods and only create one target, or error out and tell them. Overall very un-nice.

install_vala_vapi and friends is also sub-optimal. We should think of a more concise way of specifying directories to install secondary target outputs to.

nirbheek commented 8 years ago

Also, how do you tell install_data what to install where when you pass a target with multiple outputs? I think in the end install directives are best specified as part of targets.

arteymix commented 8 years ago

@nirbheek I actually just pass the output

That's why I would love to see something like:

library('foo', 'foo.vala', vala_vapi: 'foo.vapi')

install_data('foo.vapi', , install_dir: 'share/vapi', from_build: true)

Jussi suggested something like out for target:

foo_lib = library('foo', 'foo.vala', vala_vapi: 'foo.vapi')

install_data(foo_lib.out('foo.vapi'), install_dir: 'share/vapi')

The returned file from out should be a File object. We might need to support files from the build directory. Also, we could determine at configure-time if the file will exist. That will need #858.

We already have install_dir_gir and install_dir_typelib for generate_gir, but I really dislike it. Maybe we could deprecate that and replace it by a unique API?

I would keep primary output installable by the target though and we should always prefer tools that produce a single output to make them composable (relate somehow to currying).

nirbheek commented 8 years ago

Hmm, I like the foo_lib.out syntax because it allows us to setup dependencies properly. It would also allow people to, for instance, pass it to custom_targets or run_targets.

You've changed my mind; install_data is not a bad option in this form, I think.

arteymix commented 8 years ago

In the end, we will support installing pretty much anything without additional arguments, as long as it is a well-defined target output.

You should also ensure that install_headers, install_man and install_subdir work as well with File objects ;)

nirbheek commented 8 years ago

I disagree. I think it's fine to install secondary outputs via install_data, but primary outputs should use their own kwargs. Let's have just one way of installing things.

arteymix commented 8 years ago

Ok, but how will you prevent me from doing install_data(foo_lib.out('libfoo.so'))?

Maybe we should name out differently to refer exclusively to secondary output?

nirbheek commented 8 years ago

I think if people do that ,we should just error out telling them to use the target itself.

nirbheek commented 8 years ago

(and then the target itself won't work in install_data, and we should use a better error message for that telling them why)

nirbheek commented 7 years ago

I think the foo_lib.out syntax should not require the full name if just specifying the extension is unambiguous, such as:

mylib = shared_library('mylib', 'test.vala')
install_data(mylib.out('.vapi'))

I thought about allowing globs for this, but that's a slipper slope that we should avoid unless there's a compelling use-case.

arteymix commented 7 years ago

Wouldn't adding extra function to the build target object be nicer? Having something like language-dependant accessors to get specific compiler output.

The out function wouldn't work consistently if the name is platform-dependant.

mylib = shared_library('foo', 'test.vala')
install_data(mylib.vala_vapi(), install_dir: 'share/vala/vapi')
install_data(mylib.vala_gir(), install_dir: 'share/gir-1.0')
nirbheek commented 7 years ago

You may be right. For instance, the import library on Windows is compiler-dependent. With MinGW/GCC it's .dll.a and with MSVC it's .lib. Debug files are also compiler-specific. We don't splitdebug at all with GCC/Clang and MSVC always outputs separate .pdb files.

nirbheek commented 7 years ago

On the other hand, we should also think about how this squares with custom_target output and what is some good syntax to use there.

benwaffle commented 7 years ago

Should things like library() really have multiple outputs? How about vapi = gnome.vapi(library, install: true) and gir = gnome.generate_gir(library, install: true). Or library() could return an array of multiple outputs