Building SHARED and STATIC library

[cinemast]

As I can see, it is currently not possible to build both versions, correct?

Would a PR that offers this functionality be merged? I want to package a more recent version for debian, which requires .a and .so version of it.

[cdunn2001]

Would a PR that offers this functionality be merged?

Yes.

But isn't it possible to build both as two separate cmake runs, in 2 different directories? We test each in Travis CI regularly.

I want to package a more recent version for debian

Hopefully 1.4.x, at least.

[cinemast]

Yes, but it is not the common practice, and makes the packaging more complicated without benefits.

I will start with 0.7.1 for sid, and maybe build 1.4 for experimental. It depends on your ABI/API compatibility. Do you check them somewhere?

[cdunn2001]

The whole point of 1.x is to drop the old ABI compatibility. If you need that, then you still need 0.7.x.

[cdunn2001]

I'll try to include some of the recent, minor fixes into 0.8.0, still ABI compatible.

[cinemast]

That would be great. 👍

[cinemast]

Could you wait with releasing 0.8.0 until I completed the pullrequest for building shared and static lib together?

[cdunn2001]

No problem. Actually, 0.8.0 will be rebased near the tip of master anyway. I'll integrate your changes there too.

Will you be able to double-check ABI compatibility? If not, I'll just build our test with old and link with new.

[cinemast]

You might tell Andrey that the project has moved: http://upstream.rosalinux.ru/versions/jsoncpp.html

[cinemast]

But aside of that, of course I can check ABI compatibility. In fact I have to if this package should really be going on the debian archives.

[cdunn2001]

You might tell Andrey that the project has moved: http://upstream.rosalinux.ru/versions/jsoncpp.html

That only lists his linkedin account. Can you connect us? I'm @cdunn2001.

[cinemast]

http://upstream.rosalinux.ru/

Just click on the add link on the top of the list.

I don't care about linkedIn, sorry.

[cinemast]

API compliance does not look too good.

https://gist.github.com/cinemast/35a7362d9bcbd4c92f7f

result: INCOMPATIBLE (Binary: 62.3%, Source: 0.1%)
total "Binary" compatibility problems: 6, warnings: 7
total "Source" compatibility problems: 2, warnings: 7

Do you think you can fix that?

[cinemast]

Ref of the ongoing discussion on debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=762330

[cdunn2001]

Altered globals:

-  static const Value null;
+  static const Value& null;

That was for Chromium. I guess we can revert that in 0.x, if Debian doesn't need to work on ARM. Any idea?

New data-members in Json::Value:

+      // [start, limit) byte offsets in the source JSON text from which this Value
+      // was extracted.
+      size_t start_;
+      size_t limit_;

That we can definitely revert for 0.x. It will take a day or two though.

Json::FastWriter:

+  bool dropNullPlaceholders_;
+  bool omitEndingLineFeed_;

We can rename the new version and have 2 FastWriters. No problem.

Json::Features:

+  /// \c true if dropped null placeholders are allowed. Default: \c false.
+  bool allowDroppedNullPlaceholders_;
+  /// \c true if numeric object key are allowed. Default: \c false.
+  bool allowNumericKeys_;

Ugh. It's rare that anybody would need this class, but we'll have to revert this change too.

That's all I found. Those match precisely with what the ABI tool discovered, which is reassuring. Your tool overlooks changes to virtual-look-up tables, but jsoncpp is safe in that regard.

This will take a few days to revert on 0.7.0, ok?

What do you suggest for the ARM problem?

[cinemast]

To be honest, I do not understand the issue with the ARM fix. Maybe someone from Debian does and is following this thread.
So sorry, I cannot help you with the ARM fix.

On the other hand, it is great that all the other incompatibilities are not a big deal. It would be great to reserve the SOVERSION bump for the > 1.x version, because usually there should not be more than 2 versions of the same library in Debian. So we could put 1.x into experimental, and 0.8 into sid/unstable without major problems.

I will try to find someone at Debian who can help us out with the ARM problem.

Thank you for trying to resolve all this.

[cdunn2001]

Shared+Static is done (after today's fix). Binary-compatibility is separate, issue #157.

[cdunn2001]

@cinemast, check 0.8.0-rc1, and comment in issue #157, not here. Ok?

[cdunn2001]

Would you be satisfied with a script which runs the build twice, once to produce a shared lib and once for a static lib? We are trying to simplify the cmakefiles, and apparently it's difficult to do both at once.

[cinemast]

I don't think the script will be enough. I have to check how other Maintainers handle this situation. I do get that the current solution is not pretty and blows up the CMakeList files.

[cinemast]

The current way is also the suggested way of Kitware: http://www.cmake.org/Wiki/CMake_FAQ#Can_I_build_both_shared_and_static_libraries_with_one_ADD_LIBRARY_command.3F

It is also argued there why they do not provide a simple option to automatically build both versions.

I do not think we have an alternative here.

Most distribution's package build system (not only debian) consist of a configure and a build step for each package. This is why it is required to tell cmake to build both: shared and static version of the library.

[cdunn2001]

What if we have one configure script and one build script:

1. One script invokes cmake twice, to build a shared library in one directory and a static library in another.
2. A second script invokes make twice, once in each build directory, and installs both libraries plus one copy of the headers.

Could you call those 2 instead of cmake + make?

[rcdailey]

What problem are you guys solving here? How is doing cmake twice then make twice, different from just doing (cmake+make) twice? The latter makes more sense, since it's how the "build pipeline" is defined:

1. Generate using CMake based on desired configuration
2. Run build tool (e.g. ninja, make) to build binary output based on generate build scripts

Run the above steps any number of times desired for different binary output.

And don't forget, there are several important reasons why you should do it this way:

1. Different binary outputs may require different preprocessor conditions and configurations, which impact the code compiled and ultimately how the binary is built.
2. One consistent target name in CMake for jsoncpp is required for other CMake scripts to properly refer to it as a target dependency (this case is covered in my PR linked earlier).

Maybe I'm missing a fundamental requirement that you feel is missing from my solution? In that case, if you don't mind going over the details, I'd be happy to help. I'm still new to this project and I started off making this change for my own needs (more specifically, the 2nd point in the list above). But obviously any solution I propose needs to work for everyone not just my case.

Thanks guys.

[cinemast]

I already answered in #325, but I would also like to comment your points here:

ad 1:
The current situation does consider different preprocessor macros for shared and static. That is why we have currently two targets (one for shared and one for static), to allow exactly for that.

ad 2:
I do not like the idea of using CMakeFiles inside other CMake projects. Kitware added the great option for ExternalTargets to CMake (since version 2.8, I guess), which allows inclusion of any dependency project (not just CMake based). So I think this option should be used instead of embedding other CMakeFiles into the build process. CMake is just not that well suited for this approach.

An example of External Projects in CMake can be found here:
https://github.com/cinemast/libjson-rpc-cpp/blob/master/src/catch/CMakeLists.txt

Also here is the official docu to it:
http://www.cmake.org/cmake/help/v3.0/module/ExternalProject.html

Anyway, we have come to an agreement in #325. So thank you for participating.

[rcdailey]

If you're referring to ExternalProject_Add, that's another way of doing it but I don't see how it improves the situation: I still need to have special code to add the library I want (shared vs static). The way I'm doing it was intentional, even though maybe not recommended. Because it's easier to handle include directories, preprocessor definitions, and other properties of your project more transparently if I do it this way.

There is nothing wrong with doing it this way. It may not be recommended, but that doesn't mean that "CMake is not well suited for the approach".

As of CMake 3.0 there is a special packaging script you can generate:
http://www.cmake.org/cmake/help/v3.3/manual/cmake-packages.7.html#creating-packages

This will give you the best of both worlds. You should explore using this if you expect users to import your target via find_package or ExternalProject_Add.

When I brought up point 1, I was thinking of cache variables that affect preprocessor definitions in the actual code. I.e. you couldn't set the same variable differently for both passes (static and shared). It would be the same for both, always. Maybe this isn't a need ATM for jsoncpp.

There is also a lot of duplication of CMake code to manage common settings between both targets. Granted, defining more functions might take care of this, but the idea of generating 2 targets in 1 CMake configuration pass is still taboo for CMake.

If someone were to explain why Debian requires this specific setup maybe it would help me understand more. Right now it just feels unnecessary. Can you go into detail? Why does 2 builds versus 1 build to get the same result cause a problem?

[cinemast]

About Debian: It comes with a lot of handy tools that basically do the packaging automatically which is very convenient for the maintainer of the package. Especially setting hardening flags for security, optimizations flags, debugging flags, etc.

The tools however use heuristics to automatically detect how to build the software. They can not handle two different stages of configuring, building and installing automatically, and doing it by hand is very error prone. In Debian we usually ship both, the shared and the static version of a library, so that the end-user gets maximum flexiblity in using the library.

So Debian does not require a specific setup. It is just that the tools work best with following the "standard procedure" (patch, configure, build, test, intsall, package) and the maintainer can make less errors by using the standard procedure.

As a comparison, here are the two approaches to compare:
http://anonscm.debian.org/cgit/collab-maint/libjsoncpp.git/tree/debian/rules (with 2 targets)
http://anonscm.debian.org/cgit/collab-maint/libjsoncpp.git/tree/debian/rules?h=test (asssuming there is only 1 target)

I am still not sure if I got all the flags in there now. But generally it works. I am still in the mentoring process of Debian and not that much of an expert, so I was in doubt if I could keep maintaining the package in the same quality, if the target behaviour changes. But the proof of concept works for me in Debian.

It was very nice of @cdunn2001 and you to keep the discussion going until we figured out a solution. So thanks for that.

[rcdailey]

Glad you guys worked it out. I'll put this one to bed.

[cdunn2001]

See #334.

[Thuenem]

I am out of the office until 25.08.2015.

Dear Business Partner, I will be out of the office starting 17.08.2015 and
will not return until 25.08.2015. Your message will not be processed during
my abscence. I will respond to your message when I return. In urgent cases
please contact Peter Winterberg under +49.5971.80819953 or mail
peter.winterberg@prognost.com. - Thank you.

Note: This is an automated response to your message "Re: [jsoncpp]
Building SHARED and STATIC library (#147)" sent on 14.08.2015 23:59:21.

This is the only notification you will receive while this person is away.