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

Allow meson wrapper to use backends other than ninja #10447

Merged

Conversation

Makogan
Copy link
Contributor

@Makogan Makogan commented Jan 27, 2022

Changelog: Fix: Let legacy Meson build helper use other backends apart from ninja.
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.

Issue: #10440

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.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2022

CLA assistant check
All committers have signed the CLA.

@memsharded memsharded added this to the 1.45 milestone Jan 28, 2022
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

This would need some tests to check it is working. Could be unit or integration, not necessary full functional test.

Thanks for contributing!

@franramirez688
Copy link
Contributor

Thanks again for your PR @Makogan

As @memsharded said, please, it would be great some tests checking it. As a suggestion, have a look at the tests from conans/test/unittests/client/build/meson_test.py It could be a good place to add them.
Let us know if you have any questions.

@franramirez688 franramirez688 linked an issue Jan 28, 2022 that may be closed by this pull request
@Makogan
Copy link
Contributor Author

Makogan commented Jan 28, 2022

I added a unit test and I am trying to run it, the docs say to do python -m tox after installing ig it, I am getting a ton of errors like this:

___________________________________________________________________ ERROR collecting conans/test/integration/tools/cpu_count_test.py ___________________________________________________________________ 
ImportError while importing test module 'C:\Users\Makogan\mako_conan\conans\test\integration\tools\cpu_count_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
C:\Python311\Lib\importlib\__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
conans\test\integration\tools\cpu_count_test.py:4: in <module>
    from conans.test.utils.tools import TestClient
conans\test\utils\tools.py:15: in <module>
    import bottle
.tox\full\Lib\site-packages\bottle.py:44: in <module>
    from inspect import getargspec
E   ImportError: cannot import name 'getargspec' from 'inspect' (C:\Python311\Lib\inspect.py)

@memsharded
Copy link
Member

If you are adding 1 or just a very few unit tests, the best is to run them directly with pytest conans/test/../path/to/test/test_file.py, no need to run the full suite (that also requires to have more build tools installed). Then you can submit and our CI will check everything. Thanks!

@franramirez688
Copy link
Contributor

franramirez688 commented Feb 1, 2022

Hi @Makogan! I have added a simple unit test to check that it's working the change made for other backends and a little refactor to have only one _run_meson_targets method.
@memsharded I have checked a couple of conan-center-index recipes to verify that we're not breaking anything and it's so. If all the tests are passing, we can include this PR for the next 1.45 release.

@franramirez688 franramirez688 merged commit b01cfc0 into conan-io:develop Feb 1, 2022
@Makogan
Copy link
Contributor Author

Makogan commented Feb 1, 2022

Sorry I had some personal and professional issues and forgot about the PR.

@franramirez688
Copy link
Contributor

Don't worry at all @Makogan
The fix is already merged so the problem should be solved for the next 1.45 release.

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.

[feature] Support visual studio backend for meson
5 participants