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/02/16 17:14:16 UTC
[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request #1005: MINIFICPP-1496 Remove ASAN and JNI from the list of extensions
fgerlits opened a new pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005
https://issues.apache.org/jira/browse/MINIFICPP-1496
Remove ASAN and JNI from the list of extensions in bootstrap.sh
ASAN_BUILD and ENABLE_JNI should be handled like the the other Build Options
(like USE_SHARED_LIBS and SKIP_TESTS) and not like extensions. In particular,
if you select option 2, "Enable all extensions", these should not be enabled.
---
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:
- [ ] 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)?
- [ ] 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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
arpadboda commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r579053249
##########
File path: bootstrap.sh
##########
@@ -320,12 +320,11 @@ add_dependency OPC_ENABLED "mbedtls"
USE_SHARED_LIBS=${TRUE}
TESTS_DISABLED=${FALSE}
+ASAN_ENABLED=${FALSE}
## name, default, values
add_multi_option BUILD_PROFILE "RelWithDebInfo" "RelWithDebInfo" "Debug" "MinSizeRel" "Release"
-add_disabled_option ASAN_ENABLED ${FALSE} "ASAN_BUILD"
Review comment:
Agreed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r578977079
##########
File path: bootstrap.sh
##########
@@ -320,12 +320,11 @@ add_dependency OPC_ENABLED "mbedtls"
USE_SHARED_LIBS=${TRUE}
TESTS_DISABLED=${FALSE}
+ASAN_ENABLED=${FALSE}
## name, default, values
add_multi_option BUILD_PROFILE "RelWithDebInfo" "RelWithDebInfo" "Debug" "MinSizeRel" "Release"
-add_disabled_option ASAN_ENABLED ${FALSE} "ASAN_BUILD"
Review comment:
indeed it seems, it has to be explicitly added to `save_state`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r578977079
##########
File path: bootstrap.sh
##########
@@ -320,12 +320,11 @@ add_dependency OPC_ENABLED "mbedtls"
USE_SHARED_LIBS=${TRUE}
TESTS_DISABLED=${FALSE}
+ASAN_ENABLED=${FALSE}
## name, default, values
add_multi_option BUILD_PROFILE "RelWithDebInfo" "RelWithDebInfo" "Debug" "MinSizeRel" "Release"
-add_disabled_option ASAN_ENABLED ${FALSE} "ASAN_BUILD"
Review comment:
nice, yes indeed, it has to be explicitly added to `save_state`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1005: MINIFICPP-1496 Remove ASAN and JNI from the list of extensions
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r577509405
##########
File path: bootstrap.sh
##########
@@ -318,14 +315,16 @@ add_dependency TENSORFLOW_ENABLED "tensorflow"
add_disabled_option OPC_ENABLED ${FALSE} "ENABLE_OPC"
add_dependency OPC_ENABLED "mbedtls"
+JNI_ENABLED=${FALSE}
+add_dependency JNI_ENABLED "jnibuild"
Review comment:
it seems to me, that JNI_ENABLED no longer being added to OPTIONS affects the platform specific dependency installer scripts, specifically they iterate over the OPTIONS and then install their dependencies, could this cause that java will no longer be installed even if we enable jni?
----------------------------------------------------------------
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.
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 #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r579051480
##########
File path: bootstrap.sh
##########
@@ -320,12 +320,11 @@ add_dependency OPC_ENABLED "mbedtls"
USE_SHARED_LIBS=${TRUE}
TESTS_DISABLED=${FALSE}
+ASAN_ENABLED=${FALSE}
## name, default, values
add_multi_option BUILD_PROFILE "RelWithDebInfo" "RelWithDebInfo" "Debug" "MinSizeRel" "Release"
-add_disabled_option ASAN_ENABLED ${FALSE} "ASAN_BUILD"
Review comment:
Indeed, it was missing from `bt_state`. Fixed in https://github.com/apache/nifi-minifi-cpp/pull/1005/commits/28970606a01063e4491bbb35ca4de5899e1cd865.
This bootstrap script is a bit of a mess, and should be refactored, but I think that should be done in a separate PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1005: MINIFICPP-1496 Remove ASAN and JNI from the list of extensions
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r577506574
##########
File path: bootstrap.sh
##########
@@ -318,14 +315,16 @@ add_dependency TENSORFLOW_ENABLED "tensorflow"
add_disabled_option OPC_ENABLED ${FALSE} "ENABLE_OPC"
add_dependency OPC_ENABLED "mbedtls"
+JNI_ENABLED=${FALSE}
+add_dependency JNI_ENABLED "jnibuild"
Review comment:
it seems to me, that `JNI_ENABLED` no longer being added to `OPTIONS` affects the platform specific dependency installer scripts, specifically they iterate over the `OPTIONS` and then install their dependencies, could this cause that java will no longer be installed even if we enable jni?
----------------------------------------------------------------
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.
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 #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r577715377
##########
File path: bootstrap.sh
##########
@@ -318,14 +315,16 @@ add_dependency TENSORFLOW_ENABLED "tensorflow"
add_disabled_option OPC_ENABLED ${FALSE} "ENABLE_OPC"
add_dependency OPC_ENABLED "mbedtls"
+JNI_ENABLED=${FALSE}
+add_dependency JNI_ENABLED "jnibuild"
Review comment:
Yes, you're right. I have reverted the JNI part of the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
arpadboda commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r578809604
##########
File path: bootstrap.sh
##########
@@ -320,12 +320,11 @@ add_dependency OPC_ENABLED "mbedtls"
USE_SHARED_LIBS=${TRUE}
TESTS_DISABLED=${FALSE}
+ASAN_ENABLED=${FALSE}
## name, default, values
add_multi_option BUILD_PROFILE "RelWithDebInfo" "RelWithDebInfo" "Debug" "MinSizeRel" "Release"
-add_disabled_option ASAN_ENABLED ${FALSE} "ASAN_BUILD"
Review comment:
Are you sure this could be removed?
For eg., settings are persisted without this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1005: MINIFICPP-1496 Remove ASAN and JNI from the list of extensions
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005#discussion_r577506574
##########
File path: bootstrap.sh
##########
@@ -318,14 +315,16 @@ add_dependency TENSORFLOW_ENABLED "tensorflow"
add_disabled_option OPC_ENABLED ${FALSE} "ENABLE_OPC"
add_dependency OPC_ENABLED "mbedtls"
+JNI_ENABLED=${FALSE}
+add_dependency JNI_ENABLED "jnibuild"
Review comment:
it seems to me, that `JNI_ENABLED` no longer being added to `OPTIONS` affects the platform specific dependency installer scripts, specifically they iterate over the `OPTIONS` and then installer their dependencies, could this cause that java will no longer be installed even if we enable jni?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni closed pull request #1005: MINIFICPP-1496 "Enable all extensions" in bootstrap.sh should not enable ASAN_BUILD
Posted by GitBox <gi...@apache.org>.
adamdebreceni closed pull request #1005:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org