[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-11-24 Thread mkedwards
Github user mkedwards commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
I'd like to see this brought back.  Not cool to drop autoconf when the 
CMake setup can't build shared libraries.  From 
`zookeeper-client/zookeeper-client-c/README`:
```
Current limitations of the CMake build system include lack of Solaris 
support,
no shared library option, no explicitly exported symbols (all are exported 
by
default), no versions on the libraries, and no documentation generation.
```


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-08-08 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Btw. I think it's perfectly fine to choose option 2.
I was able to build ZooKeeper C client and run the tests on Ubuntu 18.04 
with CMake.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-08-08 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
@phunt I ran a few cycles with the curl commit that you linked and here're 
my results:

- there's no such thing with autotools that "check for a macro existence 
and use it if present", because macros must always be extracted even if they 
behind an if condition,
- Curl circumvents this behaviour by directly calling commands like 
`pkg-config --libs MYLIB` instead of using the macro that package maintainer 
provides,
- as a consequence we either can use the old AM_PATH_CPPUNIT macro or the 
new pkg-config macro, but not both.

I'm not a fan of Curl approach: it runs custom code essentially which seems 
to me fragile.

To make an end to this conversion I think we have 2 options.
1. Replace the old macro with the new one and commit it to trunk only --> 
we stop supporting the old way from 3.6.0 and pkg-config will be required. 
Reward: ZK builds on new systems like Ubuntu 18.04.
2. Don't do anything. 


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
I had no luck with verifying a specific macro existence for some reason. 
I'm not sure if there's a way at all. Maybe I should check how other projects 
are dealing with similar problems.

On the other hand, it's up to us whether we want to support `cppunit` 
1.14.0 and move to `pkg-config` or stay on m4 which means folks won't be able 
to build ZooKeeper on recent distros. 


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Hm. iiuc the m4 checks are during configure script creation itself, so 
perhaps that's not the best idea either. :-(




---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Perhaps we could do this? -- check if the cppunit macro is defined, if not 
then check for pkg-config, if not then error out (or whatever we do today). otw 
use the first macro/approach available.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Hm. It's not very clear that

> ./configure: line 5034: PKG_PROG_PKG_CONFIG: command not found

means that pkg-config needs to be installed. Esp given we've never needed 
it - it's a new dependency.

Here's an example of some pushback in the curl community 
https://github.com/curl/curl/issues/972

If you do want to add this you probably need to add something like:

```
m4_ifndef([PKG_PROG_PKG_CONFIG],
  [m4_fatal([Could not locate the pkg-config autoconf macros.
Please make sure pkg-config is installed and, if necessary, set the 
environment
variable ACLOCAL="aclocal -I/path/to/pkg.m4".])])
```

although I'm honestly not very psyched about this change. It's going to 
break a bunch of folk's builds.



---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
@phunt have you installed `pkg-config`?
Because your Ubuntu image doesn't include it.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-31 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
I'm afraid it still doesn't work for me. This is ubuntu 14.04 - the docker 
image I pointed to previously.


# ./configure
checking for doxygen... no
checking for perl... /usr/bin/perl
checking for dot... no
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
./configure: line 5034: PKG_PROG_PKG_CONFIG: command not found
./configure: line 5039: syntax error near unexpected token `CPPUNIT,'
./configure: line 5039: `PKG_CHECK_MODULES(CPPUNIT, cppunit >= 
1.10.2, , , )'




I also see all kinds of warnings popping up:

# autoreconf -if
configure.ac:39: warning: PKG_PROG_PKG_CONFIG is m4_require'd but not 
m4_defun'd
configure.ac:28: ZK_PKG_CONFIG_STATIC is expanded from...
../../lib/m4sugar/m4sh.m4:639: AS_IF is expanded from...
configure.ac:39: the top level
configure.ac:39: warning: PKG_PROG_PKG_CONFIG is m4_require'd but not 
m4_defun'd
configure.ac:28: ZK_PKG_CONFIG_STATIC is expanded from...
../../lib/m4sugar/m4sh.m4:639: AS_IF is expanded from...
configure.ac:39: the top level
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: Consider adding `AC_CONFIG_MACRO_DIR([m4])' to configure.ac and
libtoolize: rerunning libtoolize, to keep the correct libtool macros 
in-tree.
libtoolize: Consider adding `-I m4' to ACLOCAL_AMFLAGS in Makefile.am.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-30 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
@phunt I ended up defining a custom macro to support static linking. Now 
the script works with older versions of `pkg-config` too.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-30 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Yeah, I see what the problem is: `pkg-config` package has to be installed, 
but there's another problem with the version that is shipped with 14.04: it 
doesn't have `PKG_CHECK_MODULES_STATIC` macro, because it has been added in a 
more recent version according to this bug:
https://bugs.freedesktop.org/show_bug.cgi?id=19541

Adding the definition of the macro to our `configure.ac` script solves the 
problem, but I don't want it to be effective if recent version of pkg-config is 
in use. Trying to add it as a conditional, but my autoconf knowledge doesn't 
seem to be enough here. Still looking.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-29 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
ps. you can try using my repos, makes it easy to try out various envs:
https://hub.docker.com/r/phunt/
See the readme on how to run - I typically map my zk source directory into 
the container directly to aid in testing.

these images have cppunit installed - you can use the package manager to 
uninstall them if you want to test with and without --without-cppunit


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-29 Thread phunt
Github user phunt commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Seems like it breaks ubuntu 14.04 (not sure what else, that's what I 
tried). This is working fine w/o the patch:


./configure
checking for doxygen... no
checking for perl... /usr/bin/perl
checking for dot... no
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
./configure: line 5034: syntax error near unexpected token `CPPUNIT,'
./configure: line 5034: `PKG_CHECK_MODULES_STATIC(CPPUNIT, cppunit >= 
1.10.2)'


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-28 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
@phunt @afine @hanm You guys might want to take a quick look at this in 
@rgs1 absence. It's kind of an infrastructural change only and would be useful 
in the long term.


---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-24 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Using pkg-config in the latest patch.
I also removed unused method `print_completion_queue()`, because it doesn't 
compile under Ubuntu 18.04
@rgs1 Would you care to take a look?



---


[GitHub] zookeeper issue #528: ZOOKEEPER-3034 Facing issues while building from sourc...

2018-05-23 Thread anmolnar
Github user anmolnar commented on the issue:

https://github.com/apache/zookeeper/pull/528
  
Raised the question on libreoffice mailing list and from libcppunit-1.14.0 
the recommended way to configure build flags is using `pkg-config`.
I'm working on a patch to address this and will test it on CentOS 7 and 
Ubuntu 18.04. If both works fine, we could go forward with it.


---