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

Add external dependencies option #547

Merged
merged 14 commits into from
May 24, 2024

Conversation

uilianries
Copy link
Contributor

Add support to use external dependencies instead of vendorized ones. The new CMake option USE_SYSTEM_DEPENDENCIES, uses find_package to include (mandatory) fmt and gtest. In case that option is enabled, and the compiler has std::format supported, CMake will use the external dependencies, because the user configured it explicitly.

Added a new CI job to validate the configuration.

closes #546

uilianries added 11 commits May 23, 2024 15:44
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Copy link
Owner

@cieslarmichal cieslarmichal left a comment

Choose a reason for hiding this comment

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

Nice

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

@cieslarmichal The current error is due -Werror with unused variable:

FAILED: CMakeFiles/faker-cxx.dir/src/modules/string/String.cpp.o 
/usr/bin/g++-13  -I/home/runner/work/faker-cxx/faker-cxx/include -isystem /home/runner/.conan2/p/b/fmt808a6edee61f6/p/include -m64 -O3 -DNDEBUG -std=c++20 -Wall -Wextra -Wpedantic -Wconversion -Wformat -Werror -MD -MT CMakeFiles/faker-cxx.dir/src/modules/string/String.cpp.o -MF CMakeFiles/faker-cxx.dir/src/modules/string/String.cpp.o.d -o CMakeFiles/faker-cxx.dir/src/modules/string/String.cpp.o -c /home/runner/work/faker-cxx/faker-cxx/src/modules/string/String.cpp
In file included from /usr/include/c++/13/map:62,
                 from /home/runner/work/faker-cxx/faker-cxx/include/faker-cxx/String.h:4,
                 from /home/runner/work/faker-cxx/faker-cxx/src/modules/string/String.cpp:1:
In member function ‘std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::iterator std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_lower_bound(_Link_type, _Base_ptr, const _Key&) [with _Key = char; _Val = std::pair<const char, faker::CharCount>; _KeyOfValue = std::_Select1st<std::pair<const char, faker::CharCount> >; _Compare = std::less<char>; _Alloc = std::allocator<std::pair<const char, faker::CharCount> >]’,
    inlined from ‘std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::iterator std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::find(const _Key&) [with _Key = char; _Val = std::pair<const char, faker::CharCount>; _KeyOfValue = std::_Select1st<std::pair<const char, faker::CharCount> >; _Compare = std::less<char>; _Alloc = std::allocator<std::pair<const char, faker::CharCount> >]’ at /usr/include/c++/13/bits/stl_tree.h:2532:36,
    inlined from ‘std::map<_Key, _Tp, _Compare, _Alloc>::iterator std::map<_Key, _Tp, _Compare, _Alloc>::find(const key_type&) [with _Key = char; _Tp = faker::CharCount; _Compare = std::less<char>; _Alloc = std::allocator<std::pair<const char, faker::CharCount> >]’ at /usr/include/c++/13/bits/stl_map.h:1220:25,
    inlined from ‘static std::string faker::String::generateStringWithGuarantee(faker::GuaranteeMap&, std::set<char>&, unsigned int)’ at /home/runner/work/faker-cxx/faker-cxx/src/modules/string/String.cpp:96:37:
/usr/include/c++/13/bits/stl_tree.h:1952:9: error: ‘item’ may be used uninitialized [-Werror=maybe-uninitialized]
 1952 |         if (!_M_impl._M_key_compare(_S_key(__x), __k))
      |         ^~
In file included from /home/runner/work/faker-cxx/faker-cxx/src/modules/string/String.cpp:9:
/home/runner/work/faker-cxx/faker-cxx/include/faker-cxx/Helper.h: In static member function ‘static std::string faker::String::generateStringWithGuarantee(faker::GuaranteeMap&, std::set<char>&, unsigned int)’:
/home/runner/work/faker-cxx/faker-cxx/include/faker-cxx/Helper.h:92:11: note: ‘item’ was declared here
   92 |         T item;
      |           ^~~~
cc1plus: all warnings being treated as errors

Not sure what best approach here, it could ignore Werror (remove from CMake), ignore that specific error (add pragma in String.cpp), or fixing it in the code (I don't know should be fixed in that case.

@cieslarmichal
Copy link
Owner

I think we can disable werror flag.

@uilianries
Copy link
Contributor Author

@cieslarmichal Done, without -Werror everything passed.

@cieslarmichal cieslarmichal merged commit 60593d9 into cieslarmichal:main May 24, 2024
6 checks passed
00thirdeye00 pushed a commit to 00thirdeye00/faker-cxx that referenced this pull request Jun 20, 2024
* Add external dependencies option

Signed-off-by: Uilian Ries <[email protected]>

* Add entry documenation to explain option

Signed-off-by: Uilian Ries <[email protected]>

* Add build for external dependencies

Signed-off-by: Uilian Ries <[email protected]>

* build on feature branch

Signed-off-by: Uilian Ries <[email protected]>

* Use Ubuntu 24.04

Signed-off-by: Uilian Ries <[email protected]>

* Use python setup

Signed-off-by: Uilian Ries <[email protected]>

* fix conan install

Signed-off-by: Uilian Ries <[email protected]>

* Use std format only when no deps

Signed-off-by: Uilian Ries <[email protected]>

* No werror

Signed-off-by: Uilian Ries <[email protected]>

* Add Werror

Signed-off-by: Uilian Ries <[email protected]>

* Use compile options

Signed-off-by: Uilian Ries <[email protected]>

* Pass msvc flags

Signed-off-by: Uilian Ries <[email protected]>

* Drop -Werror

Signed-off-by: Uilian Ries <[email protected]>

---------

Signed-off-by: Uilian Ries <[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.

suggestion: Add support for external libfmt and google test
2 participants