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 2020/02/18 08:47:29 UTC
[GitHub] [nifi-minifi-cpp] am-c-p-p opened a new pull request #739:
MINIFICPP-1163 Implemented.
am-c-p-p opened a new pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739
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 master)?
- [ ] 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 travis-ci 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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-588128524
Basically it is a comparison of dynamically vs statically linked exes, since exe with VS DLLs in bin directory is similar to statically linked exe in sense that in both cases the exes don't depend on DLLs in Windows/System32 directory.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380558837
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
+ file(GLOB VCRUNTIME_X86_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x86/Microsoft.VC141.CRT")
+ file(GLOB VCRUNTIME_X64_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x64/Microsoft.VC141.CRT")
+
+ if (NOT VCRUNTIME_X86_REDIST_CRT_DIR OR NOT VCRUNTIME_X64_REDIST_CRT_DIR)
+ message(FATAL_ERROR "Could not find the VC Redistributable. Please set VCRUNTIME_X86_REDIST_CRT_DIR and VCRUNTIME_X64_REDIST_CRT_DIR manually!")
endif()
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
+ message("Using ${VCRUNTIME_X64_REDIST_DIR} VC Redistributables")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/concrt140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140_1.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140_2.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/vccorlib140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/vcruntime140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
elseif(CMAKE_SIZEOF_VOID_P EQUAL 4)
- message("Using ${VCRUNTIME_X86_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x86.wsi" "msi/x86.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x86.wsi")
+ message("Using ${VCRUNTIME_X86_REDIST_DIR} VC Redistributables")
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380558948
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
+ file(GLOB VCRUNTIME_X86_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x86/Microsoft.VC141.CRT")
+ file(GLOB VCRUNTIME_X64_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x64/Microsoft.VC141.CRT")
+
+ if (NOT VCRUNTIME_X86_REDIST_CRT_DIR OR NOT VCRUNTIME_X64_REDIST_CRT_DIR)
+ message(FATAL_ERROR "Could not find the VC Redistributable. Please set VCRUNTIME_X86_REDIST_CRT_DIR and VCRUNTIME_X64_REDIST_CRT_DIR manually!")
endif()
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
+ message("Using ${VCRUNTIME_X64_REDIST_DIR} VC Redistributables")
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380543187
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
Review comment:
This is extra whitespace.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587843637
If a requirement to not reboot machine, we don't have an option other than to put VS Dlls in the bin directory.
In https://developercommunity.visualstudio.com/content/problem/111499/how-to-detect-if-visual-c-2017-redistributable-is.html, reply `Daniel Griffing, Visual C++ Libraries` - `If you have absolute version-specific requirements, you should choose one of the other deployment options including static linking and/or app local deployment that enable you to control the exact version of your dependencies are in use.`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587843637
If a requirement to not reboot machine, we don't an option other than to put VS Dlls in the bin directory.
In https://developercommunity.visualstudio.com/content/problem/111499/how-to-detect-if-visual-c-2017-redistributable-is.html, reply `Daniel Griffing, Visual C++ Libraries` - `If you have absolute version-specific requirements, you should choose one of the other deployment options including static linking and/or app local deployment that enable you to control the exact version of your dependencies are in use.`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-600556379
Added parameter /M (for installer with merge modules) to win_build_vs.bat.
Created new WixWinMergeModules.wsi for installer with merge modules, ideally it would be better to have only one WixWin.wsi, but couldn't figure out simple way to do it.
Please review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-588128524
Basically it is a comparison of dynamically vs statically linked exes, since exe with VS DLLs in bin directory is similar to statically linked exe in sense that in both cases the exes don't depend on VS DLLs in Windows/System32 directory.
Cisco AnyConnect and Slack are dynamically linked and use VS DLLs in bin directory,
but Chrome, Edge and Zoom are statically linked.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380558724
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-588128524
Basically it is a comparison of dynamically vs statically linked exes, since exe with VS DLLs in bin directory is similar to statically linked exe in sense that in both cases the exes don't depend on VS DLLs in Windows/System32 directory.
Cisco AnyConnect and Slack are dynamically linked and use VS DLLs in bin directory,
but Chrome, Edge and Zoom are statically linked.
And not to require reboot is one the reason that these apps are created like 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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-588128524
Basically it is a comparison of dynamically vs statically linked exes, since exe with VS DLLs in bin directory is similar to statically linked exe in sense that in both cases the exes don't depend on VS DLLs in Windows/System32 directory.
Cisco AnyConnect and Slack are dynamically linked with VS DLLs in bin directory,
but Chrome, Edge and Zoom are statically linked.
And not to require reboot is one the reason that these apps are created like 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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380543419
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
+ file(GLOB VCRUNTIME_X86_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x86/Microsoft.VC141.CRT")
+ file(GLOB VCRUNTIME_X64_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x64/Microsoft.VC141.CRT")
+
+ if (NOT VCRUNTIME_X86_REDIST_CRT_DIR OR NOT VCRUNTIME_X64_REDIST_CRT_DIR)
+ message(FATAL_ERROR "Could not find the VC Redistributable. Please set VCRUNTIME_X86_REDIST_CRT_DIR and VCRUNTIME_X64_REDIST_CRT_DIR manually!")
endif()
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
+ message("Using ${VCRUNTIME_X64_REDIST_DIR} VC Redistributables")
Review comment:
This is non-existent, `VCRUNTIME_X64_REDIST_CRT_DIR` should be used.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
szaszm commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-597108997
> @szaszm Which dll is missing on Windows 2008 R2?
`api-ms-win-crt-runtime-l1-1-0.dll`
https://imgur.com/a/nO4Zzue
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r404924654
##########
File path: CMakeLists.txt
##########
@@ -543,27 +543,60 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
- endif()
- if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
- elseif(CMAKE_SIZEOF_VOID_P EQUAL 4)
- message("Using ${VCRUNTIME_X86_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x86.wsi" "msi/x86.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x86.wsi")
- else()
- message(FATAL_ERROR "Could not determine architecture, CMAKE_SIZEOF_VOID_P is unexpected: ${CMAKE_SIZEOF_VOID_P}")
- endif()
- set(CPACK_WIX_TEMPLATE "${CMAKE_CURRENT_SOURCE_DIR}/msi/WixWin.wsi")
+ if (INSTALLER_MERGE_MODULES)
Review comment:
Please make this a cmake `option` as we do for all of our other configuration options, so that it has an explanation and shows up in the listing of our cmake project configuration options.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-600556379
Added parameter /M (for installer with merge modules) to win_build_vs.bat.
Created new WixWinMergeModules.wsi for installer with merge modules, ideally it would be better to have only one WixWin.wsi, but couldn't figure out simple way to do it.
Please review.
Microsoft stopped supporting Windows Server 2008 R2 (for which Dlls like api-ms-win-*.dll are still required) https://support.microsoft.com/en-us/help/4456235/end-of-support-for-windows-server-2008-and-windows-server-2008-r2.
Some companies will continue using this OS, for which installer can be built with /M switch.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid closed pull request #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid closed pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587372925
It should be a case when bundling VS redistributable DLLs doesn't work. If we have this case, we can restore merge module and make it conditional.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
szaszm commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587962342
Let's collect the arguments on both sides and decide which approach is better!
for config option defaulting to merge modules:
* more choice on the use side
* we transparently benefit from later updates to vcredist
* recommended by microsoft (more than DLLs in app directory)
* one less instance of duplicate DLLs on the target system, less disk space needed
for shipping DLLs without merge module option:
* simpler codebase, we don't have to maintain two kinds of packaging
* no need to reboot the machine after installation
* easier to implement
Any more points for/against each side, or new suggestions? Is there a clear industry practice in this question?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587372925
It should be a case when bundling VS redistributable DLLs doesn't work. If we have this case, we can restore and make it conditional.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r405506934
##########
File path: CMakeLists.txt
##########
@@ -53,7 +53,7 @@ option(FORCE_WINDOWS "Instructs the build system to force Windows builds when WI
option(DISABLE_CURL "Disables libCurl Properties." OFF)
option(USE_GOLD_LINKER "Use Gold Linker" OFF)
-
+option(INSTALLER_MERGE_MODULES "Creates installer with merge modules" OFF)
Review comment:
I am fine with it, I feel that the general direction is we want to go forward with the DLL-bundle version once we are sure that it works everywhere/deprecate support everywhere where it doesn't, but for now we have the ability to use the merge module version with a simple configuration.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r405265995
##########
File path: CMakeLists.txt
##########
@@ -543,27 +543,60 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
- endif()
- if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
- elseif(CMAKE_SIZEOF_VOID_P EQUAL 4)
- message("Using ${VCRUNTIME_X86_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x86.wsi" "msi/x86.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x86.wsi")
- else()
- message(FATAL_ERROR "Could not determine architecture, CMAKE_SIZEOF_VOID_P is unexpected: ${CMAKE_SIZEOF_VOID_P}")
- endif()
- set(CPACK_WIX_TEMPLATE "${CMAKE_CURRENT_SOURCE_DIR}/msi/WixWin.wsi")
+ if (INSTALLER_MERGE_MODULES)
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r405453588
##########
File path: CMakeLists.txt
##########
@@ -53,7 +53,7 @@ option(FORCE_WINDOWS "Instructs the build system to force Windows builds when WI
option(DISABLE_CURL "Disables libCurl Properties." OFF)
option(USE_GOLD_LINKER "Use Gold Linker" OFF)
-
+option(INSTALLER_MERGE_MODULES "Creates installer with merge modules" OFF)
Review comment:
Currently we build with merge modules. This changes to shipped dlls by default. Do we want that?
If you change the PR to the old behavior, please have a look at win_build_vs.bat as well.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on a change in pull request
#739: MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on a change in pull request #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#discussion_r380543563
##########
File path: CMakeLists.txt
##########
@@ -543,23 +543,30 @@ if(WIN32)
list(REVERSE VCRUNTIME_REDIST_VERSIONS)
list(GET VCRUNTIME_REDIST_VERSIONS 0 VCRUNTIME_REDIST_DIR)
endif()
- file(GLOB VCRUNTIME_X86_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x86.msm")
- file(GLOB VCRUNTIME_X64_MERGEMODULES "${VCRUNTIME_REDIST_DIR}/MergeModules/Microsoft_VC*_CRT_x64.msm")
- if (NOT VCRUNTIME_X86_MERGEMODULES OR NOT VCRUNTIME_X64_MERGEMODULES)
- message(FATAL_ERROR "Could not find the VC Redistributable Merge Modules. Please set VCRUNTIME_X86_MERGEMODULE_PATH and VCRUNTIME_X64_MERGEMODULE_PATH manually!")
- else()
- list(GET VCRUNTIME_X86_MERGEMODULES 0 VCRUNTIME_X86_MERGEMODULE_PATH)
- list(GET VCRUNTIME_X64_MERGEMODULES 0 VCRUNTIME_X64_MERGEMODULE_PATH)
+
+ file(GLOB VCRUNTIME_X86_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x86/Microsoft.VC141.CRT")
+ file(GLOB VCRUNTIME_X64_REDIST_CRT_DIR "${VCRUNTIME_REDIST_DIR}/x64/Microsoft.VC141.CRT")
+
+ if (NOT VCRUNTIME_X86_REDIST_CRT_DIR OR NOT VCRUNTIME_X64_REDIST_CRT_DIR)
+ message(FATAL_ERROR "Could not find the VC Redistributable. Please set VCRUNTIME_X86_REDIST_CRT_DIR and VCRUNTIME_X64_REDIST_CRT_DIR manually!")
endif()
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
- message("Using ${VCRUNTIME_X64_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x64.wsi" "msi/x64.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x64.wsi")
+ message("Using ${VCRUNTIME_X64_REDIST_DIR} VC Redistributables")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/concrt140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140_1.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/msvcp140_2.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/vccorlib140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
+ file(COPY "${VCRUNTIME_X64_REDIST_CRT_DIR}/vcruntime140.dll" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/redist")
elseif(CMAKE_SIZEOF_VOID_P EQUAL 4)
- message("Using ${VCRUNTIME_X86_MERGEMODULE_PATH} VC Redistributable Merge Module")
- configure_file("msi/x86.wsi" "msi/x86.wsi" @ONLY)
- set(CPACK_WIX_EXTRA_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/msi/x86.wsi")
+ message("Using ${VCRUNTIME_X86_REDIST_DIR} VC Redistributables")
Review comment:
This is non-existent, `VCRUNTIME_X86_REDIST_CRT_DIR` should be used.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-600556379
Added parameter /M (for installer with merge modules) to win_build_vs.bat.
Created new WixWinLocalRedist.wsi for installer with local redistributables Dlls, ideally it would be better to have only one WixWin.wsi, but couldn't figure out simple way to do it.
Please review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-587372925
It should be a case when the later doesn't work. If we have this case, we can restore and make it conditional.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-595963815
@szaszm Which dll is missing on Windows 2008 R2?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] am-c-p-p edited a comment on issue #739:
MINIFICPP-1163 Implemented.
Posted by GitBox <gi...@apache.org>.
am-c-p-p edited a comment on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-588128524
Basically it is a comparison of dynamically vs statically linked exes, since exe with VS DLLs in bin directory is similar to statically linked exe in sense that in both cases the exes don't depend on VS DLLs in Windows/System32 directory.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-593397123
@am-c-p-p Until we are sure that the new approach works on all systems that the old one supported, I want to keep the option to easily switch between the two methods and not having to dig it up from git history when required.
When the new method has been widely verified by both us and the users, and it proves superior to the Merge Module one, I am perfectly fine with removing the Merge Module one in favour of 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] bakaid commented on issue #739: MINIFICPP-1163
Implemented.
Posted by GitBox <gi...@apache.org>.
bakaid commented on issue #739: MINIFICPP-1163 Implemented.
URL: https://github.com/apache/nifi-minifi-cpp/pull/739#issuecomment-603775231
@am-c-p-p Thank you, I will start builds and tests on the new changes.
----------------------------------------------------------------
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
With regards,
Apache Git Services