You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/11/21 22:45:17 UTC

[GitHub] [zookeeper] Hugmeir opened a new pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Hugmeir opened a new pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546


   Previously, did this:
   
       ; cmake .
       ; make
       ; make install
   
   ended up doing nothing, because CMakeLists.txt didn't include
   INSTALL() directives.  With this commit applied, the last
   command outputs:
   
       ; make install
       [ 18%] Built target hashtable
       [ 75%] Built target zookeeper
       [ 87%] Built target load_gen
       [100%] Built target cli
       Install the project...
       -- Install configuration: ""
       -- Installing: /usr/local/include/zookeeper/zookeeper.h
       -- Installing: /usr/local/include/zookeeper/proto.h
       -- Installing: /usr/local/include/zookeeper/config.h
       -- Installing: /usr/local/include/zookeeper/zookeeper_version.h
       -- Installing: /usr/local/include/zookeeper/winconfig.h
       -- Installing: /usr/local/include/zookeeper/zookeeper_log.h
       -- Installing: /usr/local/include/zookeeper/recordio.h
       -- Installing: /usr/local/lib/libzookeeper.a


----------------------------------------------------------------
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] [zookeeper] eolivelli commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-734711857


   I am happy that it worked for you.
   Do you want to send a PR to update the README file please?


----------------------------------------------------------------
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] [zookeeper] eolivelli commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-734261103


   Probably you have only a JRE and not a fully JDK.
   I suggest you to install a full JDK.
   Consider using JDK11.
   I am using the one distributed by AdoptOpenJDK for instance. 
   


----------------------------------------------------------------
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] [zookeeper] Hugmeir commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
Hugmeir commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-731655516


   Whoops, missed the generated header in the list of files that need to be installed -- patched 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ztzg commented on a change in pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
ztzg commented on a change in pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#discussion_r528568179



##########
File path: zookeeper-client/zookeeper-client-c/CMakeLists.txt
##########
@@ -290,3 +290,21 @@ if(WANT_CPPUNIT)
     "ZKROOT=${CMAKE_CURRENT_SOURCE_DIR}/../.."
     "CLASSPATH=$CLASSPATH:$CLOVER_HOME/lib/clover*.jar")
 endif()
+
+# set up `make install`.
+# headers end up in /usr/local/include/zookeeper/*.h
+# (with /usr/local being overrideable by using --prefix etc)
+INSTALL(FILES
+    include/zookeeper.h
+    include/proto.h
+    include/config.h

Review comment:
       Note `$C_CLIENT/include/config.h` only exists if one builds in-place.  This, which is valid (and recommended) in CMake:
   
   ```sh
   mkdir build \
       && cd build \
       && cmake -DCMAKE_INSTALL_PREFIX=/tmp/zkc -DCMAKE_VERBOSE_MAKEFILE=YES .. \
       && cmake --build . \
       && cmake --install .
   ```
   
   fails with "no such file," as `config.h` ends up in `$C_CLIENT/build/include`.
   
   But `config.h` should not be installed, anyway: its only purpose is to drive the compilation of the library.  Each program which is compiled and linked against the ZooKeeper client should have their own `config.h`, and the binary interface to `libzookeeper` should be independent from it.
   
   (I know that `zookeeper.h` includes some `HAVE_*`/`THREADED` preprocessor defines, but that is unfortunate and historical.  And I have also seen your [ZOOKEEPER-4013](https://issues.apache.org/jira/browse/ZOOKEEPER-4013), "pkg-config file for C library," and agree that this is how client program compilation should be driven.)
   
   So: would you mind removing that line?

##########
File path: zookeeper-client/zookeeper-client-c/CMakeLists.txt
##########
@@ -290,3 +290,21 @@ if(WANT_CPPUNIT)
     "ZKROOT=${CMAKE_CURRENT_SOURCE_DIR}/../.."
     "CLASSPATH=$CLASSPATH:$CLOVER_HOME/lib/clover*.jar")
 endif()
+
+# set up `make install`.
+# headers end up in /usr/local/include/zookeeper/*.h
+# (with /usr/local being overrideable by using --prefix etc)
+INSTALL(FILES
+    include/zookeeper.h
+    include/proto.h
+    include/config.h
+    include/zookeeper_version.h
+    include/winconfig.h
+    include/zookeeper_log.h
+    include/recordio.h
+    generated/zookeeper.jute.h
+  DESTINATION include/zookeeper/)
+# libzookeeper.a ends up in /usr/local/lib/libzookeeper.a
+INSTALL(TARGETS zookeeper
+  ARCHIVE DESTINATION lib)
+

Review comment:
       I have seen your [ZOOKEEPER-4014](https://issues.apache.org/jira/browse/ZOOKEEPER-4014), "libzookeeper.a not properly linking to libhashtable.a," but I believe there is some confusion due to the way `.a` archives ("static libraries") work.
   
   `.a` archives cannot be "linked" against each other; they're just "ZIP archives of objects" of sorts.  The fact that the objects contained in `x.a` require more objects from `y.a` is, at best, encoding in a `pkg-config` or `libtool` metadata file.
   
   In the current state of affairs, the "least wrong" thing to do probably is to install both archives to the destination:
   
   ```
   INSTALL(TARGETS zookeeper hashtable
      ARCHIVE DESTINATION lib)
   ```
   
   and to link any client program against both; i.e., something like:
   
   ```
   $LINK […] -L/path/to/client/lib \
       -lzookeeper -lhashtable \
       -lcrypto -lssl -lsasl2
   ```
   
   But: it is indeed unclear why/if `libhashtable.a` is still needed if we go the CMake route.
   
   The Autoconf path produces a `libhashtable.a`, but it is systematically used with the internal `libzk{st,mt}.a`; as far as I can tell, its only purpose is to allow its thread-agnostic source files to only be compiled once. And as opposed to `libzk{st,mt}.a`, the installed `libzookeeper_{st,mt}.a` archives include the `hashtable` objects.
   
   (`.so` shared objects—or DLLs—also include the `hashtable` code, but that is different as these are "hard-linked" rather than being collections of loose objects.)




----------------------------------------------------------------
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] [zookeeper] symat commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-733576904


   @moruoxian please try this way:
   - download the latest release (3.6.2 now)
   - in the parent folder run 'mvn clean install -DskipTests -Pfull-build`
   
   more info for building the C-client can be found here: https://github.com/apache/zookeeper/blob/master/README_packaging.md#building-the-c-client-using-maven
   
   I think this readme file is not up-to-date, we should fix it: https://github.com/apache/zookeeper/blob/master/zookeeper-client/zookeeper-client-c/README (it is still referring to ant, what we don't use anymore)
   
   also feel free to ask these type of generic questions on the mailing list (https://zookeeper.apache.org/lists.html), here in this pull request much less people are watching :)


----------------------------------------------------------------
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] [zookeeper] moruoxian commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
moruoxian commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-733558986


   hello dear friend I found a problem  , I. download the 3.6.0 version and I cannot find the build.xml in the footpath then I cannot build the zookeeper-client-c  ,and I get. the error like this xxxx ant xxxx ,I think the build.xml is missing  !


----------------------------------------------------------------
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] [zookeeper] ShallyLai commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
ShallyLai commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-818383280


   Excuse me, I have a problem of install zookeeper on my MBP M1. When I type
   `brew install --build-from-source zookeeper`
   to install it, but appear these error messages below, and I can't find the answer online. Can you guys please help me how to solve it? 
   Thank you.
   
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (pre-compile-jute) on project zookeeper-jute: Fatal error compiling: error: invalid flag: -Xdoclint:-missing -> [Help 1]
   [ERROR]
   [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
   [ERROR] Re-run Maven using the -X switch to enable full debug logging.
   [ERROR]
   [ERROR] For more information about the errors and possible solutions, please read the following articles:
   [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
   [ERROR]
   [ERROR] After correcting the problems, you can resume the build with the command
   [ERROR]   mvn <args> -rf :zookeeper-jute


-- 
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] [zookeeper] moruoxian commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
moruoxian commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-734567974


   > Probably you have only a JRE and not a fully JDK.
   > I suggest you to install a full JDK.
   > Consider using JDK11.
   > I am using the one distributed by AdoptOpenJDK for instance.
   
   thank u
   I solve the jdk problem in windows system ,I change the path in system and  I use the cmake to build the project client_c , and mkdir build && cd build 
   ​cmake ..   -G "Visual Studio 16 2019"  -A win32 -T v140  -DCMAKE_BUILD_TYPE=Release
    then open the sln file with vs2019 and build  the I found the release file success !! thanks a lot  I think it need to put in the README file to help other C++ programmer to know it ,thanks  to everyone help me .


----------------------------------------------------------------
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] [zookeeper] moruoxian commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
moruoxian commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-734257898


   > Hello! Thanks for the initiative / contribution @Hugmeir ! In general I like the idea, and I also think the questions raised by @ztzg are important.
   > 
   > Sorry, I can't go much deeper to this PR right now, also I'm not really an expert in cmake or in make in general. But I'll follow this PR and I'm happy to test it later.
   > 
   > > One thing you have to be aware of, is that this build mechanism was initially contributed for "easier" Windows support; as far as I can tell, everybody still uses Autotools on POSIX-ish platforms.
   > 
   > this is true, and AFAIK cmake is the only way now how to build the C client on windows, so it is important that whatever we change here we should test it on windows too (I can help with that).
   
   thanks  a lot  and I find that I build it success on linux ,but  I  cannot build on windows  it  post error like this 
   
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (pre-compile-jute) on project zookeeper-jute: Compilation failure
   [ERROR] No compiler is provided in this environment. Perhaps you are running on a JRE rather than a JDK?
   [ERROR]
   [ERROR] -> [Help 1]
   can u help me dear friend?thanks 


----------------------------------------------------------------
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] [zookeeper] ShallyLai edited a comment on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
ShallyLai edited a comment on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-818383280


   Excuse me, I have a problem of installing zookeeper on my MBP M1. When I type
   `brew install --build-from-source zookeeper`
   to install it, but appear these error messages below, and I can't find the answer online. Can you guys please help me how to solve it? 
   Thank you.
   
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (pre-compile-jute) on project zookeeper-jute: Fatal error compiling: error: invalid flag: -Xdoclint:-missing -> [Help 1]
   [ERROR]
   [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
   [ERROR] Re-run Maven using the -X switch to enable full debug logging.
   [ERROR]
   [ERROR] For more information about the errors and possible solutions, please read the following articles:
   [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
   [ERROR]
   [ERROR] After correcting the problems, you can resume the build with the command
   [ERROR]   mvn <args> -rf :zookeeper-jute


-- 
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] [zookeeper] symat commented on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-732769920


   Hello! Thanks for the initiative / contribution @Hugmeir !  In general I like the idea, and I also think the questions raised by @ztzg are important. 
   
   Sorry, I can't go much deeper to this PR right now, also I'm not really an expert in cmake or in make in general. But I'll follow this PR and I'm happy to test it later.
   
   >One thing you have to be aware of, is that this build mechanism was initially contributed for "easier" Windows support; as far as I can tell, everybody still uses Autotools on POSIX-ish platforms.
   
   this is true, and AFAIK cmake is the only way now how to build the C client on windows, so it is important that whatever we change here we should test it on windows too (I can help with that). 


----------------------------------------------------------------
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] [zookeeper] ShallyLai removed a comment on pull request #1546: ZOOKEEPER-4012: zookeeper-client-c: add an `install` target to CMakeLists

Posted by GitBox <gi...@apache.org>.
ShallyLai removed a comment on pull request #1546:
URL: https://github.com/apache/zookeeper/pull/1546#issuecomment-818383280


   Excuse me, I have a problem of installing zookeeper on my MBP M1. When I type
   `brew install --build-from-source zookeeper`
   to install it, but appear these error messages below, and I can't find the answer online. Can you guys please help me how to solve it? 
   Thank you.
   
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (pre-compile-jute) on project zookeeper-jute: Fatal error compiling: error: invalid flag: -Xdoclint:-missing -> [Help 1]
   [ERROR]
   [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
   [ERROR] Re-run Maven using the -X switch to enable full debug logging.
   [ERROR]
   [ERROR] For more information about the errors and possible solutions, please read the following articles:
   [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
   [ERROR]
   [ERROR] After correcting the problems, you can resume the build with the command
   [ERROR]   mvn <args> -rf :zookeeper-jute


-- 
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