Skip to content
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

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

Closed
nirbheek opened this issue Oct 11, 2016 · 22 comments
Closed

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

nirbheek opened this issue Oct 11, 2016 · 22 comments
Assignees
Milestone

Comments

@nirbheek
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor

arteymix commented Oct 14, 2016

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

arteymix commented Oct 14, 2016

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
Copy link
Member Author

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

@arteymix
Copy link
Contributor

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
Copy link
Contributor

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

  • if you write a GModule-based shared library
  • if you don't target Vala (just header + shared library installed and maybe GIR)

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

arteymix commented Oct 26, 2016

@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
Copy link
Member Author

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.

@nirbheek nirbheek self-assigned this Oct 26, 2016
@arteymix
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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

@nirbheek
Copy link
Member Author

(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 nirbheek modified the milestones: 0.36.0, 0.37.0 Nov 2, 2016
@nirbheek
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Member Author

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.

@nirbheek nirbheek modified the milestones: 0.38.0, 0.37.0 Dec 18, 2016
@benwaffle
Copy link
Contributor

benwaffle commented Dec 31, 2016

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

@nirbheek nirbheek modified the milestones: 0.38.0, 0.39.0 Feb 1, 2017
@nirbheek nirbheek modified the milestones: 0.39.0, 0.40.0 Mar 9, 2017
gnomesysadmins pushed a commit to GNOME/tracker that referenced this issue Mar 30, 2017
This is done in a horrendously hacky way for the time being. We really
need accessor functions for target outputs to do it more cleanly, so
we could call .vala_gir() on the libtracker_sparql target object and
get the actual GIR output.

See mesonbuild/meson#891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants