libpng icon indicating copy to clipboard operation
libpng copied to clipboard

Allow overwriting png library name

Open eldruin opened this issue 6 years ago • 2 comments

This allows overwriting the png library output name from the outside by making it a cmake cache variable.

eldruin avatar Nov 08 '18 13:11 eldruin

Thank you, this may be useful, but before that:

I inherited the code base with both PNGLIB_NAME and PNG_LIB_NAME. There should only be one, and I need to resolve that, eventually.

As far as nomenclature is concerned, I see that CMake uses both _LIB_NAME and _LIBRARY_NAME, and I don't know which name is appropriate for which use case.

(I'm not a CMake expert, BTW.)

ctruta avatar Nov 26 '18 06:11 ctruta

I see. I do not know if there is an actual convention about _LIB_NAME vs ._LIBRARY_NAME. What is somewhat unusual is not having an underscore before LIB (i.e. PNGLIB_NAME), I would say. Although I see that it matches the other PNGLIB_* variable names nicely. Looking at the CMake modules, here are the occurrences of LIB_NAME and LIBRARY_NAME I think having just one variable is much more important than whatever is named. I saw that the difference in both variables is that PNGLIB_NAME contains the library prefix for all platforms (but is only used for POSIX platform cases) and PNG_LIB_NAME does not contain the library prefix and is used to actually give the library the name, which will make CMake add the lib prefix only where appropriate. I will open a separate PR addressing this.

eldruin avatar Nov 26 '18 08:11 eldruin

As I mentioned in #260:

  • I withdrew completely PNGLIB_NAME and I used the hard-coded name "libpng" instead, because that name should never change.
  • I replaced PNG_LIB_NAME with PNG_SHARED_OUTPUT_NAME and PNG_STATIC_OUTPUT_NAME. Rather than making them cached, I opted to let the user set prefix and suffix modifiers, such as CMAKE_SHARED_LIBRARY_PREFIX, etc., CMAKE_STATIC_LIBRARY_SUFFIX, etc. I understood that this is CMake's "proper" way of modifying target file names.

ctruta avatar Feb 12 '23 23:02 ctruta

I need to check on it but I think that should work for us and will make a new suggestion if it somehow does not. Thank you!

eldruin avatar Feb 13 '23 07:02 eldruin