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

fix meson build_type #4489

Merged
merged 2 commits into from Feb 11, 2019

Conversation

Projects
None yet
5 participants
@ericLemanissier
Copy link
Contributor

commented Feb 8, 2019

Meson's documentation currently mentions default-library option,
but it is a typo, the proper option is default_library

Changelog: Bugfix: meson build type actually reflects recipe shared option
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@jgsogo

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

I'm not sure if this is an error, I've just installed Meson and this is what I get:

⇒  meson --version
0.49.2

⇒  meson setup --help
usage: meson setup [-h]
                   ...
                   [--default-library {shared,static,both}]
                   ...
                   [builddir] [sourcedir]

positional arguments:
  builddir
  sourcedir

optional arguments:
 ....
  --default-library {shared,static,both}
                        Default library type (default: shared).

It looks like default-library is the name of the parameter.

@uilianries

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

--default-library is an argument, but the Meson helper uses options instead, -Ddefault-library

If try to build some project, it will see the follow warning: WARNING: Unknown options: "default-library"

Also, the package xkbcommon is a good example, you can see log here

@jgsogo

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

You are right, @uilianries. Thanks a lot!

Please, @ericLemanissier, modify the test that is failing to fix it.

Thanks!

@jgsogo jgsogo self-assigned this Feb 8, 2019

@ericLemanissier

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

default_library has 8 occurences whereas default-library has only 1, and it is documentation. it looks like a typo to me.

fix meson build_type
Meson's documentation currently mentions default-library option,
but it is a typo, the proper option is default_library

@ericLemanissier ericLemanissier force-pushed the ericLemanissier:patch-1 branch from 79e49fb to 5777aaa Feb 8, 2019

@uilianries

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

BTW, should we include libdir on this issue? As you can see, I needed to add for xkbcommon, otherwise the libraries are installer on package_folder/lib//

@ericLemanissier ericLemanissier force-pushed the ericLemanissier:patch-1 branch from ef9c7d5 to f3e47fd Feb 8, 2019

@jgsogo

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

Using arguments it is by default ( --libdir LIBDIR Library directory (default: lib).), but if using options we need to set it, then we would need to set all of them... libdir, libexecdir, bindir, sbindir,... no?

@uilianries

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

hmm, you are right, as done for cmake. Only libdir is enough for xkbcommon, but because we only need to install the libraries.

@ericLemanissier

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

What would the default be for libexecdir ? meson has 'libexec' as default value
the other defaults are actually good : https://github.com/mesonbuild/meson/blob/master/mesonbuild/coredata.py#L807

@jgsogo

This comment has been minimized.

Copy link
Member

commented Feb 8, 2019

Here I need to invoke @danimtb, he worked on that for Autotools if I remember it well.

@jgsogo jgsogo requested a review from danimtb Feb 8, 2019

@uilianries

This comment has been minimized.

@ericLemanissier ericLemanissier force-pushed the ericLemanissier:patch-1 branch from f3e47fd to c60c3fe Feb 8, 2019

@ericLemanissier

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

the typo on default-/_library documentation is fixed upstream: mesonbuild/meson#4897

@memsharded memsharded added this to the 1.13 milestone Feb 10, 2019

@danimtb

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

As @uilianries referenced above, I followed in autotools the same pattern of cmake. libexecdir definition can be found here like "The directory for installing executable programs to be run by other programs rather than by users" but were actually not making any difference, so to me it should be force to the "bin" directory to be consistent with the other build helpers.

@jgsogo jgsogo merged commit aa56072 into conan-io:master Feb 11, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ericLemanissier ericLemanissier deleted the ericLemanissier:patch-1 branch Feb 11, 2019

@jgsogo

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Upsss 😓 , this PR should have been merged into develop. I'm going to revert it from master and create a PR to develop. It was my fault... but your code will be in the next release, thanks @ericLemanissier .

jgsogo added a commit that referenced this pull request Feb 11, 2019

jgsogo added a commit to jgsogo/conan that referenced this pull request Feb 11, 2019

fix meson build_type (conan-io#4489)
* fix meson build_type

Meson's documentation currently mentions default-library option,
but it is a typo, the proper option is default_library

* set default meson dir options

jgsogo added a commit that referenced this pull request Feb 11, 2019

Revert "fix meson build_type" (#4503)
* Revert "fix meson build_type (#4489)"

This reverts commit aa56072.

* touch

jgsogo added a commit that referenced this pull request Feb 11, 2019

fix meson build_type (#4489) (#4504)
* fix meson build_type

Meson's documentation currently mentions default-library option,
but it is a typo, the proper option is default_library

* set default meson dir options
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.