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/10/27 16:10:47 UTC
[GitHub] [nifi-minifi-cpp] szaszm opened a new pull request #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
szaszm opened a new pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207
Azure-sdk dependencies were not respected, because of a mismatch between
cmake target names (-external vs -external-build). After fixing this
issue, the build failed, because the azure-sdk cmake scripts tried to
install stuff to /usr/local, which they thankfully couldn't.
Apparently the azure sdk third party was used directly from its
source/build directory, so it had no install dir and defaulted to
/usr/local. I adapted it to look like other third parties, with an
install directory under our build/thirdparty dir, where the package can
install and we can consume its artifacts.
https://issues.apache.org/jira/browse/MINIFICPP-1674
---
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] lordgamez commented on a change in pull request #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r738978605
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
I'm okay with the current change, I don't think we need to revert to BUILD_IN_SOURCE
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r737667969
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
I didn't know that the -build suffix had actual functionality, so thanks for clarifying this. I don't know then why the dependencies were not respected, but it seems to work now.
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r737648134
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
Was this a workaround?
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r738195515
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
It seems this change caused some problems with the Windows build that needs to be checked. I rerun the build locally to see if it was some transient failure, but it fails consistently, but it needs to be investigated what the root cause of this is.
```FileTracker : error FTK1011: could not create the new file tracking log file: D:\a\nifi-minifi-cpp\nifi-minifi-cpp\build\azure-sdk-cpp-external-prefix\src\azure-sdk-cpp-external-build\sdk\keyvault\azure-security-keyvault-certificates\azure-security-keyvault-certificates.dir\Release\azure-se.9DB92436.tlog\CL-cl_original.6536.write.1.tlog. The system cannot find the path specified.```
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r737647120
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
BUILD_IN_SOURCE and STEP_TARGETS parameters can be also removed. They were also part of this "workaround"
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r738782522
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
I think this is a MAX_PATH issue. The build failed for me too locally, but when I moved the repo to a short path (`C:\a\m`), it worked fine. Probably related to removing BUILD_IN_SOURCE. A few ideas:
- rename the cmake target to something shorter, like "asdkext" to save a few characters on the ExternalProject build dir names
- move the build directory outside GITHUB_WORKSPACE (== `D:\a\nifi-minifi-cpp\nifi-minifi-cpp`). I don't want to go more than one level up.
- revert to BUILD_IN_SOURCE
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
adamdebreceni closed pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207
--
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 #1207: MINIFICPP-1674 fix azure-sdk target name, use install dir
Posted by GitBox <gi...@apache.org>.
lordgamez commented on a change in pull request #1207:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1207#discussion_r737663971
##########
File path: cmake/BundledAzureSdkCpp.cmake
##########
@@ -56,6 +63,7 @@ function(use_bundled_libazure SOURCE_DIR BINARY_DIR)
URL_HASH "SHA256=d4e80ea5e786dc689ddd04825d97ab91f5e1ef2787fa88a3d5ee00f0b820433f"
BUILD_IN_SOURCE true
Review comment:
I only remember that the first time Azure was introduced the install step wasn't working for our use case for some reason and that's why only the build step was executed and we depended on *-build target as a workaround. Since then it has changed as we updated it a few times so it may have been fixed so these parts can be removed.
--
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