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

Support multiple install dirs for built/custom targets #1469

Merged
merged 4 commits into from Apr 9, 2017

Conversation

Projects
None yet
4 participants
@nirbheek
Copy link
Member

commented Mar 14, 2017

You can now pass a list of strings to the install_dir: kwarg to build_target and custom_target.

Custom Targets:

Allows you to specify the installation directory for each corresponding output. For example:

custom_target('different-install-dirs',
  output : ['first.file', 'second.file'],
  ...
  install : true,
  install_dir : ['somedir', 'otherdir])

This would install first.file to somedir and second.file to otherdir.

If only one install_dir is provided, all outputs are installed there (same behaviour as before).

To only install some outputs, pass false for the outputs that you don't want installed. For example:

custom_target('only-install-second',
  output : ['first.file', 'second.file'],
  ...
  install : true,
  install_dir : [false, 'otherdir])

This would install second.file to otherdir and not install first.file.

Build Targets:

With build_target() (which includes executable(), library(), etc), usually there is only one primary output. However some types of targets have multiple outputs.

For example, while generating Vala libraries, valac also generates a header and a .vapi file both of which often need to be installed. This allows you to specify installation directories for those too.

# This will only install the library (same as before)
shared_library('somevalalib', 'somesource.vala',
  ...
  install : true)

# This will install the library, the header, and the vapi into the
# respective directories
shared_library('somevalalib', 'somesource.vala',
  ...
  install : true,
  install_dir : ['libdir', 'incdir', 'vapidir'])

# This will install the library into the default libdir and
# everything else into the specified directories
shared_library('somevalalib', 'somesource.vala',
  ...
  install : true,
  install_dir : [true, 'incdir', 'vapidir'])

# This will NOT install the library, and will install everything
# else into the specified directories
shared_library('somevalalib', 'somesource.vala',
  ...
  install : true,
  install_dir : [false, 'incdir', 'vapidir'])

true/false can also be used for secondary outputs in the same way.

Includes tests for all these.

Closes #705
Closes #891
Closes #892
Closes #1178
Closes #1193

@nirbheek nirbheek force-pushed the centricular:install-secondary-outputs branch 2 times, most recently from 2c6365e to 2d98fc6 Mar 14, 2017

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2017

@ssssam @arteymix @TingPing, I believe this is relevant to your interests. Comments are welcome.

@nirbheek nirbheek force-pushed the centricular:install-secondary-outputs branch from 2d98fc6 to 4b486c6 Mar 14, 2017

@ssssam

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2017

Looks good, although would it be possible to use a dict instead? Something like:

install: {
  binary: false,
  vapi: vapidir,
  include: includedir
}

I think that'd be a lot more intuitive to read, although maybe something to put on the todo list for Meson 2.0 ? ;-)

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2017

We don't have dictionaries in meson build files. :)

However, if we do add dictionaries, it would be easy to extend install_dir: to also accept dictionaries instead of lists.

@arteymix

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2017

@nirbheek It looks good to me. One thing though: how does that cover the GIR output?

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2017

Good point, I forgot about vala_gir:. Will add that too.

@nirbheek nirbheek force-pushed the centricular:install-secondary-outputs branch from 4b486c6 to 2e702ef Mar 16, 2017

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2017

Support for that was already there; I've just added an explanation for how to use it to the commit message. The same would also go into the documentation:

Valac can also generate a GIR file for libraries when the `vala_gir:`
keyword argument is passed to library(). In that case, `install_dir:`
must be given a list with four elements, one for each output.
@arteymix

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2017

Of course as @ssssam already said, a dictionary would libe much more proper. However, I really like that build targets are treated consistently with custom targets.

Maybe it would be better to have an array of boolean for install and an array of string for install_dir to override default install dirs with true only to signify the default directory. This way, we don't have to prevent installation using install_dir:

library(
    install: [true, true, true, true],
    install_dir: [true, 'include', 'share/vala/vapi', 'share/gir-1.0'])
@arteymix

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2017

Seeing this, install and install_dir could essentialy be merged:

  • true to install in default directory
  • false to prevent installation
  • with a string to signify install in specific directory, which is implicitly true

I would have install and install_dir for single output installation and a install list for multiple output.

Here's the simplified version:

library(
    install: [true, 'include', 'share/vala/vapi', 'share/gir-1.0'])
  • install the library in default path
  • install the generated header, vapi and GIR in specific directories
@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2017

Yes, I thought of that too, but let's not do that (yet) since it's:

a) Unnecessary for implementing this feature
b) Complicates the code since we have to maintain backwards compat
c) Breaks the existing workflow of toggling the install: kwarg to enable/disable installation of an entire target which can be done via an option, for instance, with no simple replacement

@arteymix

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2017

@nirbheek I agree on "c". The semantic we give to install would be to install the target as a whole, and to install_dir would be to tell where each output goes (or dosen't go).

I'm convinced!

@nirbheek nirbheek force-pushed the centricular:install-secondary-outputs branch from ca95691 to 0bb0258 Mar 27, 2017

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2017

Branch rebased, conflicts fixed.

gnomesysadmins pushed a commit to GNOME/tracker that referenced this pull request Mar 30, 2017

Meson build instructions for Tracker
See <http://mesonbuild.com/> for information about Meson.

Remaining issues:

  * There's no `make dist` equivalent. We currently produce release
    tarballs containing the output files of the Vala compiler. We need
    to think through whether we can stop doing that. Shipping the
    generated .c files does make the Vala preprocessor useless so
    it would be good if we can stop.

  * The Firefox, Thunderbird, Evolution and Nautilus plugins are not
    built

  * mesonbuild/meson#671 -- means we can't
    depend on tracker_common_dep in most places and have to manually
    set link_with, include_directories and sources instead.

  * mesonbuild/meson#1469 -- without this we
    have to install generated Vala headers using a script

  * mesonbuild/meson#1229 -- means adding
    the #include guard to libtracker-sparql/tracker-generated-no-checks.h
    is a pain

Here's a rough speed comparison.

Meson:
        time sh -c 'meson .. --prefix=/opt/tracker-meson -D bash_completion=/opt/tracker-meson/share/bash-completion -Dsystemd_user_services=/opt/tracker-meson/lib/systemd/user -Ddbus_services=/opt/tracker-meson/share/dbus-1/services && ninja-build -j 4 && ninja-build install'

        real  1m8.194s
        user  2m16.962s
        sys   0m20.532s

Autotools:
        time sh -c './configure --prefix=/opt/tracker-autotools --with-bash-completion-dir=/opt/tracker-autotools/share/bash-completion --with-session-bus-services-dir=/opt/tracker-autotools/share/dbus-1/services --disable-nautilus-extension && make -j 4 && make install'

        real  2m37.750s
        user  4m37.214s
        sys   0m54.806s

        Plus 30+ seconds of ./autogen.sh first.

Note that Meson builds may fail if your source tree has generated files
from an Autotools build in there. If you see errors about duplicate
definitions, first try cleaning your source tree (use `git clean -dfx`,
but make sure you commit any work first!!)

nirbheek added some commits Mar 12, 2017

Support multiple install dirs for built/custom targets
You can now pass a list of strings to the install_dir: kwarg to
build_target and custom_target.

Custom Targets:
===============
Allows you to specify the installation directory for each
corresponding output. For example:

    custom_target('different-install-dirs',
      output : ['first.file', 'second.file'],
      ...
      install : true,
      install_dir : ['somedir', 'otherdir])

This would install first.file to somedir and second.file to otherdir.

If only one install_dir is provided, all outputs are installed there
(same behaviour as before).

To only install some outputs, pass `false` for the outputs that you
don't want installed. For example:

    custom_target('only-install-second',
      output : ['first.file', 'second.file'],
      ...
      install : true,
      install_dir : [false, 'otherdir])

This would install second.file to otherdir and not install first.file.

Build Targets:
==============
With build_target() (which includes executable(), library(), etc),
usually there is only one primary output. However some types of
targets have multiple outputs.

For example, while generating Vala libraries, valac also generates
a header and a .vapi file both of which often need to be installed.
This allows you to specify installation directories for those too.

    # This will only install the library (same as before)
    shared_library('somevalalib', 'somesource.vala',
      ...
      install : true)

    # This will install the library, the header, and the vapi into the
    # respective directories
    shared_library('somevalalib', 'somesource.vala',
      ...
      install : true,
      install_dir : ['libdir', 'incdir', 'vapidir'])

    # This will install the library into the default libdir and
    # everything else into the specified directories
    shared_library('somevalalib', 'somesource.vala',
      ...
      install : true,
      install_dir : [true, 'incdir', 'vapidir'])

    # This will NOT install the library, and will install everything
    # else into the specified directories
    shared_library('somevalalib', 'somesource.vala',
      ...
      install : true,
      install_dir : [false, 'incdir', 'vapidir'])

true/false can also be used for secondary outputs in the same way.

Valac can also generate a GIR file for libraries when the `vala_gir:`
keyword argument is passed to library(). In that case, `install_dir:`
must be given a list with four elements, one for each output.

Includes tests for all these.

Closes #705
Closes #891
Closes #892
Closes #1178
Closes #1193
Fix custom directory installation of import library
When install_dir was set for a shared_library, the import library
would not be installed at all, which is unintended.

Instead, install it into the custom directory if it is set, otherwise
install it in the default import library installation directory.

Includes a test for this.
Don't generate import library for shared modules
Also add a test for this on all platforms.
Reduce an indent level in the install for loop
if not t.should_install(): continue

No logic changes at all.

Also improve the message when erroring out about insufficient
install_dir: list elements.

@nirbheek nirbheek force-pushed the centricular:install-secondary-outputs branch from 0bb0258 to aa3480d Apr 4, 2017

@jpakkane

This comment has been minimized.

Copy link
Member

commented Apr 4, 2017

Looks good. The only thing I'm wondering is what should happen in the case that one produces many extra files (lib, import lib, vapi file, something else etc)? The order in which they are put on the list is currently unspecified (though it should be stable).

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2017

I wasn't sure what to do about import libraries. They are platform-specific, and it doesn't make sense to force people using this on Linux to specify an installation directory for them. This PR makes it so that if the DLL is installed into a custom directory (i.e., not bindir), the import library is installed alongside it. I think this is what most people will expect.

For vala targets, we require [libdir, incdir, vapidir] (in that order) if they want headers and vapis installed. For backwards-compat, if you don't specify incdir and vapidir we assume you don't want to install them.

@jpakkane jpakkane merged commit 1652dcc into mesonbuild:master Apr 9, 2017

3 checks passed

ci/sideci Meow! No issues found. It's clean code!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nirbheek nirbheek deleted the centricular:install-secondary-outputs branch Apr 9, 2017

@arteymix

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2017

@nirbheek

Just got this error:

Traceback (most recent call last):
  File "/home/guillaume/.local/bin/meson", line 4, in <module>
    __import__('pkg_resources').run_script('meson==0.40.0.dev1', 'meson')
  File "/home/guillaume/.local/lib/python3.6/site-packages/pkg_resources/__init__.py", line 738, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/home/guillaume/.local/lib/python3.6/site-packages/pkg_resources/__init__.py", line 1499, in run_script
    exec(code, namespace, namespace)
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/EGG-INFO/scripts/meson", line 37, in <module>
    sys.exit(main())
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/EGG-INFO/scripts/meson", line 34, in main
    return mesonmain.run(launcher, sys.argv[1:])
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/mesonmain.py", line 260, in run
    sys.exit(run_script_command(args[1:]))
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/mesonmain.py", line 248, in run_script_command
    return cmdfunc(cmdargs)
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/scripts/meson_install.py", line 306, in run
    do_install(datafilename)
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/scripts/meson_install.py", line 125, in do_install
    install_targets(d)
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/scripts/meson_install.py", line 237, in install_targets
    outdir = get_destdir_path(d, t[1])
  File "/home/guillaume/.local/lib/python3.6/site-packages/meson-0.40.0.dev1-py3.6.egg/mesonbuild/scripts/meson_install.py", line 112, in get_destdir_path
    if os.path.isabs(path):
  File "/usr/lib64/python3.6/posixpath.py", line 64, in isabs
    s = os.fspath(s)
TypeError: expected str, bytes or os.PathLike object, not bool
@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2017

You need to wipe your builddir. The format for install scripts data changed and since the version is the same (0.40.0dev1), meson doesn't know that you upgraded and can't warn you.

@arteymix

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2017

It's a bug, really, you can't pass true to the last items in the array (include dir, vapi dir and gir dir).

@arteymix

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2017

If there is no default directory, at least we should have an explanatory error.

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2017

Hmm, I see what you mean.

nirbheek added a commit to centricular/meson that referenced this pull request Apr 9, 2017

@nirbheek

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2017

@arteymix can you check #1599 please?

nirbheek added a commit to centricular/meson that referenced this pull request Apr 10, 2017

nirbheek added a commit to centricular/meson that referenced this pull request Apr 10, 2017

nirbheek added a commit to centricular/meson that referenced this pull request Apr 10, 2017

nirbheek added a commit to centricular/meson that referenced this pull request Apr 10, 2017

jpakkane added a commit that referenced this pull request Apr 12, 2017

vala: Allow installation into default directories
For generated headers, vapis, and GIRs.

#1469 (comment)

gnomesysadmins pushed a commit to GNOME/tracker that referenced this pull request May 21, 2017

Meson build instructions for Tracker
See <http://mesonbuild.com/> for information about Meson.

Remaining issues:

  * There's no `make dist` equivalent. We currently produce release
    tarballs containing the output files of the Vala compiler. We need
    to think through whether we can stop doing that. Shipping the
    generated .c files does make the Vala preprocessor useless so
    it would be good if we can stop.

  * The Firefox, Thunderbird, Evolution and Nautilus plugins are not
    built

  * mesonbuild/meson#671 -- means we can't
    depend on tracker_common_dep in most places and have to manually
    set link_with, include_directories and sources instead.

  * mesonbuild/meson#1469 -- without this we
    have to install generated Vala headers using a script

  * mesonbuild/meson#1229 -- means adding
    the #include guard to libtracker-sparql/tracker-generated-no-checks.h
    is a pain

Here's a rough speed comparison.

Meson:
        time sh -c 'meson .. --prefix=/opt/tracker-meson -D bash_completion=/opt/tracker-meson/share/bash-completion -Dsystemd_user_services=/opt/tracker-meson/lib/systemd/user -Ddbus_services=/opt/tracker-meson/share/dbus-1/services && ninja-build -j 4 && ninja-build install'

        real  1m8.194s
        user  2m16.962s
        sys   0m20.532s

Autotools:
        time sh -c './configure --prefix=/opt/tracker-autotools --with-bash-completion-dir=/opt/tracker-autotools/share/bash-completion --with-session-bus-services-dir=/opt/tracker-autotools/share/dbus-1/services --disable-nautilus-extension && make -j 4 && make install'

        real  2m37.750s
        user  4m37.214s
        sys   0m54.806s

        Plus 30+ seconds of ./autogen.sh first.

Note that Meson builds may fail if your source tree has generated files
from an Autotools build in there. If you see errors about duplicate
definitions, first try cleaning your source tree (use `git clean -dfx`,
but make sure you commit any work first!!)

gnomesysadmins pushed a commit to GNOME/tracker that referenced this pull request May 22, 2017

Meson build instructions for Tracker
See <http://mesonbuild.com/> for information about Meson.

Remaining issues:

  * There's no `make dist` equivalent. We currently produce release
    tarballs containing the output files of the Vala compiler. We need
    to think through whether we can stop doing that. Shipping the
    generated .c files does make the Vala preprocessor useless so
    it would be good if we can stop.

  * The Firefox, Thunderbird, Evolution and Nautilus plugins are not
    built

  * mesonbuild/meson#671 -- means we can't
    depend on tracker_common_dep in most places and have to manually
    set link_with, include_directories and sources instead.

  * mesonbuild/meson#1469 -- without this we
    have to install generated Vala headers using a script

  * mesonbuild/meson#1229 -- means adding
    the #include guard to libtracker-sparql/tracker-generated-no-checks.h
    is a pain

  * The test suite has some spurious failures.

Here's a rough speed comparison.

Meson:
        time sh -c 'meson .. --prefix=/opt/tracker-meson -D bash_completion=/opt/tracker-meson/share/bash-completion -Dsystemd_user_services=/opt/tracker-meson/lib/systemd/user -Ddbus_services=/opt/tracker-meson/share/dbus-1/services && ninja-build -j 4 && ninja-build install'

        real  1m8.194s
        user  2m16.962s
        sys   0m20.532s

Autotools:
        time sh -c './configure --prefix=/opt/tracker-autotools --with-bash-completion-dir=/opt/tracker-autotools/share/bash-completion --with-session-bus-services-dir=/opt/tracker-autotools/share/dbus-1/services --disable-nautilus-extension && make -j 4 && make install'

        real  2m37.750s
        user  4m37.214s
        sys   0m54.806s

        Plus 30+ seconds of ./autogen.sh first.

Note that Meson builds may fail if your source tree has generated files
from an Autotools build in there. If you see errors about duplicate
definitions, first try cleaning your source tree (use `git clean -dfx`,
but make sure you commit any work first!!)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.