szaszm commented on code in PR #1776:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1776#discussion_r1587428695
##########
.github/workflows/ci.yml:
##########
@@ -72,35 +72,34 @@ jobs:
uses: actions/cache/restore@v4
with:
path: ${{ env.CCACHE_DIR }}
- key: macos-xcode-ccache-${{github.ref}}-${{github.sha}}
+ key: macos-arm-xcode-ccache-${{github.ref}}-${{github.sha}}
restore-keys: |
- macos-xcode-ccache-${{github.ref}}-
- macos-xcode-ccache-refs/heads/main-
+ macos-arm-xcode-ccache-${{github.ref}}-
+ macos-arm-xcode-ccache-refs/heads/main-
- id: install_dependencies
run: |
# Skip brew update until
https://github.com/actions/setup-python/issues/577 is fixed
- # brew update
- HOMEBREW_NO_AUTO_UPDATE=1 brew install ossp-uuid bison flex ccache
sqliteodbc automake autoconf ninja
+ brew update
Review Comment:
the comment above this line should be removed
##########
docker/centos/Dockerfile:
##########
Review Comment:
Is there anything that necessitates the removal of centos7, while it still
works? Until we drop support, it can stick around IMO.
##########
docker/focal/Dockerfile:
##########
Review Comment:
I don't think we should drop focal just yet.
##########
libminifi/src/utils/Cron.cpp:
##########
@@ -50,7 +50,7 @@ namespace {
// the month parsing with '%b' and the weekday parsing with '%a' is
case-sensitive in gcc11
// This has been fixed in gcc12.2
std::stringstream getCaseInsensitiveCStream(const std::string& str) {
-#if defined(__GNUC__) && (__GNUC__ < 12 || (__GNUC__ == 12 && __GNUC_MINOR__ <
2))
+#if defined(__GNUC__) && (__GNUC__ < 12 || (__GNUC__ == 12 && __GNUC_MINOR__ <
3))
Review Comment:
The comment above these lines needs to be checked and updated. It states
this is fixed in 12.2, so why do we include the workaround in 12.2?
##########
docker/rockylinux/Dockerfile:
##########
@@ -40,26 +40,30 @@ COPY . ${MINIFI_BASE_DIR}
# Install the system dependencies needed for a build
# gpsd-devel and ccache are in EPEL
-RUN dnf -y install epel-release && dnf -y install sudo git which make
libarchive ccache ca-certificates perl && \
+RUN dnf -y install epel-release && dnf -y install gcc-toolset-12 sudo git
which make libarchive ccache ca-certificates perl patch bison flex libtool
cmake && \
+ if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_ALL=ON"; then dnf -y
--enablerepo=devel install gpsd-devel libpng-devel libusbx-devel python3-devel
java-1.8.0-openjdk maven libpcap-devel; fi && \
if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_GPS=ON"; then dnf -y install
gpsd-devel; fi && \
if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_JNI=ON"; then dnf -y install
java-1.8.0-openjdk maven; fi && \
- if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_PCAP=ON"; then dnf -y install
libpcap-devel; fi && \
+ if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_PCAP=ON"; then dnf -y
--enablerepo=devel install libpcap-devel; fi && \
if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_USB_CAMERA=ON"; then dnf -y
install libpng-devel libusbx-devel; fi && \
if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_PYTHON_SCRIPTING=ON"; then dnf
-y install python3-devel; fi && \
if echo "$MINIFI_OPTIONS" | grep -q "ENABLE_SFTP=ON" && [
"${DOCKER_SKIP_TESTS}" == "OFF" ]; then dnf -y install java-1.8.0-openjdk
maven; fi
RUN cd $MINIFI_BASE_DIR && \
- ./bootstrap.sh -t && \
ln -s /usr/bin/ccache /usr/lib64/ccache/c++
# Setup minificpp user
RUN groupadd -g ${GID} ${USER} && useradd -g ${GID} ${USER} && \
chown -R ${USER}:${USER} ${MINIFI_BASE_DIR}
+RUN ls -lah ${MINIFI_BASE_DIR}/docker
+RUN patch -p1
/opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/basic_string.tcc
${MINIFI_BASE_DIR}/thirdparty/libstdc++/avoid_bogus_Wrestrict_PR105651.patch
Review Comment:
The `ls` doesn't seem necessary. The patch deserves a comment IMO, to draw
attention to the fact that we're patching the system standard headers.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]