Allow usage as sub-project

[Flamefire]

I suggest to support use cases using this as a git submodule and via cmake add_subdirectory

Changes required that I've seen:

• Conditionally enable test/install if top-level project:

  libzippp/CMakeLists.txt

  Line 8 in be75a34

  option(INSTALL_HEADERS "Install library headers" ON)

  (CMAKE_SOURCE_DIR vs PROJECT_SOURCE_DIR)
• Decide on include method. You seem to prefer #include <libzippp.h>? Often a subfolder is used: #include <libzippp/libzipp.h> This allows adding more headers later without clobbering the includes. Example: libzippp/config.h which is impossible with having the subfolder in the include path. Relevant code: https://github.com/ctabin/libzippp/blob/master/CMakeLists.txt#L18-L19
• Don't make LIBZIPPP_EXPORTS public:

  libzippp/CMakeLists.txt

  Line 25 in be75a34

  target_compile_definitions(libzippp PUBLIC LIBZIPPP_EXPORTS)

• allow disabling install of anything like (see also first point)

  libzippp/CMakeLists.txt

  Line 36 in be75a34

  if (INSTALL_HEADERS)

• add alias target libzippp::libzippp (same as installed name, so either can be used)

BTW: Nice modern CMake! 👍

[ctabin]

@Flamefire Thanks for your report. Could you submit a PR with the needed changes ? That would be great !

[Flamefire]

I will. What about my second point? Needs an owner decision ;)

[ctabin]

@Flamefire Thanks for your feedback. It makes sense to me to have a subfolder. Maybe we can make that configurable somehow ?

[Flamefire]

While I understand the idea of having it configurable I'd rather not. Every option added must/should be tested so every option added increases the amount to test by a factor of 2. (On/off with every other option)

Downside of the subfolder: currently installed project uses no subfolder in the include, but a subfolder in the install: installed to include/libzippp/libzippp.h but #include <libzippp.h> due to the subfolder being added to -I, so that would be a breaking change. However as it is probably installed into some global location for users not using CMake they could be using #include <libzippp/libzippp.h> without using any -I already.

So IMO: Use a subfolder, bump the major version as per SemVer, don't use an option toggle. Oh and while doing that: Maybe require C++11? Should be supported wide enough already and allows to replace the typedefs at

libzippp/src/libzippp.h

Lines 55 to 70 in be75a34

	#ifdef WIN32
	typedef long long libzippp_int64;
	typedef unsigned long long libzippp_uint64;
	typedef unsigned short libzippp_uint16;
	//special declarations for windows to use libzippp from a DLL
	#define LIBZIPPP_SHARED_LIBRARY_EXPORT __declspec(dllexport)
	#define LIBZIPPP_SHARED_LIBRARY_IMPORT
	#else
	//standard ISO c++ does not support long long
	typedef long int libzippp_int64;
	typedef unsigned long int libzippp_uint64;
	typedef unsigned short libzippp_uint16;
	#define LIBZIPPP_SHARED_LIBRARY_EXPORT
	#define LIBZIPPP_SHARED_LIBRARY_IMPORT
	#endif

by their standard equivalents as well as e.g. returning a unique_ptr from fromBuffer and/or making it movable. I also noticed that your MakeFile already adds -std=c++0x although it doesn't seem to be used. Finally: Maybe drop the makefile completely? It is not a "clean" Makefile to be used e.g. by package maintainers, so rather factor out the dependency installation into shell scripts (code like in my updated travis.yml) and announce CMake as the "official" way.
Just mentioning all this so it can be considered when bumping the major version.

[Flamefire]

@ctabin Take a look at my PR #52 which implements the basics and puts CMake into focus. The include path change is not done as I'm waiting for your decisions. Besides that I think the setup and testing is pretty solid.

[ctabin]

@Flamefire That's very promising ! Regarding the include, the goal is to follow the standards as much as possible, and also to keep it simple. I wonder how vcpkg will behave, since libzippp has been included in it (see microsoft/vcpkg#6801 and #46).

Since this will change the building of libzippp, I don't mind making a new major version with some breaking changes, such as the include, so let's go 👍

[Flamefire]

I wonder how vcpkg will behave, since libzippp has been included in it (see microsoft/vcpkg#6801 and #46).

That is indeed an issue. Users will use the targets and expect #include "libzippp.h" to work. Maybe the better solution would be to skip the subfolder and install libzippp.h into include directly assuming this will always be the only header file? This would unify usage with non-cmake there libzippp is installed into /usr/local and hence #include "libzippp.h would work without any -I. However this would break such uses of the current libzippp where #include "libzippp/libzippp.h" is already used without any -I.

Maybe: Use the subfolder, but add include AND include/libzippp to the include path of the targets and announce one of them to be deprecated/discouraged so users should start using the other.

Then which one? If there will only be 1 header then #include "libzippp/libzippp.h" looks rather ugly :/

Maybe just keep it as-is. IMO there is no real standard for handling stuff like that, so...

[ctabin]

@Flamefire I don't see any other header than libzippp.h in a near future. Actually, this library is intended to be small, since it is only a wrapper.

I would say to keep things as-is, having #include "libzippp/libzippp.h does'nt look so ugly to me 😅

[Flamefire]

Ok then it will stay as #include "libzippp.h" for CMake users and #include "libzippp/libzippp.h# for users using default install location and no -I or CMake