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

- Feature: add backend support to MesonToolchain #9990

Merged
merged 5 commits into from
Nov 16, 2021

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Nov 9, 2021

Changelog: Feature: Add backend support to MesonToolchain.
Docs: conan-io/docs#2295

Closes: #9922

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

@lasote
Copy link
Contributor

lasote commented Nov 10, 2021

Could we add a test using the visual backend?

Signed-off-by: SSE4 <[email protected]>
@SSE4
Copy link
Contributor Author

SSE4 commented Nov 10, 2021

Could we add a test using the visual backend?

sure, I've added some test

@SSE4 SSE4 marked this pull request as ready for review November 10, 2021 11:52
client.run("build .")
client.run_command(os.path.join("build", "demo"))

assert "main _M_X64 defined" in client.out
Copy link
Member

Choose a reason for hiding this comment

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

It is very likely that this would be identical irrespective of the backend. If it is, and we cannot test the backend this way, then maybe move this test to "integration" make it faster (no need to compile anything), and just check the generated conan_meson_native.ini?

Copy link
Member

Choose a reason for hiding this comment

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

Please check this @SSE4 if possible, to get this merged to 1.43

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what do you mean by we cannot test the backend this way.
we can test, and it's already tested that way. we build an executable and verify it can be run successfully, and was compiled for the proper architecture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is very likely that this would be identical irrespective of the backend.
I am also not sure about this one. in case of ninja (default), there is ninja build generated and built via ninja.
in case of vs backend, it generates vcxproj file and built it via msbuild.
in second case you can use an IDE to open and edit the project. so it's not identical.

Copy link
Member

Choose a reason for hiding this comment

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

The output of running the compiled executable depends only in the compiler (and the conan settings), it should be the same with either backend. So this test output checks will still pass even if the backend was not vs. The only valid check in this test is the one checking backend = "vs", and the test can stop there, unless some other check is done, like checking the output of Meson for something like "Using the Visual Studio backend".

Copy link
Contributor Author

@SSE4 SSE4 Nov 16, 2021

Choose a reason for hiding this comment

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

What I am saying is that the test should somehow check that the VS is currently in use, and there is nothing there.

I doubt it's worth implementing that check, to be honest. the only way to check is to verify which process is executed during the meson build (like msbuild.exe or devenv.exe or whatever), and this process is launched from another child process of the conan client, so it's tricky to handle that.

Copy link
Member

Choose a reason for hiding this comment

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

Restored the check. Added assert "Auto detected Visual Studio backend" in client.out to the test.
Now, there is some good evidence that Meson is actually using the Visual Studio backend.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I messed with the branches. I will restore things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, it might be enough for our purposes. it only verifies that meson prints something (e.g. claims that it uses Visual Studio backend), but we still don't know does it actually use it. but even if it doesn't, I'd say it's meson bug and not our problem :)

Copy link
Member

@memsharded memsharded Nov 16, 2021

Choose a reason for hiding this comment

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

Yes, that would be a Meson bug. Our job ends verifying that Meson uses that VS backend, and that check should be enough with it.

@memsharded memsharded merged commit 68b457f into conan-io:develop Nov 16, 2021
memsharded added a commit to memsharded/conan that referenced this pull request Nov 19, 2021
* - [MesonToolchain] add backend support

Signed-off-by: SSE4 <[email protected]>

* - add test

Signed-off-by: SSE4 <[email protected]>

* - move

Signed-off-by: SSE4 <[email protected]>

* fixed wrong develop2 merge, added test check for VS backend

Co-authored-by: memsharded <[email protected]>
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.

[question] Why use ninja in meson to replace meson's built-in compilation
3 participants