-
Notifications
You must be signed in to change notification settings - Fork 8
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
Port to Conan 2 #33
Port to Conan 2 #33
Conversation
|
||
def test(self): | ||
cmake = CMake(self) | ||
cmake.test() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to check unit test docs for how to properly do this
void writeKv(const std::string_view key, const int value, const jsonopt opts); | ||
void writeKv(const char *key, const std::string &value, const jsonopt opts); | ||
void writeKv(const char *key, const int value, const jsonopt opts); | ||
void writeKv(const std::string_view key, const long long value, const jsonopt opts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the description, these changes are just so it could build in apple-clang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd hate to suggest it, but maybe you can use #if defined(__APPLE__)
I wonder where this is coming from though, I don't recall a long long type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nor should it have any issues with std::string&
I wonder if maybe libc++ is the culprit, I can probably test that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/compiler-explorer/asm-parser/blob/main/.github/workflows/build.yml also needs to be changed here
I think libcxx=libstdc++11 is now the default and shouldn't have to be mentioned (it was used to differentiate between it comforming to the cxx11 ABI (all compilers set to >= c++11 match that ABI, all before do not))
@@ -38,7 +38,7 @@ size_t AsmParser::ustrlen(const std::string_view s) | |||
ulen += 1; | |||
if (maxlen > (size_t)mbcharlen) | |||
{ | |||
maxlen -= mbcharlen; | |||
maxlen -= static_cast<size_t>(mbcharlen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
void writeKv(const std::string_view key, const int value, const jsonopt opts); | ||
void writeKv(const char *key, const std::string &value, const jsonopt opts); | ||
void writeKv(const char *key, const int value, const jsonopt opts); | ||
void writeKv(const std::string_view key, const long long value, const jsonopt opts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd hate to suggest it, but maybe you can use #if defined(__APPLE__)
I wonder where this is coming from though, I don't recall a long long type
void writeKv(const std::string_view key, const int value, const jsonopt opts); | ||
void writeKv(const char *key, const std::string &value, const jsonopt opts); | ||
void writeKv(const char *key, const int value, const jsonopt opts); | ||
void writeKv(const std::string_view key, const long long value, const jsonopt opts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nor should it have any issues with std::string&
I wonder if maybe libc++ is the culprit, I can probably test that
added conan 2 but went a different way in #42 |
Draft, made this while getting breakfast :)
For this to work, you only need Conan 2 installed. The profile will be detected when running the first time.
The build command is simply:
conan build . -s build_type=Release -s compiler.cppstd=20 --build=missing
(Or, if the profile already defines those --settings, tbose can also be skipped) which generates abuild/
folder with the executable. The only bit I'm not sure how to handle is thelibcxx=libstdc++11
in thesetup.sh
file.Note that all the changes in the util's folder are the changes I needed to make in order to get it to compile in apple-clang