[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-13 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1165592135


##
extensions/aws/CMakeLists.txt:
##
@@ -34,6 +34,9 @@ add_library(minifi-aws SHARED ${SOURCES})
 target_link_libraries(minifi-aws PUBLIC ${LIBMINIFI} Threads::Threads)
 
 target_wholearchive_library_private(minifi-aws AWS::aws-cpp-sdk-s3)
+if(NOT CMAKE_SYSTEM_PROCESSOR MATCHES "(x86)|(X86)")

Review Comment:
   I actually made the change before I read Martin's comment, I thought it 
would be better to link the library on any non-x86 architecture as it may be 
needed there as well. But as I checked 
https://stackoverflow.com/questions/70475665/what-are-the-possible-values-of-cmake-system-processor
 there are more possibilities than I thought, like you said AMD64 also should 
have been included on windows. So it may be better to only concentrate on arm64 
possibilities and we'll see in the future if we want to support more. I changed 
it to match only on arm64, ARM64, aarch64 and armv8 in 
4037275310298711c7f949011b3e09e32a10ccdb (From what I gathered the overall 
arm64 architecture possiblities are arm64, ARM64, aarch64, aarch64_be, armv8b, 
armv8l on Windows, Linux and MacOS)



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-13 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1165592135


##
extensions/aws/CMakeLists.txt:
##
@@ -34,6 +34,9 @@ add_library(minifi-aws SHARED ${SOURCES})
 target_link_libraries(minifi-aws PUBLIC ${LIBMINIFI} Threads::Threads)
 
 target_wholearchive_library_private(minifi-aws AWS::aws-cpp-sdk-s3)
+if(NOT CMAKE_SYSTEM_PROCESSOR MATCHES "(x86)|(X86)")

Review Comment:
   I actually made the change before I read Martin's comment, I thought it 
would be better to link the library on any non-x86 architecture as it may be 
needed there as well. But as I checked 
https://stackoverflow.com/questions/70475665/what-are-the-possible-values-of-cmake-system-processor
 there are more possibilities than I thought, like you said AMD64 also should 
have been included on windows. So it may be better to only concentrate on arm64 
possibilities and we'll see in the future if we want to support more. I changed 
it to match only on arm64, ARM64, aarch64 and armv8 in 
4037275310298711c7f949011b3e09e32a10ccdb



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-13 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1165209315


##
extensions/aws/CMakeLists.txt:
##


Review Comment:
   Restricted linkage and removed test workaround in 
3ad1d51a4b678eed4550251c33f5a0a62d19d524



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-12 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1164391311


##
extensions/aws/CMakeLists.txt:
##


Review Comment:
   It seems that I was wrong as the linkage problem didn't originate from the 
aws crt library as I originally suspected. All the missing symbols can be 
resolved if we link the aws-checksums as a whole archive. I tested it on arm 
and it solves the issue, updated in ec32f02435dc9735994f7c6fb4ab364f34a19266



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-12 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1164391311


##
extensions/aws/CMakeLists.txt:
##


Review Comment:
   It seems that I was wrong as the linkage problem didn't originate from the 
aws crt library as I originally suspected. All the missing symbols can be 
resolved if we link the aws-checksums as a whole archive. I tested it on arm 
and it solves the issue, updated in 82e8074988ef5caf6b06e68140c3faae8ce0fd83



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1554: MINIFICPP-2058 Fix AWS extension link error on ARM64

2023-04-12 Thread via GitHub


lordgamez commented on code in PR #1554:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1554#discussion_r1163975274


##
extensions/aws/CMakeLists.txt:
##


Review Comment:
   There isn't much documentation on this, most examples I found in other 
repositories links all the resulting libraries to the client library. It also 
seemed that not only this single symbol was missing in the linking process, 
because when this was resolved there were other missing aws symbols from other 
libraries. Unfortunately this was the only way I could make it work on both 
ARM64 linux and on Windows as there were other alternative ways that did not 
work on Windows.



##
extensions/aws/CMakeLists.txt:
##


Review Comment:
   There isn't much documentation on this, most examples I found in other 
repositories linked all the resulting libraries to the client library. It also 
seemed that not only this single symbol was missing in the linking process, 
because when this was resolved there were other missing aws symbols from other 
libraries. Unfortunately this was the only way I could make it work on both 
ARM64 linux and on Windows as there were other alternative ways that did not 
work on Windows.



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org