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/21 10:59:40 UTC
[GitHub] [nifi-minifi-cpp] adamdebreceni opened a new pull request #1203: MINIFICPP-1672 - Configurable msi
adamdebreceni opened a new pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203
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:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] 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.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] 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.
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 #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r736429795
##########
File path: CMakeLists.txt
##########
@@ -808,14 +809,28 @@ else()
endif()
set(CPACK_PACKAGE_INSTALL_DIRECTORY "ApacheNiFiMiNiFi")
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
-set(CPACK_RDKAFKA_COMPONENT_INSTALL ON)
-set(CPACK_MQTT_COMPONENT_INSTALL ON)
+
+
+# !WARNING! we need to manually add to CPACK_COMPONENTS_ALL otherwise
+# all components specified in "install" directives are added
+# (including thirdparties like range-v3)
+
+list(APPEND CPACK_COMPONENTS_ALL bin)
+cpack_add_component(bin REQUIRED)
if(WIN32)
- set(CPACK_COMPONENTS_ALL bin conf minifijni tzdata)
+ list(APPEND CPACK_COMPONENTS_ALL tzdata)
+ cpack_add_component(tzdata)
else()
- set(CPACK_COMPONENTS_ALL bin conf minifijni)
+ list(APPEND CPACK_COMPONENTS_ALL conf)
+ cpack_add_component(conf REQUIRED)
endif()
Review comment:
Could you add some display names? I think it would look nicer in the generated MSI.
```c++
list(APPEND CPACK_COMPONENTS_ALL bin)
cpack_add_component(bin DISPLAY_NAME "MiNiFi C++ executables" REQUIRED)
if(WIN32)
list(APPEND CPACK_COMPONENTS_ALL tzdata)
cpack_add_component(tzdata DISPLAY_NAME "Timezone database for Expression Language")
else()
list(APPEND CPACK_COMPONENTS_ALL conf)
cpack_add_component(conf REQUIRED DISPLAY_NAME "Default configuration files")
endif()
```
--
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 #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r738172463
##########
File path: msi/WixWin.wsi
##########
@@ -113,6 +117,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
<Publish Dialog="MaintenanceWelcomeDlg" Control="Next" Event="NewDialog" Value="MaintenanceTypeDlg">1</Publish>
+ <Publish Dialog="MaintenanceTypeDlg" Control="ChangeButton" Event="NewDialog" Value="ApacheLicenseDlg">1</Publish>
Review comment:
I don't know either; let's keep this thread open for a while, maybe someone who knows the answer will comment -- if not, I agree that it's safer to show the license
--
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 pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
lordgamez commented on pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#issuecomment-952687101
Some of these changes seem to have broken the tests by not finding the processors with the names previously defined:
```
[2021-10-26 13:35:16.656] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.GenerateFlowFile
[2021-10-26 13:51:26.999] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.ListenHTTP
[2021-10-26 14:06:39.040] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.GetFile
[2021-10-26 14:08:43.518] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.ConsumeKafka
```
--
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 #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r738138779
##########
File path: msi/WixWin.wsi
##########
@@ -113,6 +117,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
<Publish Dialog="MaintenanceWelcomeDlg" Control="Next" Event="NewDialog" Value="MaintenanceTypeDlg">1</Publish>
+ <Publish Dialog="MaintenanceTypeDlg" Control="ChangeButton" Event="NewDialog" Value="ApacheLicenseDlg">1</Publish>
Review comment:
do we need to redisplay the `ApacheLicenseDlg`? I would expect to be taken to `CustomizeDlg` directly
--
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 commented on a change in pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r738409844
##########
File path: msi/WixWin.wsi
##########
@@ -113,6 +117,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
<Publish Dialog="MaintenanceWelcomeDlg" Control="Next" Event="NewDialog" Value="MaintenanceTypeDlg">1</Publish>
+ <Publish Dialog="MaintenanceTypeDlg" Control="ChangeButton" Event="NewDialog" Value="ApacheLicenseDlg">1</Publish>
Review comment:
removed the license dialog step for "Change" action
--
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 #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r736429795
##########
File path: CMakeLists.txt
##########
@@ -808,14 +809,28 @@ else()
endif()
set(CPACK_PACKAGE_INSTALL_DIRECTORY "ApacheNiFiMiNiFi")
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
-set(CPACK_RDKAFKA_COMPONENT_INSTALL ON)
-set(CPACK_MQTT_COMPONENT_INSTALL ON)
+
+
+# !WARNING! we need to manually add to CPACK_COMPONENTS_ALL otherwise
+# all components specified in "install" directives are added
+# (including thirdparties like range-v3)
+
+list(APPEND CPACK_COMPONENTS_ALL bin)
+cpack_add_component(bin REQUIRED)
if(WIN32)
- set(CPACK_COMPONENTS_ALL bin conf minifijni tzdata)
+ list(APPEND CPACK_COMPONENTS_ALL tzdata)
+ cpack_add_component(tzdata)
else()
- set(CPACK_COMPONENTS_ALL bin conf minifijni)
+ list(APPEND CPACK_COMPONENTS_ALL conf)
+ cpack_add_component(conf REQUIRED)
endif()
Review comment:
Could you add some display names? I think it would look nicer in the generated MSI.
```c++
list(APPEND CPACK_COMPONENTS_ALL bin)
cpack_add_component(bin DISPLAY_NAME "MiNiFi C++ executables" REQUIRED)
if(WIN32)
list(APPEND CPACK_COMPONENTS_ALL tzdata)
cpack_add_component(tzdata DISPLAY_NAME "Timezone database for Expression Language")
else()
list(APPEND CPACK_COMPONENTS_ALL conf)
cpack_add_component(conf REQUIRED DISPLAY_NAME "Default configuration files")
endif()
```
current screenshot of an MSI:
##########
File path: CMakeLists.txt
##########
@@ -808,14 +809,28 @@ else()
endif()
set(CPACK_PACKAGE_INSTALL_DIRECTORY "ApacheNiFiMiNiFi")
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
-set(CPACK_RDKAFKA_COMPONENT_INSTALL ON)
-set(CPACK_MQTT_COMPONENT_INSTALL ON)
+
+
+# !WARNING! we need to manually add to CPACK_COMPONENTS_ALL otherwise
+# all components specified in "install" directives are added
+# (including thirdparties like range-v3)
+
+list(APPEND CPACK_COMPONENTS_ALL bin)
+cpack_add_component(bin REQUIRED)
if(WIN32)
- set(CPACK_COMPONENTS_ALL bin conf minifijni tzdata)
+ list(APPEND CPACK_COMPONENTS_ALL tzdata)
+ cpack_add_component(tzdata)
else()
- set(CPACK_COMPONENTS_ALL bin conf minifijni)
+ list(APPEND CPACK_COMPONENTS_ALL conf)
+ cpack_add_component(conf REQUIRED)
endif()
Review comment:
Could you add some display names? I think it would look nicer in the generated MSI.
```c++
list(APPEND CPACK_COMPONENTS_ALL bin)
cpack_add_component(bin DISPLAY_NAME "MiNiFi C++ executables" REQUIRED)
if(WIN32)
list(APPEND CPACK_COMPONENTS_ALL tzdata)
cpack_add_component(tzdata DISPLAY_NAME "Timezone database for Expression Language")
else()
list(APPEND CPACK_COMPONENTS_ALL conf)
cpack_add_component(conf REQUIRED DISPLAY_NAME "Default configuration files")
endif()
```
current screenshot of an MSI GUI:
--
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 commented on a change in pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r738156449
##########
File path: msi/WixWin.wsi
##########
@@ -113,6 +117,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
<Publish Dialog="MaintenanceWelcomeDlg" Control="Next" Event="NewDialog" Value="MaintenanceTypeDlg">1</Publish>
+ <Publish Dialog="MaintenanceTypeDlg" Control="ChangeButton" Event="NewDialog" Value="ApacheLicenseDlg">1</Publish>
Review comment:
since the user might select new extension to be added I felt like the same legal requirements apply as during a fresh install, IANAL so if it is not required I'll 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] adamdebreceni commented on pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#issuecomment-953592311
> Some of these changes seem to have broken the tests by not finding the processors with the names previously defined:
>
> ```
> [2021-10-26 13:35:16.656] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.GenerateFlowFile
> [2021-10-26 13:51:26.999] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.ListenHTTP
> [2021-10-26 14:06:39.040] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.GetFile
> [2021-10-26 14:08:43.518] [org::apache::nifi::minifi::core::FlowConfiguration] [error] No Processor defined for org.apache.nifi.processors.standard.ConsumeKafka
> ```
added a directive to produce a single archive with all the components, also changed the references to the artifact by removing the `-bin` suffix
--
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 commented on a change in pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203#discussion_r736522171
##########
File path: CMakeLists.txt
##########
@@ -808,14 +809,28 @@ else()
endif()
set(CPACK_PACKAGE_INSTALL_DIRECTORY "ApacheNiFiMiNiFi")
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
-set(CPACK_RDKAFKA_COMPONENT_INSTALL ON)
-set(CPACK_MQTT_COMPONENT_INSTALL ON)
+
+
+# !WARNING! we need to manually add to CPACK_COMPONENTS_ALL otherwise
+# all components specified in "install" directives are added
+# (including thirdparties like range-v3)
+
+list(APPEND CPACK_COMPONENTS_ALL bin)
+cpack_add_component(bin REQUIRED)
if(WIN32)
- set(CPACK_COMPONENTS_ALL bin conf minifijni tzdata)
+ list(APPEND CPACK_COMPONENTS_ALL tzdata)
+ cpack_add_component(tzdata)
else()
- set(CPACK_COMPONENTS_ALL bin conf minifijni)
+ list(APPEND CPACK_COMPONENTS_ALL conf)
+ cpack_add_component(conf REQUIRED)
endif()
Review comment:
added display names
--
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 closed pull request #1203: MINIFICPP-1672 - Configurable msi
Posted by GitBox <gi...@apache.org>.
szaszm closed pull request #1203:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1203
--
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