ams-tschoening commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554610161
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS
HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
Review comment:
How do you execute that target, on the shell and with which command?
Visual Studio doesn't seem to provide anything obvious in its UI, `installing`
doesn't seem to trigger that target. Adding `ALL` like for Doxygen does, but
that's not what you intended?
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS
HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
Review comment:
I have more things to exclude, some of those even preventing things from
working:
```
Schweregrad Code Beschreibung Projekt Datei Zeile
Unterdrückungszustand
Fehler Problem copying file:
C:/Users/tschoening/Documents/Svn/Src/Libs/trunk/C++/Logging/log4cxx/master/src/.vs/src/v16/Browse.VC.db
->
C:/Users/tschoening/Documents/Svn/Src/Libs/trunk/C++/Logging/log4cxx/master/src/out/build/x64-Debug/_CPack_Packages/win64-Source/TGZ/apache-log4cxx-0.11.0////.vs/src/v16/Browse.VC.db
C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\out\build\x64-Debug\src
C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\out\build\x64-Debug\EXEC
1
```
I need the following instead:
> set(CPACK_SOURCE_IGNORE_FILES ".git/;.vs/;build/;out/")
Your `build` is `out/build` and `out/install` for me. That's configured in
my JSON file, though I'm not sure if that was a default of Visual Studio or a
decision I made.
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS
HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
+ COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} -- package_source
+ COMMAND ${CMAKE_COMMAND} -E sha512sum
"apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz" >
"apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz.sha512"
Review comment:
`sha512sum` is simply a binary expected to be available using `PATH`,
correct? Because in my case it isn't, but the build seems to succeed. I didn't
see any error message that this command or that for `gpg` failed. Though, only
the archives themself got created.
Is that something to ignore or make more robust?
##########
File path: src/CMakeLists.txt
##########
@@ -13,3 +13,19 @@ if(BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(examples/cpp)
endif()
+
+option(BUILD_SITE "Build log4cxx website" OFF)
Review comment:
Why is this placed here instead of the upper level `CMakeLists.txt`
containing `APACHE_MAINTAINER` already? Seems to make more sense to me to have
both near by each other.
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS
HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
Review comment:
I suggest adding some indentation here to make things more readable,
like you did with Doxygen already.
----------------------------------------------------------------
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:
[email protected]