You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/07/16 10:55:48 UTC
[GitHub] [nifi-minifi-cpp] szaszm opened a new pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
szaszm opened a new pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133
MINIFICPP-1425 Upgrade compilers on Windows, CentOS 7, debian, ubuntu https://issues.apache.org/jira/browse/MINIFICPP-1425
MINIFICPP-1427 Use devtoolset-10/gcc-toolset-10 or newer on CentOS/Rocky Linux 7+ https://issues.apache.org/jira/browse/MINIFICPP-1427
MINIFICPP-1428 Ensure that MiNiFi builds on debian 10 and ubuntu 18.04 with GCC 8 https://issues.apache.org/jira/browse/MINIFICPP-1428
MINIFICPP-1429 Update minimal supported language version to C++17, replace workarounds https://issues.apache.org/jira/browse/MINIFICPP-1429
MINIFICPP-1430 Explore the possibility of creating binaries for target linux OSes with latest compilers https://issues.apache.org/jira/browse/MINIFICPP-1430
This commit not replace workarounds in the code yet. Various fixes are
included for partial C++20 support, as all of the required compiler
versions implement C++20 to various (usually small) degrees.
----
Squashed commit of the following:
commit 686ebeb56b9953051e88924a1dc3d974b7c0e8d0
Author: Marton Szasz <sz...@apache.org>
Date: Fri Jul 16 12:28:34 2021 +0200
update readme, remove accidentally committed file
commit 5046b9ea0983575e8956d16ba586b2510b56adf7
Author: Marton Szasz <sz...@apache.org>
Date: Fri Jul 16 08:41:00 2021 +0200
force CPPFLAGS into CFLAGS/CXXFLAGS on mac
Because cmake doesn't seem to recognize CPPFLAGS the same way it does
CFLAGS/CXXFLAGS. https://cmake.org/cmake/help/latest/manual/cmake-env-variables.7.html#id4
commit 9601ac037be4a101456d1b7ca44cb4e2f5e6b140
Author: Marton Szasz <sz...@apache.org>
Date: Thu Jul 15 21:42:05 2021 +0200
maybe flex would work without --c++?
commit c483e294d7c6a3b5908d3df956bb64e868d91b58
Author: Marton Szasz <sz...@apache.org>
Date: Thu Jul 15 11:11:52 2021 +0200
attempt to fix flex on mac
commit 13b8ca990c09f545f95c585de96685c4b3d78a12
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 14 15:53:03 2021 +0200
use newer flex on mac, more win macro workarounds
commit f14ddea5870a3db8896e386bca03fd0fcc310a41
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 14 11:01:59 2021 +0200
windows macro hell, try avoiding GetObject errors
commit 48bbcf640f342abc6c7088570c3e122e0a51d802
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 14 05:07:09 2021 +0200
revert to c++17
commit 38fce919e8cc1721a826e7d05a471030aad724f3
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 14 04:46:27 2021 +0200
patch sol2 to not fail in C++20 mode
commit 041249beb59cb163f8458c97ff1f4cab450771e1
Author: Marton Szasz <sz...@gmail.com>
Date: Tue Jul 13 17:37:55 2021 +0200
windows macro workaround
commit fdbb1602fc394a09ce462b3c225ca80613498c64
Author: Marton Szasz <sz...@apache.org>
Date: Tue Jul 13 09:41:38 2021 +0200
fix patch commands
commit 7722d478a93fe889a160f02ac93b74da15312eb0
Author: Marton Szasz <sz...@apache.org>
Date: Mon Jul 12 16:22:42 2021 +0200
attempt to generate better error msg on msvc
date::parse failed to specialize, but I can't see why. Changing to
a direct call of date::from_stream.
commit ff88bb64e3a8a71d331b661b1e18c206aca47f5f
Author: Marton Szasz <sz...@apache.org>
Date: Fri Jul 9 01:14:52 2021 +0200
patch aws-sdk-cpp to fix compilation error
and update other git patch commands to not touch files unless required
commit aee3bdf21f8910511697475b389c9dc99326cb21
Author: Marton Szasz <sz...@apache.org>
Date: Thu Jul 8 18:37:32 2021 +0200
shellcheck fixes, disable older macos xcode ci
commit 85f9ecef5f036983a946e82059da474e0792269d
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 7 19:17:19 2021 +0200
avoid bootstrap on some docker builds, centos fix
Because the systemd GCC version of ubuntu 20.04 is fine for just running cmake
commit 034046f0645e3f04c0a7056282a89072cf16ef99
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 7 18:58:11 2021 +0200
fix various issues
commit a5c419b811efd73893e27e96828366773634312a
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 7 16:41:44 2021 +0200
update CI, drop old distros
commit f178340472f34a4cf1010a0417580ba8d55d2b60
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jul 7 16:36:25 2021 +0200
debian buster fixes
commit c7e6e8a8c33e94ec245da4ca45757481d397a24c
Author: Marton Szasz <sz...@apache.org>
Date: Tue Jun 22 14:27:51 2021 +0000
ubuntu 18.04 bootstrap
commit b0d20b6c34fd5be0721036c56d5d91cef64d3ee2
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jun 16 18:31:08 2021 +0200
centos7 fix
commit ac7f2baa77e33cdc9b49d34ae72c34d2f25529ba
Author: Marton Szasz <sz...@apache.org>
Date: Wed Jun 16 15:51:44 2021 +0200
MINIFICPP-1427 use recent toolchain on CentOS
commit 6549a12226b6ae8e10820604c0de14cb97096c50
Author: Marton Szasz <sz...@apache.org>
Date: Tue Jun 15 17:49:06 2021 +0200
forward compilers to ossp-uuid build
otherwise libtool fails on centos7
commit ae5ce6789e6a297006e335d62055d805e468be1d
Author: Marton Szasz <sz...@apache.org>
Date: Tue Jun 15 16:23:49 2021 +0200
drop CentOS 6, use devtoolset-10 on CentOS 7
commit 2344d24083d8d35875be6ff6d4467c14df226106
Author: Marton Szasz <sz...@gmail.com>
Date: Tue Jun 15 15:29:48 2021 +0200
clang fix
commit 2eb8dc40230e8043dbde132068a8b077631de254
Author: Marton Szasz <sz...@gmail.com>
Date: Fri Jun 11 17:55:43 2021 +0200
WIP: GCC 11 C++20
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:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] 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.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [x] If applicable, have you updated the LICENSE file?
- [x] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [x] 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673843086
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
I left it as a documentation of what could be changed there, but I had to comment it out, because shellcheck complained that it was a noop. Since it doesn't currently complain in the CI, I assume it might be because of a version difference between the CI and my machine. Mine is 0.7.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673843606
##########
File path: .github/workflows/ci.yml
##########
@@ -196,7 +110,13 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1
+ run: |
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=gcc-11
+ export CXX=g++-11
+ cmake -DUSE_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
Review comment:
I don't think it's strictly necessary to have all extensions with both compilers, but it doesn't hurt either, so I will extend the ubuntu 20.04 job later today.
And thank you for taking the time for a detailed review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673973573
##########
File path: .github/workflows/ci.yml
##########
@@ -196,7 +110,13 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1
+ run: |
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=gcc-11
+ export CXX=g++-11
+ cmake -DUSE_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
Review comment:
I made an attempt in 9f3a968, let's wait for the CI results
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673852914
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
Thanks for the info, I checked and it IS because of the version difference. I had version 0.7.0 and it passed after uncommenting it, but after checking it with version 0.7.2 it failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673867965
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
I'm okay with this, you can keep it this way in both cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674661185
##########
File path: extensions/expression-language/Expression.cpp
##########
@@ -25,6 +25,10 @@
#include <functional>
#include <string>
+#ifdef GetObject
+#undef GetObject
+#endif
Review comment:
As in #1129, I would change all occurrences of this to:
```suggestion
#undef GetObject // windows.h #defines GetObject = GetObjectA or GetObjectW, which conflicts with rapidjson
```
(not sure if they are the same lines; if so then I guess resolving merge conflicts will take care of this)
##########
File path: libminifi/test/Utils.h
##########
@@ -16,8 +16,16 @@
*/
#pragma once
+#ifdef GetObject
+#undef GetObject
+#endif
+
#include <string>
#include <utility>
+
+#ifdef GetObject
+#undef GetObject
+#endif
Review comment:
do we need this twice?
##########
File path: extensions/script/CMakeLists.txt
##########
@@ -37,18 +37,20 @@ if (NOT DISABLE_PYTHON_SCRIPTING)
find_package(PythonLibs 3.0 REQUIRED)
endif()
- include_directories(${PYTHON_INCLUDE_DIR})
- include_directories(SYSTEM ../../thirdparty/pybind11/include)
-
- include_directories(python)
add_definitions(-DPYTHON_SUPPORT)
file(GLOB PY_SOURCES "python/*.cpp")
add_library(minifi-python-extensions STATIC ${PY_SOURCES})
-
+ target_include_directories(minifi-python-extensions SYSTEM PRIVATE ${PYTHON_INCLUDE_DIR})
+ target_include_directories(minifi-python-extensions SYSTEM PRIVATE ../../thirdparty/pybind11/include)
+ target_include_directories(minifi-python-extensions PRIVATE python)
target_link_libraries(minifi-python-extensions ${LIBMINIFI})
target_link_libraries(minifi-python-extensions ${PYTHON_LIBRARIES})
+ target_compile_options(minifi-python-extensions PUBLIC $<$<CXX_COMPILER_ID:Clang>:-fsized-deallocation>)
Review comment:
just out of curiosity, why is this needed?
##########
File path: linux.sh
##########
@@ -16,42 +16,6 @@
# specific language governing permissions and limitations
# under the License.
verify_gcc_enable(){
- feature="$1"
- if [ "$feature" = "BUSTACHE_ENABLED" ]; then
- if (( COMPILER_MAJOR == 6 && COMPILER_MINOR >= 3 && COMPILER_REVISION >= 1 )); then
- echo "true"
- elif (( COMPILER_MAJOR > 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "EXECUTE_SCRIPT_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "KAFKA_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- elif [ "$feature" = "PCAP_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- else
- echo "true"
- fi
+ #feature="$1"
Review comment:
can this line be removed?
##########
File path: README.md
##########
@@ -154,45 +147,22 @@ and rebuild.
* libgps-dev -- Required if building libGPS support
* Zlib headers
-#### CentOS 6
+#### CentOS 7
-Additional environmental preparations are required for CentOS 6 support. Before
+Additional environmental preparations are required for CentOS 7 support. Before
building, install and enable the devtoolset-6 SCL:
Review comment:
```suggestion
building, install and enable the devtoolset-10 SCL:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674761792
##########
File path: extensions/script/CMakeLists.txt
##########
@@ -37,18 +37,20 @@ if (NOT DISABLE_PYTHON_SCRIPTING)
find_package(PythonLibs 3.0 REQUIRED)
endif()
- include_directories(${PYTHON_INCLUDE_DIR})
- include_directories(SYSTEM ../../thirdparty/pybind11/include)
-
- include_directories(python)
add_definitions(-DPYTHON_SUPPORT)
file(GLOB PY_SOURCES "python/*.cpp")
add_library(minifi-python-extensions STATIC ${PY_SOURCES})
-
+ target_include_directories(minifi-python-extensions SYSTEM PRIVATE ${PYTHON_INCLUDE_DIR})
+ target_include_directories(minifi-python-extensions SYSTEM PRIVATE ../../thirdparty/pybind11/include)
+ target_include_directories(minifi-python-extensions PRIVATE python)
target_link_libraries(minifi-python-extensions ${LIBMINIFI})
target_link_libraries(minifi-python-extensions ${PYTHON_LIBRARIES})
+ target_compile_options(minifi-python-extensions PUBLIC $<$<CXX_COMPILER_ID:Clang>:-fsized-deallocation>)
Review comment:
Due to the error mentioned in this PR: https://github.com/pybind/pybind11/pull/1957
Since pybind11 is now updated and they included a workaround in their codebase, I will remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673988752
##########
File path: .github/workflows/ci.yml
##########
@@ -228,39 +151,18 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. && cmake --build . --parallel 4
- - name: test
- run: cd build && make test ARGS="--timeout 300 -j8 --output-on-failure"
- ubuntu_16_04_all:
- name: "ubuntu-16.04-all"
- runs-on: ubuntu-16.04
- timeout-minutes: 90
- steps:
- - id: checkout
- uses: actions/checkout@v2
- - id: cache
- uses: actions/cache@v2
- with:
- path: ~/.ccache
- key: ubuntu-16.04-all-ccache-${{github.ref}}-${{github.sha}}
- restore-keys: |
- ubuntu-16.04-all-ccache-${{github.ref}}-
- ubuntu-16.04-all-ccache-refs/heads/main-
- - id: install_deps
run: |
- sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
- sudo apt update
- sudo apt install -y ccache openjdk-8-jdk maven libusb-1.0-0-dev libpng12-dev libgps-dev libsqliteodbc
- sudo ln -s /usr/lib/x86_64-linux-gnu/odbc/libsqlite3odbc.so /usr/lib/x86_64-linux-gnu/libsqlite3odbc.so
- echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- - name: build
- run: sudo mount tmpfs -t tmpfs /tmp && ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DENABLE_NANOFI=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_MQTT=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DENABLE_AZURE=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=clang
+ export CXX=clang++
+ cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
+ cmake --build . --parallel $(($(nproc) * 2))
Review comment:
What the reason for $(nproc) * 2 instead of just $(nproc)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674408499
##########
File path: .github/workflows/ci.yml
##########
@@ -66,9 +31,12 @@ jobs:
sudo xcode-select -switch /Applications/Xcode_12.app
- name: build
run: |
- export PATH="/usr/local/opt/lua@5.3/lib:/usr/local/opt/lua@5.3/include:/usr/local/opt/lua@5.3/bin:$PATH"
+ export PATH="/usr/local/opt/lua@5.3/lib:/usr/local/opt/lua@5.3/include:/usr/local/opt/lua@5.3/bin:/usr/local/opt/flex/bin:$PATH"
Review comment:
https://github.com/apache/nifi-minifi-cpp/runs/3126854791#step:4:447
##########
File path: extensions/expression-language/Expression.cpp
##########
@@ -628,7 +632,7 @@ Value expr_toDate(const std::vector<Value> &args) {
auto arg_0 = args[0].asString();
std::istringstream arg_s { arg_0 };
date::sys_time<std::chrono::milliseconds> t;
- arg_s >> date::parse(args[1].asString(), t);
+ date::from_stream(arg_s, args[1].asString().c_str(), t);
Review comment:
These lines should be equivalent, but the first one failed to compile on MSVC for some reason. The error message was not very helpful to me.
https://github.com/szaszm/nifi-minifi-cpp/runs/3024143793?check_suite_focus=true#step:7:16514
##########
File path: libminifi/test/unit/MemoryUsageTest.cpp
##########
@@ -24,14 +25,16 @@
TEST_CASE("Test Physical memory usage", "[testphysicalmemoryusage]") {
constexpr bool cout_enabled = true;
std::vector<char> v(30000000);
+ // use v to prevent recent compilers from optimizing out the allocation
Review comment:
It likely has something to do with `std::vector` being constexpr-ready in C++20 and I assume the optimizer executes such code at compile-time regardless of `constexpr` at the usage site. Printing the contents to the console keeps this code working for now, but there is nothing preventing future compilers from seeing through the trick here.
##########
File path: .github/workflows/ci.yml
##########
@@ -66,9 +31,12 @@ jobs:
sudo xcode-select -switch /Applications/Xcode_12.app
- name: build
run: |
- export PATH="/usr/local/opt/lua@5.3/lib:/usr/local/opt/lua@5.3/include:/usr/local/opt/lua@5.3/bin:$PATH"
+ export PATH="/usr/local/opt/lua@5.3/lib:/usr/local/opt/lua@5.3/include:/usr/local/opt/lua@5.3/bin:/usr/local/opt/flex/bin:$PATH"
export PKG_CONFIG_PATH="/usr/local/opt/lua@5.3/lib/pkgconfig"
- ./bootstrap.sh -e -t && cd build && cmake -DCMAKE_BUILD_TYPE=Release -DENABLE_LUA_SCRIPTING=1 -DENABLE_SQL=ON -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_RULE_MESSAGES=OFF -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ export LDFLAGS="-L/usr/local/opt/flex/lib"
+ export CPPFLAGS="-I/usr/local/opt/flex/include"
+ # CPPFLAGS are not recognized by cmake, so we have to force them to CFLAGS and CXXFLAGS to have flex 2.6 working
Review comment:
CPP here: **C** **P**re**p**rocessor
CPPFLAGS is normally the place where you want to put your include paths and definitions. Not to be confused with CXXFLAGS, which are C++ compiler flags the same way CFLAGS are C compiler flags.
In most cases, including ours, cpp is called by the compiler driver, but it's possible to call it separately, similarly to as and ld.
##########
File path: thirdparty/aws-sdk-cpp/core-wstr-fix.patch
##########
@@ -0,0 +1,20 @@
+diff -ruN orig/aws-cpp-sdk-core/source/utils/crypto/bcrypt/CryptoImpl.cpp patched/aws-cpp-sdk-core/source/utils/crypto/bcrypt/CryptoImpl.cpp
+--- orig/aws-cpp-sdk-core/source/utils/crypto/bcrypt/CryptoImpl.cpp 2021-07-08 23:54:17.936278552 +0200
++++ patched/aws-cpp-sdk-core/source/utils/crypto/bcrypt/CryptoImpl.cpp 2021-07-08 23:57:13.702620555 +0200
+@@ -11,6 +11,7 @@
+ #include <aws/core/utils/Outcome.h>
+ #include <aws/core/utils/crypto/Hash.h>
+ #include <aws/core/utils/HashingUtils.h>
++#include <aws/core/utils/StringUtils.h>
+ #include <atomic>
+ #include <bcrypt.h>
+ #include <winternl.h>
+@@ -126,7 +127,7 @@
+ NTSTATUS status = BCryptOpenAlgorithmProvider(&m_algorithmHandle, algorithmName, MS_PRIMITIVE_PROVIDER, isHMAC ? BCRYPT_ALG_HANDLE_HMAC_FLAG : 0);
+ if (!NT_SUCCESS(status))
+ {
+- AWS_LOGSTREAM_ERROR(logTag, "Failed initializing BCryptOpenAlgorithmProvider for " << algorithmName);
++ AWS_LOGSTREAM_ERROR(logTag, "Failed initializing BCryptOpenAlgorithmProvider for " << StringUtils::FromWString(algorithmName));
Review comment:
`algorithmName` is a `std::wstring` and at least in C++20 mode, this macro piped the second argument to a `std::ostringstream`, whose CharT = char. This resulted in a compilation error, but aws stringutils had an easy fix.
I didn't submit this change upstream because I wasn't able to upgrade aws libs to verify that my change is still necessary and working with the latest version.
##########
File path: extensions/systemd/tests/ConsumeJournaldTest.cpp
##########
@@ -176,7 +176,7 @@ TEST_CASE("ConsumeJournald", "[consumejournald]") {
libwrapper_observer->journal.emplace_back("systemd", "Mounted /boot.", 1);
plan->runCurrentProcessor();
REQUIRE("6" == get_cursor_position());
- REQUIRE("2021-04-15T17:17:09.123456000+00:00 test-pc systemd[1]: Mounted /boot." == plan->getContent(plan->getCurrentFlowFile()));
+ REQUIRE("2021-04-15T17:17:09.123456+00:00 test-pc systemd[1]: Mounted /boot." == plan->getContent(plan->getCurrentFlowFile()));
Review comment:
On Debian with clang-11, this is the date format that came out of the old code, making the string comparison and the test fail. I decided to restrict the precision to microseconds for all platforms. That's why ConsumeJournald is changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673843606
##########
File path: .github/workflows/ci.yml
##########
@@ -196,7 +110,13 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1
+ run: |
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=gcc-11
+ export CXX=g++-11
+ cmake -DUSE_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
Review comment:
I don't think it's strictly necessary to have all extensions with both compilers, but it doesn't hurt either, so I will extend the ubuntu 20.04 job later today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674784049
##########
File path: linux.sh
##########
@@ -16,42 +16,6 @@
# specific language governing permissions and limitations
# under the License.
verify_gcc_enable(){
- feature="$1"
- if [ "$feature" = "BUSTACHE_ENABLED" ]; then
- if (( COMPILER_MAJOR == 6 && COMPILER_MINOR >= 3 && COMPILER_REVISION >= 1 )); then
- echo "true"
- elif (( COMPILER_MAJOR > 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "EXECUTE_SCRIPT_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "KAFKA_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- elif [ "$feature" = "PCAP_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- else
- echo "true"
- fi
+ #feature="$1"
Review comment:
Makes sense, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r675431209
##########
File path: extensions/expression-language/Expression.cpp
##########
@@ -25,6 +25,10 @@
#include <functional>
#include <string>
+#ifdef GetObject
+#undef GetObject
+#endif
Review comment:
They are gone without issues, I'll reintroduce the necessary ones in the C++20 followup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673834998
##########
File path: debian.sh
##########
@@ -21,10 +21,17 @@ verify_enable_platform(){
verify_gcc_enable "$feature"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
Could this be removed here as well?
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
Could this be removed?
##########
File path: .github/workflows/ci.yml
##########
@@ -196,7 +110,13 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4 VERBOSE=1
+ run: |
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=gcc-11
+ export CXX=g++-11
+ cmake -DUSE_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQL=ON -DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
Review comment:
With the ubuntu_16_04_all job removed we don't have a job building all extensions with GCC, only with clang. We could add those in the ubuntu_20_04 job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674763868
##########
File path: extensions/expression-language/Expression.cpp
##########
@@ -25,6 +25,10 @@
#include <functional>
#include <string>
+#ifdef GetObject
+#undef GetObject
+#endif
Review comment:
There are more lines here. I started getting these errors all the time for some reason. It seemed that even standard headers somehow got this defined. Not sure what triggered the explosion of these definitions, but I added one undef near every rapidjson include.
I'll try removing them to see if they occur in C++17 mode or are specific to /std:c++latest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673860139
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
Given this, are you OK with leaving them both as is or do you still want a change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674766329
##########
File path: linux.sh
##########
@@ -16,42 +16,6 @@
# specific language governing permissions and limitations
# under the License.
verify_gcc_enable(){
- feature="$1"
- if [ "$feature" = "BUSTACHE_ENABLED" ]; then
- if (( COMPILER_MAJOR == 6 && COMPILER_MINOR >= 3 && COMPILER_REVISION >= 1 )); then
- echo "true"
- elif (( COMPILER_MAJOR > 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "EXECUTE_SCRIPT_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 6 )); then
- echo "true"
- else
- echo "false"
- fi
- elif [ "$feature" = "KAFKA_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- elif [ "$feature" = "PCAP_ENABLED" ]; then
- if (( COMPILER_MAJOR >= 4 )); then
- if (( COMPILER_MAJOR > 4 || COMPILER_MINOR >= 8 )); then
- echo "true"
- else
- echo "false"
- fi
- else
- echo "false"
- fi
- else
- echo "true"
- fi
+ #feature="$1"
Review comment:
My intention was to leave it there as a documentation for future extension of this function, because it might not be obvious that the first argument is the feature name. Currently all the features are working on all of the supported compilers, due to dropping old ones, but if this ever changes, here is the place to put those checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674044805
##########
File path: .github/workflows/ci.yml
##########
@@ -228,39 +151,18 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. && cmake --build . --parallel 4
- - name: test
- run: cd build && make test ARGS="--timeout 300 -j8 --output-on-failure"
- ubuntu_16_04_all:
- name: "ubuntu-16.04-all"
- runs-on: ubuntu-16.04
- timeout-minutes: 90
- steps:
- - id: checkout
- uses: actions/checkout@v2
- - id: cache
- uses: actions/cache@v2
- with:
- path: ~/.ccache
- key: ubuntu-16.04-all-ccache-${{github.ref}}-${{github.sha}}
- restore-keys: |
- ubuntu-16.04-all-ccache-${{github.ref}}-
- ubuntu-16.04-all-ccache-refs/heads/main-
- - id: install_deps
run: |
- sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
- sudo apt update
- sudo apt install -y ccache openjdk-8-jdk maven libusb-1.0-0-dev libpng12-dev libgps-dev libsqliteodbc
- sudo ln -s /usr/lib/x86_64-linux-gnu/odbc/libsqlite3odbc.so /usr/lib/x86_64-linux-gnu/libsqlite3odbc.so
- echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- - name: build
- run: sudo mount tmpfs -t tmpfs /tmp && ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DENABLE_NANOFI=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_MQTT=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DENABLE_AZURE=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=clang
+ export CXX=clang++
+ cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
+ cmake --build . --parallel $(($(nproc) * 2))
Review comment:
It might improve CPU utilization as not all make threads use the CPU all the time. For example, when one thread is downloading a tarball, another compilation thread could still use that core, as IO doesn't need too much cpu. And even when all threads are compiler/linker threads, fully utilizing the CPU, the performance degradation is negligible compared to doing the same over $(nproc) threads.
A drawback of going higher is that more threads need more memory, and if we run out, swapping kills compilation times. Linking is especially memory-heavy. On Gentoo, 32C/64T 128G RAM, when I run 4 portage compilation tasks on 32 threads each, I tend to test the limits of even 128 GB of RAM, and some cores are still idling the majority of the time, even after going 2x the threads.
The general advice from a few years ago was ranging from N, N+1, N+2 to 2*N make jobs per cpu thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r673860139
##########
File path: arch.sh
##########
@@ -21,7 +21,8 @@ verify_enable_platform(){
verify_gcc_enable "${feature}"
}
add_os_flags() {
- CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
+ #CMAKE_BUILD_COMMAND="${CMAKE_BUILD_COMMAND}"
Review comment:
Given this, are you OK with leaving it as is or do you still want a change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni closed pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
adamdebreceni closed pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674058041
##########
File path: .github/workflows/ci.yml
##########
@@ -228,39 +151,18 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. && cmake --build . --parallel 4
- - name: test
- run: cd build && make test ARGS="--timeout 300 -j8 --output-on-failure"
- ubuntu_16_04_all:
- name: "ubuntu-16.04-all"
- runs-on: ubuntu-16.04
- timeout-minutes: 90
- steps:
- - id: checkout
- uses: actions/checkout@v2
- - id: cache
- uses: actions/cache@v2
- with:
- path: ~/.ccache
- key: ubuntu-16.04-all-ccache-${{github.ref}}-${{github.sha}}
- restore-keys: |
- ubuntu-16.04-all-ccache-${{github.ref}}-
- ubuntu-16.04-all-ccache-refs/heads/main-
- - id: install_deps
run: |
- sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
- sudo apt update
- sudo apt install -y ccache openjdk-8-jdk maven libusb-1.0-0-dev libpng12-dev libgps-dev libsqliteodbc
- sudo ln -s /usr/lib/x86_64-linux-gnu/odbc/libsqlite3odbc.so /usr/lib/x86_64-linux-gnu/libsqlite3odbc.so
- echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- - name: build
- run: sudo mount tmpfs -t tmpfs /tmp && ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DENABLE_NANOFI=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_MQTT=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DENABLE_AZURE=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=clang
+ export CXX=clang++
+ cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
+ cmake --build . --parallel $(($(nproc) * 2))
Review comment:
I reverted to $(nproc), because we already ran into problems: the ubuntu 20.04 job timed out. I think it's more likely to be because of the number of extensions and the fact that these executors are not great, but it's worth a try.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674047944
##########
File path: .github/workflows/ci.yml
##########
@@ -228,39 +151,18 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. && cmake --build . --parallel 4
- - name: test
- run: cd build && make test ARGS="--timeout 300 -j8 --output-on-failure"
- ubuntu_16_04_all:
- name: "ubuntu-16.04-all"
- runs-on: ubuntu-16.04
- timeout-minutes: 90
- steps:
- - id: checkout
- uses: actions/checkout@v2
- - id: cache
- uses: actions/cache@v2
- with:
- path: ~/.ccache
- key: ubuntu-16.04-all-ccache-${{github.ref}}-${{github.sha}}
- restore-keys: |
- ubuntu-16.04-all-ccache-${{github.ref}}-
- ubuntu-16.04-all-ccache-refs/heads/main-
- - id: install_deps
run: |
- sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
- sudo apt update
- sudo apt install -y ccache openjdk-8-jdk maven libusb-1.0-0-dev libpng12-dev libgps-dev libsqliteodbc
- sudo ln -s /usr/lib/x86_64-linux-gnu/odbc/libsqlite3odbc.so /usr/lib/x86_64-linux-gnu/libsqlite3odbc.so
- echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- - name: build
- run: sudo mount tmpfs -t tmpfs /tmp && ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DENABLE_NANOFI=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_MQTT=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DENABLE_AZURE=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=clang
+ export CXX=clang++
+ cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
+ cmake --build . --parallel $(($(nproc) * 2))
Review comment:
I can revert to $(nproc) if you prefer it that way. The main motivation was getting rid of hardcoded core numbers to be able to utilize more powerful executors, and I thought if I'm already changing it, might as well make sure that we maximize CPU utilization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674053134
##########
File path: .github/workflows/ci.yml
##########
@@ -228,39 +151,18 @@ jobs:
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- name: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ .. && cmake --build . --parallel 4
- - name: test
- run: cd build && make test ARGS="--timeout 300 -j8 --output-on-failure"
- ubuntu_16_04_all:
- name: "ubuntu-16.04-all"
- runs-on: ubuntu-16.04
- timeout-minutes: 90
- steps:
- - id: checkout
- uses: actions/checkout@v2
- - id: cache
- uses: actions/cache@v2
- with:
- path: ~/.ccache
- key: ubuntu-16.04-all-ccache-${{github.ref}}-${{github.sha}}
- restore-keys: |
- ubuntu-16.04-all-ccache-${{github.ref}}-
- ubuntu-16.04-all-ccache-refs/heads/main-
- - id: install_deps
run: |
- sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
- sudo apt update
- sudo apt install -y ccache openjdk-8-jdk maven libusb-1.0-0-dev libpng12-dev libgps-dev libsqliteodbc
- sudo ln -s /usr/lib/x86_64-linux-gnu/odbc/libsqlite3odbc.so /usr/lib/x86_64-linux-gnu/libsqlite3odbc.so
- echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null
- - name: build
- run: sudo mount tmpfs -t tmpfs /tmp && ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS= -DENABLE_NANOFI=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_MQTT=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DENABLE_AZURE=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && cmake --build . --parallel 4
+ ./bootstrap.sh -e -t
+ cd build
+ export CC=clang
+ export CXX=clang++
+ cmake -DUSE_SHARED_LIBS= -DCMAKE_BUILD_TYPE=Release -DENABLE_NANOFI=ON -DENABLE_JNI=ON -DENABLE_SENSORS=ON -DENABLE_OPENWSMAN=ON -DENABLE_OPENCV=ON -DENABLE_MQTT=ON -DENABLE_GPS=ON -DENABLE_USB_CAMERA=ON -DENABLE_LIBRDKAFKA=ON -DENABLE_OPC=ON -DENABLE_SFTP=ON -DENABLE_COAP=ON -DENABLE_PYTHON=ON -DENABLE_SQL=ON -DENABLE_AWS=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON ..
+ cmake --build . --parallel $(($(nproc) * 2))
Review comment:
I support removing the hardcoded core numbers, I wasn't sure if the build can utilize core numbers above $(nproc). If it's okay and we do not run into any problems I'm okay with keeping the $(nproc) * 2 job count.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1133: MINIFICPP-1425 upgrade compilers on linux and mac, use C++17, prepare for C++20
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1133#discussion_r674764445
##########
File path: libminifi/test/Utils.h
##########
@@ -16,8 +16,16 @@
*/
#pragma once
+#ifdef GetObject
+#undef GetObject
+#endif
+
#include <string>
#include <utility>
+
+#ifdef GetObject
+#undef GetObject
+#endif
Review comment:
Can't remember the reason. I'll remove all of them and add them back one by one on related windows CI failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org