Skip to content

[meson] Fixes for test and install + meson_test & meson_install - #6574

Merged
memsharded merged 1 commit into
conan-io:developfrom
TheQwertiest:feature/meson_fixes
Mar 5, 2020
Merged

memsharded merged 1 commit into
conan-io:developfrom
TheQwertiest:feature/meson_fixes

Conversation

@TheQwertiest

@TheQwertiest TheQwertiest commented Feb 20, 2020

Copy link
Copy Markdown
Contributor

Changelog: Feature: Fixed inability to run execute test and install separately, that is, without build step. Added meson_test() method, which executes meson test (compared to ninja test in test()). Added meson_install() method, which executes meson install (compared to ninja install in install()).

Docs: conan-io/docs#1568

Fixed inability to run execute test and install separately, that is, without build step. Added meson_test() method, which executes meson test (compared to ninja test in test()). meson test have multiple options for additional test configuration (https://mesonbuild.com/Unit-tests.html#testing-tool). Added meson_install() method, which executes meson install (compared to ninja install in install()). meson install have multiple options for additional installation configuration (https://mesonbuild.com/Installing.html#custom-install-behaviour).

  • 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.

@claassistantio

claassistantio commented Feb 20, 2020

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

- Fixed inability to run execute `test` and `install` separately, that is without `build` step.
- Added `meson_test` method, which executes `meson test` (compared to `ninja test` in test). `meson test` have multiple options for tests configuration (https://mesonbuild.com/Unit-tests.html).
@TheQwertiest

TheQwertiest commented Feb 21, 2020

Copy link
Copy Markdown
Contributor Author

Ideally (implementations of) test() and install() should be replaced with (implementations of) meson_test() and meson_install(), since the later are backend-agnostic, while the former are restricted to ninja backend only.
But that would be a breaking change, thus I left it as-is.

@jgsogo jgsogo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your work on this PR, specially for taking care not breaking the existing behavior and writing the docs. Thanks!


The behavior will change: now the barrier self._conanfile.should_build is not taken into account for install and test functions, but it matches current behavior in other build-helpers like CMake and it totally makes sense. I hope noone complains 🤞

About functions meson_test and meson_install, your suggestion should be taken into account for Conan v2, let's open an issue with that.

Thanks again 🎉 !!

@jgsogo jgsogo added this to the 1.23 milestone Feb 24, 2020
@jgsogo
jgsogo requested a review from czoido February 24, 2020 19:05
@memsharded memsharded self-assigned this Mar 2, 2020
@memsharded
memsharded requested a review from uilianries March 2, 2020 15:12
@TheQwertiest

TheQwertiest commented Mar 3, 2020

Copy link
Copy Markdown
Contributor Author

I want to add meson version checking to this PR before merging (in case user has an older version, where new methods are not available).
But I need help, since I'm unfamiliar with conan code: there is a get_version method (meson.py#L214) which can be used to get meson version. Would it be correct to cache it's result during Meson's __init__()? I.e. smth like self._version=get_version().

@memsharded

Copy link
Copy Markdown
Member

Hi @TheQwertiest

I want to add meson version checking to this PR before merging (in case user has an older version, where new methods are not available).
But I need help, since I'm unfamiliar with conan code: there is a get_version method (meson.py#L214) which can be used to get meson version. Would it be correct to cache it's result during Meson's init()? I.e. smth like self._version=get_version().

Yes, I think that we can assume that a Meson instance doesn't change the meson version during its lifetime, go ahead, please.

def meson_install(self, args=None, build_dir=None):
if not self._conanfile.should_install:
return
self._run_meson_command(subcommand='install', args=args, build_dir=build_dir)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commands might not be supported by all meson versions, so @TheQwertiest is suggesting to add a version check to decide behavior. I am fine with that.

@memsharded

Copy link
Copy Markdown
Member

Hi @TheQwertiest

Tomorrow I will merge this. I think it can be merged even if the version check is not there, after all, they are new methods, very low risk. Version check could be added later, or maybe even not necessary. Do you think that it is very important?

@TheQwertiest

Copy link
Copy Markdown
Contributor Author

it's not very important, just a better diagnostics message.
I was planning to add those tomorrow, but can push them to a separate PR, if needed.

@memsharded

Copy link
Copy Markdown
Member

it's not very important, just a better diagnostics message.
I was planning to add those tomorrow, but can push them to a separate PR, if needed.

Cool, I am going to merge this now. If you manage to do it, please open a new PR, thanks!

@memsharded
memsharded merged commit a99199c into conan-io:develop Mar 5, 2020
czoido added a commit to conan-io/docs that referenced this pull request Mar 10, 2020
@jgsogo jgsogo mentioned this pull request Mar 11, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants