[docs] Add maintainer guidelines

[ras0219-msft]

This PR adds a set of maintainer guidelines, loosely modeled after Debian's Policy Manual, Homebrew's Maintainer Guidelines, and Homebrew's Formula Cookbook.

Once merged, this document will continue to be updated and maintained over time.

Feedback is welcome and encouraged!

Todos:

• Integrate into GitHub's PR workflow to make it discoverable (CONTRIBUTING.md?)
• Add to docs TOC

[Neumann-A]

Missing:
If CMakes find_package expects a different name for the debug libraries and the library does not provide this by default use CMAKE_DEBUG_POSTFIX in vcpkg_configure_cmake to "rename" the library. This is required so that downstream projects can correctly find the debug configuration of the library using CMake. Do not set CMAKE_DEBUG_POSTFIX if CMakes find_package does not require it.

[ras0219-msft]

I think that in general, we should not be doing this.

The reasoning is twofold:

1. CMake (i.e. Kitware) doesn't have the authority to tell open source projects what their binaries should be called. Just because someone at Kitware merged a change to a FindXYZ script does not mean that zlib/expat/boost/openssl should now globally have their binaries renamed without the consent of the upstream maintainers.
2. Existing users of the libraries who aren't using CMake will have existing expectations. For example, Makefiles will often just use -lfoo because that's what the binaries are called in all existing package managers and systems. We want to enable these users to switch to vcpkg seamlessly, which means we need to use the same names that they are used to using -- renaming the debug name to libfood.a would break them.

When using single configuration generators with CMake, this should be a non-issue -- find_library(the_one_name) will find the right library.

However, we do need to handle the case of Multi-configuration generators. I believe the right way to proceed for this case will be to add vcpkg-cmake-wrapper.cmake helpers which fixup the built-in cmake FindXYZ modules by (for instance) modifying XYZ_LIBRARIES to include debug;debug/lib/foo;optimized;lib/foo. There is a finite number of FindXYZ scripts in CMake and they've publicly stated that they do not want to add any more, so I'm not concerned with the costs of this approach ballooning out of control.

This appears (to me) to be the ONLY way to:

1. Work with existing non-CMake build scripts we don't control
2. Work with CMake multi-config generators (like Visual Studio) when using built-in FindXYZ modules
3. Avoid alienating upstream authors by introducing names for their binaries that they have not "blessed"

[Neumann-A]

@ras0219-msft:

CMake (i.e. Kitware) doesn't have the authority to tell open source projects what their binaries should be called. Just because someone at Kitware merged a change to a FindXYZ script does not mean that zlib/expat/boost/openssl should now globally have

That is in general a correct stance with one important exception: If the project uses CMake the maintainers of the project implicitly agree to renaming the library because passing CMAKE_<config>_POSTFIX is a valid - and i would even say, common - option for the users to pass to the configure step which the libraries authors have to expect. Unless they explicitly unset CMAKE_<config>_POSTFIX they agreed to such a renaming.
To apply this to the mentioned examples: zlib, expat are CMake based projects so no issue there. boost probably has a build option to define library suffixes. OpenSSL is somewhat special since the build does not generate the suffixed version but "officially" distributed binaries have the additional suffix. So do with that whatever you like; I don't mind to much in this particular case because it is just a mess. They probably just do a library renaming like I have seen in the openblas CMakeLists.txt for shared build but I won't investigate that further.

CMake (i.e. Kitware) doesn't have the authority to tell open source projects what their binaries should be called

Does VCPKG have that authority? e.g.: #6698

Existing users of the libraries who aren't using CMake will have existing expectations. For example, Makefiles will often just use -lfoo because that's what the binaries are called in all existing package managers and systems. We want to enable these users to switch to vcpkg seamlessly, which means we need to use the same names that they are used to using -- renaming the debug name to libfood.a would break them.

I think your arguments is based on a wrong assumption here. Do package managers and system even supply debug libraries? Do Makefiles care about external debug libraries? Did the writer of the Makefile even consider external debug libraries? I think the answer to those questions are probably all "No" because I read somewhere that on linux debug/release are not so well defined as on Windows due to the iterator debug levels. Due to that VCPKG should not care about them because we still deliver the release libraries and it should still work for external users. Your main concern here seems to be the ports within VCPKG which use Makefiles instead of CMake. To patch these Makefiles is the work VCPKG has to do internally.
Furthermore there has been examples/requests where users copied both configurations into the same folder and here the different naming is of course mandatory. Without the renaming you would brake those users. (also I agree that this is probably a bad reason and different configurations should go into different folders)
To further add to this: Adding a debug suffix is the only way for CMake to pickup the debug libraries with 100% certainty due to how find_library works. It also avoids the silly case of doing a dropin replacement of debug/release shared libraries. So instead of removing debug suffixes VCPKG should probably rather add them (but that is not something I am willing to actively promote).

However, we do need to handle the case of Multi-configuration generators. I believe the right way to proceed for this case will be to add vcpkg-cmake-wrapper.cmake helpers which fixup the built-in cmake FindXYZ modules by (for instance) modifying XYZ_LIBRARIES to include debug;debug/lib/foo;optimized;lib/foo. There is a finite number of FindXYZ scripts in CMake and they've publicly stated that they do not want to add any more, so I'm not concerned with the costs of this approach ballooning out of control.

From my experience with PR 5543. debug;debug/lib/foo;optimized;lib/foo does not work everywhere. Generator expressions do not work everywhere. finite number around 150 in CMake and probably a factor 3 to 10 more in all ports either derived from the CMake ones or customized to satisfy the libraries demands. So i would at least expect 10-30% of all ports in VCPKG require a wrapper. vcpkg-cmake-wrapper.cmake can we please rename that to vcpkg_find_xyz.cmake because that is what it is actually doing and add a function vcpkg_install_find_wrapper.cmake to install them.

TL;DR: My opinion

• If the project is using CMake, renaming via configure options is ok because library authors implicitly agreed due to the usage of CMake configure options.
• We should not care about external Makefiles not using renamed debug libraries because they probably did not care about the debug case anyway. VCPKG internally used Makefiles must be patched to account for the debug case correctly.
• We need to figure out a way for Multi-configuration generators to work correctly.

[ras0219-msft]

First, given that a library "officially" uses the same name in release and debug, we can expect that FindXYZ scripts will be written with a single find_library() call.

As I see it, our options are:

1. Do nothing -> single configuration works perfectly. Multi config does not work. External non-cmake buildsystems work (because we didn't change anything compared to "raw" upstream).
2. Add debug suffix -> All existing FindXYZ scripts are broken in all configurations, because they find the release library in debug mode. I think that attempting to hack up find_library() to "automatically" find the new names is too problematic.
3. Add debug suffix AND a vcpkg-cmake-wrapper -> Single config works. Multi config works. Non-CMake buildsystems are all broken because none of them are designed to expect the new name.
4. Add vcpkg-cmake-wrapper -> single configuration still works perfectly (no changes). Multi config also now works. Existing non-cmake buildsystems work as well (no changes).

Given this, it seems like only adding a vcpkg-cmake-wrapper is the obviously correct thing to do. I'll also note that Multi-config generators, while we do care about making them work, are going to become less and less common since VS now has built-in cmake support (which runs in Single Configuration mode :)).

I'll also note that within Vcpkg we always build in single-configuration style (even if using MSBuild!), so as long as we just don't add a suffix, everything will tend to work fine (including existing FindXYZ scripts embedded in other libraries). In this case, we actually tend to hit the opposite problem where libraries might add a suffix in their official binaries but FindXYZ scripts aren't written to handle them correctly.

option for the users to pass to the configure step which the libraries authors have to expect.

I have not seen it to be conventional to pass this value in with other package managers (apt, brew, etc). If you have evidence to the contrary, I'd love to see it!

Just because CMake has a crazy option which can mess with your library, doesn't mean that you've implicitly approved of it.

Does VCPKG have that authority?

At the end of the day, we do have to ship something, and we have the power to patch buildsystems to decide what the binaries are called. My primary point in the guidelines is that we should seek to exercise this power as little as possible while achieving compatibility. As much as possible, we should use the names that the upstream buildsystem(s) use. The one common exception is where we do choose to exercise this is static vs dynamic -- I'll omit talking more about that case for brevity.

boost probably has a build option to define library suffixes

Boost specifically has an enumeration of valid library naming schemes -- we use one of them (with asterisks).

Do Makefiles care about external debug libraries? Did the writer of the Makefile even consider external debug libraries?

You're right that Makefiles on Linux may not commonly handle this, but other cases will. Qt/QMake does, MSBuild does, and NMake Makefiles on Windows will (for exactly the reason you stated).

Your main concern here seems to be the ports within VCPKG which use Makefiles instead of CMake.

No, I'm actually primarily concerned with users outside vcpkg, using buildsystems that we can't know or fix. Within vcpkg's curated set, I'm certain that we can patch our way to victory no matter what strategy we choose here -- however, we can't patch external software like firefox (for example). Therefore, we should choose names that are maximally likely to be compatible with existing practice. There's an obvious choice for that: it's whatever the current buildsystems produce by default with no interference/non-default options.

examples/requests where users copied both configurations into the same folder

Links? Vcpkg must be able to handle libraries that use the same name for release and debug (in the general case). Therefore, we always use separate folders.

Adding a debug suffix is the only way for CMake to pickup the debug libraries with 100% certainty due to how find_library works.

That only applies if you are using find_library(), and only if you are using multi-config generators. If you are using -config.cmake files, everything works fine. If you are using a single configuration, then having debug and release named the same actually improves things, because it becomes impossible to accidentally find the wrong library (since we list debug before release).

debug;debug/lib/foo;optimized;lib/foo does not work everywhere.

This is the outcome for FindXYZ.cmake scripts that do handle both libraries at once; if that doesn't work, we have bigger problems!

vcpkg-cmake-wrapper.cmake can we please rename that to vcpkg_find_xyz.cmake

Not a bad idea, I agree this would be clearer. Though I think the primary problem with vcpkg-cmake-wrapper is simply that it's largely undocumented -- thanks for bringing this up and I should add it to this doc!

[jumpinjackie]

Could you perhaps add notes on the recommended way to do OS checks?

In my past few submitted PRs, I have received suggestions to use:

• CMAKE_HOST_(WIN32|UNIX|APPLE) [freexl]: Linux build support #6813
• CMAKE_SYSTEM_NAME regex match [sqlite3]: Shared library support for Linux #6856

Some formal guidelines on the matter would be great

[Neumann-A]

I don't think that this is required. You simply have to learn to differentiate between HOST and TARGET correctly and wehter you need to check the HOST or the TARGET in the cases you are looking at. Also you need to differentiate between the circumstance if you are in cmakes script or configure/generator mode. The portfile are consumed in script mode while the CMakeLists.txt are consumed via normal configure/generator mode. In #6856 you were patching a CMakeLists.txt where you wanted to check the TARGET. Here CMAKE_SYSTEM_NAME is set correctly and can be and should be used. It is not set in script mode. In #6813 you want to figure out the correct build command on the HOST machine in script mode so you need to use CMAKE_HOST_(WIN32|UNIX|APPLE).
So in the end you just need to read/learn what certain variables in cmake mean and when to use them correctly and that has nothing to do with VCPKG.

[ras0219-msft]

This is a great explanation and I think it would also be good to include this in the document for future maintainers!

[ras0219-msft]

I've added an ascii art diagram to illustrate the difference. Let me know if it helps!

[JackBoosY]

Related: #6045.

[seanyen]

#3707

This -N approach could be confusing since it is not able to tell if the suffix comes from the upstream or vcpkg port. I recommended to use +N conventions from SemVer.

[ras0219-msft]

This is interesting, however:

Build metadata SHOULD be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence.

If we are using SemVer as the driving design, this would imply that +2 is not to be considered "better" than +1 -- which we do intend.

Based on my experience will all the current libraries, the number of cases where a trailing -N is potentially confusing has been extremely small. The vast majority of libraries prefer to use .s or _s for numeric/semver-style versions or tend to have alphabetic terms when trailing (-rc1, -beta, -apr2018, etc). It also helps that we avoid shipping prerelease versions as much as possible :).

[Rastaban]

A link to the control-file.md documentation that already talks about this would be useful.

[Rastaban]

This does not match was is documented in control-file.md. The date the portfile was created is not as useful as a date the conveys the version of the upstream library.

[JackBoosY]

When updating the version, it is better to take the existing release version(eg: v1.2.1) than to use commit Id(eg: 11b020b9f9e111bddd40bffe3b1759aa02d966f0), because using commit Id may introduce some unresolved bugs, and the release version must be more stable unless the latest release version is too old.

[ras0219-msft]

Thanks everyone for the reviews and comments!

We're happy to receive PRs suggesting new guidelines or pieces of information that would be helpful for future maintainers!

[KindDragon]

Is it Ok to do this for C libraries?