MINIFICPP-1759 Fix linux/libc++ issues, use libc++ on ubuntu by szaszm · Pull Request #1264 · apache/nifi-minifi-cpp

szaszm · 2022-02-17T00:26:38Z

not fixed issue: OpenCV refuses to compile with libc++/C++20
~~Bustache no longer requires boost.~~ As it turns out, libboostache depends on boost. OpenCV does, too. It's also required for some test on windows. I touched a few boost-related places.

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?

For code changes:

If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE file?
If applicable, have you updated the NOTICE file?

For documentation related changes:

Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

- Unfixed issue: OpenCV refuses to compile with libc++/C++20 - Also, removed Boost from the requirements, but didn't remove every trace of boost from the codebase, as that would be too much out of scope for my taste

szaszm · 2022-02-17T00:28:48Z

extensions/bustache/ApplyTemplate.cpp

-
-int64_t ApplyTemplate::WriteCallback::process(const std::shared_ptr<io::BaseStream>& stream) {
-  logger_->log_info("ApplyTemplate reading template file from %s", template_file_);
-  boost::iostreams::mapped_file_source file(template_file_);


This line was crashing (segfault) when compiled with libc++, so I removed it. I also saw a bunch of refactoring opportunities, so ApplyTemplate has quite a few changes. As it turns out, this line was the only reason the bustache extension required boost, so I removed that requirement and checked a few boost-related pieces of code.

szaszm · 2022-02-17T00:31:12Z

extensions/expression-language/Expression.cpp

  auto utct = date::make_zoned(utc, t);
  auto zt = date::make_zoned(zone, utct.get_local_time());
-  return Value(std::chrono::duration_cast<std::chrono::milliseconds>(zt.get_sys_time().time_since_epoch()).count());
+  return Value(int64_t{std::chrono::duration_cast<std::chrono::milliseconds>(zt.get_sys_time().time_since_epoch()).count()});


With libc++, the count return value is long long, which is not convertible to int64_t (alias to long). Value doesn't have a long long constructor, so I had to add a non-narrowing cast to these places.

szaszm · 2022-02-17T00:32:39Z

extensions/standard-processors/processors/GetFile.cpp

+  context->getProperty<core::TimePeriodValue>(PollInterval)
+      | utils::map(&core::TimePeriodValue::getMilliseconds)
+      | utils::map([this](std::chrono::milliseconds ms) { request_.pollInterval = ms; });


In file included from /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/processors/GetFile.cpp:18: In file included from /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/processors/GetFile.h:29: In file included from /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/../../libminifi/include/core/Processor.h:39: /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/../../libminifi/include/core/ConfigurableComponent.h:237:15: error: no matching conversion for static_cast from 'const org::apache::nifi::minifi::core::PropertyValue' to 'std::chrono::duration<long long, std::ratio<1, 1000>>' value = static_cast<T>(property.getValue()); // cast throws if the value is invalid ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/../../libminifi/include/core/ProcessorNode.h:73:30: note: in instantiation of function template specialization 'org::apache::nifi::minifi::core::ConfigurableComponent::getProperty<std::chrono::duration<long long, std::ratio<1, 1000>>>' requested here return processor_cast->getProperty<T>(name, value); ^ /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/../../libminifi/include/core/ProcessContext.h:386:29: note: in instantiation of function template specialization 'org::apache::nifi::minifi::core::ProcessorNode::getProperty<std::chrono::duration<long long, std::ratio<1, 1000>>>' requested here return processor_node_->getProperty<typename std::common_type<T>::type>(name, value); ^ /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/../../libminifi/include/core/ProcessContext.h:118:12: note: in instantiation of function template specialization 'org::apache::nifi::minifi::core::ProcessContext::getPropertyImp<std::chrono::duration<long long, std::ratio<1, 1000>>>' requested here return getPropertyImp<typename std::common_type<T>::type>(name, value); ^ /home/szaszm/nifi-minifi-cpp-3/extensions/standard-processors/processors/GetFile.cpp:136:12: note: in instantiation of function template specialization 'org::apache::nifi::minifi::core::ProcessContext::getProperty<std::chrono::duration<long long, std::ratio<1, 1000>>>' requested here context->getProperty(PollInterval.getName(), request_.pollInterval); ^ /usr/include/c++/v1/chrono:1023:28: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const org::apache::nifi::minifi::core::PropertyValue' to 'const std::chrono::duration<long long, std::ratio<1, 1000>>' for 1st argument class _LIBCPP_TEMPLATE_VIS duration ^ /usr/include/c++/v1/chrono:1023:28: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const org::apache::nifi::minifi::core::PropertyValue' to 'std::chrono::duration<long long, std::ratio<1, 1000>>' for 1st argument /usr/include/c++/v1/chrono:1075:18: note: candidate template ignored: requirement 'is_convertible<org::apache::nifi::minifi::core::PropertyValue, long long>::value' was not satisfied [with _Rep2 = org::apache::nifi::minifi::core::PropertyValue] explicit duration(const _Rep2& __r, ^ /usr/include/c++/v1/chrono:1087:9: note: candidate template ignored: could not match 'duration<type-parameter-0-0, type-parameter-0-1>' against 'const org::apache::nifi::minifi::core::PropertyValue' duration(const duration<_Rep2, _Period2>& __d, ^ /usr/include/c++/v1/chrono:1068:9: note: candidate constructor not viable: requires 0 arguments, but 1 was provided duration() = default; ^

szaszm · 2022-02-17T00:34:19Z

libminifi/include/utils/OptionalUtils.h

-    return std::make_optional(std::invoke(std::forward<F>(f.function), *o));
+auto operator|(std::optional<SourceType> o, map_wrapper<F> f) noexcept(noexcept(std::invoke(std::forward<F>(f.function), *std::move(o)))) {
+  using cb_result = std::decay_t<std::invoke_result_t<F, SourceType>>;
+  if constexpr(std::is_same_v<cb_result, void>) {


Added support for void return types from the callback, in which case map just returns void. There is no such thing as optional<void>. This simplified the change in GetFile.

…stache itself depends on boost

extensions/bustache/ApplyTemplate.cpp

extensions/standard-processors/processors/GetFile.cpp

Co-authored-by: Ferenc Gerlits <fgerlits@gmail.com>

MINIFICPP-1759 Fix linux/libc++ issues, use libc++ on ubuntu

297e653

- Unfixed issue: OpenCV refuses to compile with libc++/C++20 - Also, removed Boost from the requirements, but didn't remove every trace of boost from the codebase, as that would be too much out of scope for my taste

szaszm commented Feb 17, 2022

View reviewed changes

szaszm marked this pull request as draft February 17, 2022 00:38

szaszm added 3 commits February 17, 2022 01:39

only forgot the whole point of this diff

4187dd1

restoring boostache -> boost dependency, since as it turns out, libbu…

c49d630

…stache itself depends on boost

try again with newer clang

e5ec1f9

szaszm marked this pull request as ready for review February 17, 2022 14:26

adam-markovics reviewed Feb 22, 2022

View reviewed changes

extensions/bustache/ApplyTemplate.cpp Outdated Show resolved Hide resolved

remove unnecessary std::move

ed63ee7

adam-markovics approved these changes Feb 23, 2022

View reviewed changes

lordgamez approved these changes Feb 24, 2022

View reviewed changes

fgerlits reviewed Mar 1, 2022

View reviewed changes

extensions/standard-processors/processors/GetFile.cpp Outdated Show resolved Hide resolved

review

b6abaeb

Co-authored-by: Ferenc Gerlits <fgerlits@gmail.com>

fgerlits approved these changes Mar 1, 2022

View reviewed changes

fgerlits closed this in 7e2dbeb Mar 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MINIFICPP-1759 Fix linux/libc++ issues, use libc++ on ubuntu#1264

MINIFICPP-1759 Fix linux/libc++ issues, use libc++ on ubuntu#1264
szaszm wants to merge 6 commits intoapache:mainfrom
szaszm:MINIFICPP-1759

szaszm commented Feb 17, 2022 •

edited

Loading

Uh oh!

szaszm Feb 17, 2022

Uh oh!

szaszm Feb 17, 2022

Uh oh!

szaszm Feb 17, 2022

Uh oh!

szaszm Feb 17, 2022

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

szaszm commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For all changes:

For code changes:

For documentation related changes:

Note:

Uh oh!

szaszm Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

szaszm Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

szaszm Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

szaszm Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

szaszm commented Feb 17, 2022 •

edited

Loading