Skip to content

create add cmocka module from scratch#1368

Closed
vhvb1989 wants to merge 8 commits intoAzure:masterfrom
vhvb1989:create-cmoka-test-cmake-module
Closed

create add cmocka module from scratch#1368
vhvb1989 wants to merge 8 commits intoAzure:masterfrom
vhvb1989:create-cmoka-test-cmake-module

Conversation

@vhvb1989
Copy link
Copy Markdown
Member

Creating a cmake module that creates the cmocka tests exe binaries.
Removing the old cmake module that we were using as a modified version from 3rd party

@vhvb1989 vhvb1989 mentioned this pull request Sep 25, 2020
3 tasks
@ahsonkhan ahsonkhan added the EngSys This issue is impacting the engineering system. label Sep 25, 2020
@ahsonkhan ahsonkhan added this to the [2020] October milestone Sep 25, 2020
@@ -0,0 +1,109 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I created one from scratch, with our specific needs (private access for tests to az_private, linker flags). So we should support and maintain our own version.
We don't support wine.

Copy link
Copy Markdown
Contributor

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

Why did we have to write our own?

@antkmsft
Copy link
Copy Markdown
Member

Can you find if it is possible to pass compilation flags / CMAKE_BUILD_TYPE to cmocka, at the time cmocka itself gets built? This would remove the need to suppress libcmtd warning.

@RickWinter RickWinter added the Client This issue points to a problem in the data-plane of the library. label Sep 28, 2020
Copy link
Copy Markdown
Member

@antkmsft antkmsft left a comment

Choose a reason for hiding this comment

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

As long as it does what we need, and is not very complicated, I'm not against the the idea of having our own.

@antkmsft
Copy link
Copy Markdown
Member

If this PR goes after #1338, please make sure to NOT include the libcmtd warning suppression workaround.

@vhvb1989
Copy link
Copy Markdown
Member Author

If this PR goes after #1338, please make sure to NOT include the libcmtd warning suppression workaround.

Yes @antkmsft , actually I have removed the warning fix and now this became blocker by #1338

So, I will wait for you to merge

@antkmsft
Copy link
Copy Markdown
Member

If this PR goes after #1338, please make sure to NOT include the libcmtd warning suppression workaround.

Yes @antkmsft , actually I have removed the warning fix and now this became blocker by #1338

So, I will wait for you to merge

Thank you @vhvb1989 , my PR is merged.

@vhvb1989 vhvb1989 closed this Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants