Use idiomatic GNUInstallDirs for all install paths; fix pkg-config and relocatability

[Copilot]

Summary

Replace all hardcoded ${CMAKE_INSTALL_PREFIX}/... path constructions in CMakeLists.txt with proper GNUInstallDirs variables, fix the pkg-config template to respect user override variables, and remove dead code.

Changes

CMakeLists.txt

Relative DIR_* variables via GNUInstallDirs — All DIR_* variables now use relative paths (idiomatic CMake for install() destinations and relocatable package config):

Old (hardcoded absolute)	New (GNUInstallDirs relative)	Default for /usr/local / /usr
${CMAKE_INSTALL_PREFIX}/include	CMAKE_INSTALL_INCLUDEDIR	include
${CMAKE_INSTALL_PREFIX}/etc	CMAKE_INSTALL_SYSCONFDIR	etc / /etc ¹
${CMAKE_INSTALL_PREFIX}/lib${LIB_SUFFIX}	CMAKE_INSTALL_LIBDIR	lib (also handles lib64, multiarch ²)
${CMAKE_INSTALL_PREFIX}/share	CMAKE_INSTALL_DATAROOTDIR	share

¹ When prefix is /usr, GNUInstallDirs correctly resolves SYSCONFDIR to /etc per GNU Coding Standards. The old code produced the non-standard /usr/etc.

² Master's LIB_SUFFIX only handled lib64. CMAKE_INSTALL_LIBDIR additionally supports Debian multiarch paths like lib/x86_64-linux-gnu.

Absolute DIR_*_FULL variants on demand — A foreach block with an IS_ABSOLUTE guard computes DIR_SHARE_FULL, DIR_INCLUDE_FULL, and DIR_LIBRARY_FULL. These are used only where absolute paths are truly required:

• Compile-time macros (PKGDATADIR, LOCALEDIR) — needed at runtime to locate dictionaries and config files
• The opencc.pc.in pkg-config template

Dead code removal:

• Removed LIB_SUFFIX / LIB_INSTALL_DIR CACHE logic (superseded by CMAKE_INSTALL_LIBDIR)
• Removed DIR_PREFIX variable (no remaining consumers after opencc.pc.in switched to CMAKE_INSTALL_PREFIX)

Legacy override consistency — All four legacy override variables are preserved and handled consistently:

• SHARE_INSTALL_PREFIX → DIR_SHARE
• INCLUDE_INSTALL_DIR → DIR_INCLUDE
• SYSCONF_INSTALL_DIR → DIR_ETC
• LIB_INSTALL_DIR → DIR_LIBRARY

opencc.pc.in

• prefix now uses @CMAKE_INSTALL_PREFIX@ directly (was @DIR_PREFIX@, now removed)
• libdir / includedir now use @DIR_LIBRARY_FULL@ / @DIR_INCLUDE_FULL@ — these are computed after all legacy overrides are applied, so user-supplied -DINCLUDE_INSTALL_DIR=/custom/path is correctly reflected (was broken in an earlier revision)

Side-effect improvements

• Relocatable CMake package config: $<INSTALL_INTERFACE> in src/CMakeLists.txt and configure_package_config_file(PATH_VARS DIR_INCLUDE) now receive relative paths, making OpenCCConfig.cmake properly relocatable
• Debian multiarch support: CMAKE_INSTALL_LIBDIR handles lib/x86_64-linux-gnu etc., which the old LIB_SUFFIX approach could not

Files changed

• CMakeLists.txt — install directory logic rewrite (+27 lines, −10 lines net)
• opencc.pc.in — switch to absolute _FULL variables and CMAKE_INSTALL_PREFIX

[BYVoid]

Thanks for the cleanup — using GNUInstallDirs properly is the right direction. The verbose documentation block is a nice touch for future maintainers. A few issues to address before merging:

Summary of concerns:

1. pkg-config regression: opencc.pc.in now uses CMAKE_INSTALL_FULL_* variables directly, which bypasses the legacy override variables (INCLUDE_INSTALL_DIR, LIB_INSTALL_DIR, etc.) that the CMakeLists.txt still supports. A packager using -DINCLUDE_INSTALL_DIR=/custom/path would get the right install layout but a broken .pc file.

2. Inconsistent removal of LIB_INSTALL_DIR: The override block for LIB_INSTALL_DIR is removed while SHARE_INSTALL_PREFIX, INCLUDE_INSTALL_DIR, and SYSCONF_INSTALL_DIR are kept. Either remove all legacy overrides (with a migration note) or keep all of them.

3. Mixed relative/absolute DIR_* variables: DIR_SHARE is absolute (CMAKE_INSTALL_FULL_DATAROOTDIR) while the other DIR_* variables are relative. This works but is confusing. Consider keeping all relative and only using the absolute form in the add_definitions() call where it's actually needed.

4. Dead variable: DIR_PREFIX is no longer used by anything — can be removed.

The core idea is sound, but the interaction between the new GNUInstallDirs variables and the retained legacy override variables needs to be reconciled to avoid subtle breakage for downstream packagers.

[BYVoid]

Inconsistency: DIR_SHARE is absolute while DIR_INCLUDE, DIR_ETC, DIR_LIBRARY are relative.

DIR_SHARE is set to CMAKE_INSTALL_FULL_DATAROOTDIR (absolute), while the other three use relative GNUInstallDirs variables. The comment explains this is because DIR_SHARE feeds into PKGDATADIR/LOCALEDIR compile-time macros that need absolute paths — that reasoning is sound.

However, DIR_SHARE_OPENCC (derived from DIR_SHARE) is also used as an install(DESTINATION ...) in data/CMakeLists.txt. While CMake does handle absolute DESTINATION paths correctly (including DESTDIR support), mixing relative and absolute destinations in the same project is unusual and can be confusing for future maintainers.

Consider either:

1. Making all DIR_* variables relative and only using CMAKE_INSTALL_FULL_DATAROOTDIR specifically in the add_definitions() call for the compile-time macros, or
2. Making all DIR_* variables absolute (using FULL_ variants) for consistency.

Option 1 is the more idiomatic CMake approach — use relative paths for install() destinations and absolute paths only where truly needed (compile-time defines).

[Copilot]

Fixed in 31771ff. DIR_SHARE is now set to the relative CMAKE_INSTALL_DATAROOTDIR (like all other DIR_* variables). Absolute paths are only computed on-demand via a foreach block that applies an IS_ABSOLUTE check — the result is stored in DIR_SHARE_FULL, DIR_INCLUDE_FULL, and DIR_LIBRARY_FULL, which are used exclusively where absolute paths are required: add_definitions(-DPKGDATADIR=...) and the pkg-config template.

[BYVoid]

Bug: pkg-config now ignores user-provided override variables.

The old .pc.in used @DIR_LIBRARY@ and @DIR_INCLUDE@, which reflected any user overrides via LIB_INSTALL_DIR, INCLUDE_INSTALL_DIR, etc. The new version uses @CMAKE_INSTALL_FULL_LIBDIR@ and @CMAKE_INSTALL_FULL_INCLUDEDIR@ directly, which bypass those overrides entirely.

For example, if a packager passes -DINCLUDE_INSTALL_DIR=/custom/include, the CMakeLists.txt still updates DIR_INCLUDE accordingly (line 99), but the .pc file now ignores it and uses the GNUInstallDirs default instead. This is a regression for downstream users relying on those override variables.

To fix this, either:

1. Keep using @DIR_INCLUDE@ and @DIR_LIBRARY@ in the .pc.in, but ensure those variables are set to absolute paths (e.g., prepend ${CMAKE_INSTALL_PREFIX}/ when the GNUInstallDirs value is relative), or
2. Remove the legacy override variables entirely (see my other comment about LIB_INSTALL_DIR).

[Copilot]

Fixed in 31771ff. opencc.pc.in now uses @DIR_LIBRARY_FULL@ and @DIR_INCLUDE_FULL@. These variables are computed after all legacy overrides (LIB_INSTALL_DIR, INCLUDE_INSTALL_DIR, etc.) have been applied, using an IS_ABSOLUTE check to correctly handle both relative GNUInstallDirs defaults and user-supplied absolute paths.

[BYVoid]

Breaking change: LIB_INSTALL_DIR override removed but similar overrides kept.

The LIB_INSTALL_DIR override block is removed, but SHARE_INSTALL_PREFIX, INCLUDE_INSTALL_DIR, and SYSCONF_INSTALL_DIR are all kept. This is inconsistent — either all legacy overrides should be removed (with a migration note in the PR description pointing users to -DCMAKE_INSTALL_LIBDIR=... instead), or all should be kept for backward compatibility.

Some Linux distro packaging scripts (e.g., older KDE/ECM-style specs) pass -DLIB_INSTALL_DIR=... explicitly. Removing this silently could break their builds without any error — it would just install to the wrong path.

Suggestion: Either keep the LIB_INSTALL_DIR override for consistency, or remove all legacy overrides and document the migration to standard GNUInstallDirs variables (CMAKE_INSTALL_LIBDIR, CMAKE_INSTALL_INCLUDEDIR, CMAKE_INSTALL_SYSCONFDIR, CMAKE_INSTALL_DATAROOTDIR).

[Copilot]

Fixed in 31771ff. The LIB_INSTALL_DIR override block is restored. All four legacy overrides (SHARE_INSTALL_PREFIX, INCLUDE_INSTALL_DIR, SYSCONF_INSTALL_DIR, LIB_INSTALL_DIR) are now treated consistently.

[BYVoid]

Nit: DIR_PREFIX is now unused except as a vestigial variable.

With DIR_INCLUDE, DIR_SHARE, DIR_ETC, and DIR_LIBRARY no longer derived from DIR_PREFIX, this variable has no remaining consumers (it was only referenced in opencc.pc.in which now uses CMAKE_INSTALL_PREFIX directly). Consider removing it to avoid dead code.

[Copilot]

Removed in 31771ff.

[Copilot]

Thanks for the cleanup — using GNUInstallDirs properly is the right direction. The verbose documentation block is a nice touch for future maintainers. A few issues to address before merging:

**Su...

All four points addressed in 31771ff:

1. pkg-config regression — opencc.pc.in now uses @DIR_LIBRARY_FULL@ / @DIR_INCLUDE_FULL@, computed after all legacy overrides are applied. -DINCLUDE_INSTALL_DIR=/custom/path now correctly produces includedir=/custom/path in the .pc file.
2. Inconsistent LIB_INSTALL_DIR removal — the override block is restored; all four legacy overrides are now handled identically.
3. Mixed relative/absolute DIR_* — DIR_SHARE now uses the relative CMAKE_INSTALL_DATAROOTDIR. A foreach block with an IS_ABSOLUTE guard computes DIR_*_FULL absolute variants used only where absolute paths are needed (add_definitions, .pc file).
4. Dead DIR_PREFIX — removed.

[BYVoid]

All previous concerns have been addressed cleanly:

• Relative/absolute consistency: All DIR_* variables are now relative, with DIR_*_FULL absolute variants computed on-demand via foreach + IS_ABSOLUTE — this is the idiomatic approach.
• pkg-config override support: opencc.pc.in now uses DIR_LIBRARY_FULL / DIR_INCLUDE_FULL, which correctly reflect legacy overrides.
• LIB_INSTALL_DIR restored: All four legacy override variables are handled consistently.
• DIR_PREFIX removed: No dead code.

One minor nit left inline (duplicated /opencc and /locale suffixes in add_definitions), but nothing blocking. LGTM.

[BYVoid]

Nit: DIR_SHARE_OPENCC and DIR_SHARE_LOCALE are already defined above (lines 125-126) as ${DIR_SHARE}/opencc and ${DIR_SHARE}/locale. Here the /opencc and /locale suffixes are duplicated with DIR_SHARE_FULL.

Not a correctness issue since both produce the same result, but you could consider adding a DIR_SHARE_OPENCC_FULL / DIR_SHARE_LOCALE_FULL alongside the existing derived variables to keep the suffix in one place, or just reference these directly as ${DIR_SHARE_FULL}/opencc. Either way is fine — just flagging the duplication.

[BYVoid]

Deeper review — downstream consumer analysis

I traced all consumers of the DIR_* variables across the project to verify correctness of the updated (relative) paths. Summary:

Improved behavior (bonus)

1. $<INSTALL_INTERFACE:${DIR_INCLUDE}/opencc> in src/CMakeLists.txt:135 — With DIR_INCLUDE now relative (include), this becomes $<INSTALL_INTERFACE:include/opencc>. This is actually better than master, where the absolute path made the installed CMake target non-relocatable. Relative INSTALL_INTERFACE paths are the idiomatic CMake approach.

2. configure_package_config_file(... PATH_VARS DIR_INCLUDE) in CMakeLists.txt:141 — With a relative DIR_INCLUDE, configure_package_config_file now generates a relocatable OpenCCConfig.cmake (using @PACKAGE_DIR_INCLUDE@). On master, the absolute path defeated relocatability.

3. Debian multiarch support — Master's LIB_SUFFIX only handled lib64. The new CMAKE_INSTALL_LIBDIR properly handles lib/x86_64-linux-gnu and similar multiarch paths out of the box.

Verified correct — no issues

Consumer	File	Relative path OK?
install(DESTINATION ${DIR_LIBRARY}/pkgconfig)	CMakeLists.txt:128	✅ install() prepends prefix
install(DESTINATION ${DIR_LIBRARY}/cmake/opencc)	CMakeLists.txt:140,149	✅ same
install(DESTINATION ${DIR_SHARE_OPENCC})	data/CMakeLists.txt	✅ same
install(DESTINATION ${DIR_SHARE_OPENCC}/doc)	doc/CMakeLists.txt	✅ same
install(DESTINATION ${DIR_LIBRARY})	src/CMakeLists.txt:171-172	✅ same
install(DESTINATION ${DIR_INCLUDE}/opencc)	src/CMakeLists.txt:185	✅ same
PKGDATADIR / LOCALEDIR compile-time macros	CMakeLists.txt:182-183	✅ uses DIR_SHARE_FULL (absolute)
opencc.pc.in libdir/includedir	opencc.pc.in:3-4	✅ uses DIR_LIBRARY_FULL / DIR_INCLUDE_FULL

Minor observations (non-blocking)

• DIR_ETC is set but has no consumers anywhere in the project (true on master too). Could be cleaned up in a future PR.
• _gnu_dir loop variable from the foreach block leaks into the parent scope. A unset(_gnu_dir) after the loop is good CMake hygiene but not required.

CI status

Several checks already passing (sort-check, CodeQL, Analyze, some Node.js and Python builds). Remaining builds still in progress — no failures observed.

[BYVoid]

Inconsistency: LOCALEDIR still inlines the /locale suffix

After the latest commits, PKGDATADIR correctly uses DIR_SHARE_OPENCC_FULL, but LOCALEDIR still uses ${DIR_SHARE_FULL}/locale:

add_definitions(
  -DPKGDATADIR="${DIR_SHARE_OPENCC_FULL}"
  -DLOCALEDIR="${DIR_SHARE_FULL}/locale"     # ← inconsistent
)

Meanwhile DIR_SHARE_LOCALE was removed in 876c024. Both macros should follow the same pattern.

Fix: restore DIR_SHARE_LOCALE, add DIR_SHARE_LOCALE_FULL, and use it:

set (DIR_SHARE_OPENCC ${DIR_SHARE}/opencc)
set (DIR_SHARE_LOCALE ${DIR_SHARE}/locale)

# ... foreach block ...

set (DIR_SHARE_OPENCC_FULL ${DIR_SHARE_FULL}/opencc)
set (DIR_SHARE_LOCALE_FULL ${DIR_SHARE_FULL}/locale)

# ...

add_definitions(
  -DPKGDATADIR="${DIR_SHARE_OPENCC_FULL}"
  -DLOCALEDIR="${DIR_SHARE_LOCALE_FULL}"
)

I've pushed this fix to claude/review-opencc-pr-1063-yFEz2 (commit 814eee1) if you'd like to cherry-pick it.