Update build/install instructions for custom install dir translations to work #3821
Conversation
Flameshot uses CMAKE_INSTALL_PREFIX to set APP_PREFIX. APP_PREFIX is used to determine one of the potential locations of the translations. This can be observed in PathInfo::translationsPaths, src/utils/pathinfo.cpp:26. CMAKE_INSTALL_PREFIX defaults to /usr/local on UNIX. The problem is that APP_PREFIX is set during the build -S step and changing this using build --install build --prefix ... does not actually change the value of APP_PREFIX. This leads to the translations not being found when running from a custom install location. To combat this, the --install-prefix argument is used to properly set the APP_PREFIX. This requires CMAKE 3.21. Also possible is to use -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> which would not require a newer CMAKE. Alternatives: - Get rid of APP_PREFIX and try to find the translations using path traversal. - Simplify the commands by using the CMAKE_INSTALL_PREFIX env var, which was added in 3.29 and allows getting rid of --install-prefix and --prefix. https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html - Add a symbolic link next to the binary and use the same method used to make running from the build directory possible. ln -s "$INST_DIR/bin/translations" ../share/flameshot/translations
This uses the CMAKE_INSTALL_PREFIX env var to simplify the build and install commands.
|
Thanks for the PR. Other than one shellcheck comment, the rest looks perfect. Thanks. |
|
@mmahmoudian, the potential word splitting issue? I added a commit to fix it. If you meant something else, let me know and I'll take a look |
README.md
Outdated
|
|
||
| #MacOS | ||
| cmake -S . -B "$BUILD_DIR" \ | ||
| -DQt5_DIR=$(brew --prefix qt5)/lib/cmake/Qt5 \ |
There was a problem hiding this comment.
Would you please put the value of DQt5_DIR in double quote to prevent word splitting:
| ```bash | ||
| # Best to use an absolute path here | ||
| INST_DIR=/opt/flameshot | ||
| # !Build with CMAKE_INSTALL_PREFIX and use cmake >= 3.29! Using an older cmake will cause |
There was a problem hiding this comment.
extremely minor comment, but is the exclamation part in the beginning of the sentence necessary?
|
@MatthiasKunnen Thanks for the PR. I personally tend to run the shell snippets with Thanks for yet another clean and detailed PR. It is always great to have such contributors that spend time explaining the issue in details. Cheers. |
Flameshot uses CMAKE_INSTALL_PREFIX to set APP_PREFIX.
APP_PREFIX is used to determine one of the potential locations of the translations.
This can be observed in
flameshot/src/utils/pathinfo.cpp
Line 26 in 10d12e0
CMAKE_INSTALL_PREFIX defaults to /usr/local on UNIX.
The problem is that APP_PREFIX is set during the build -S step and changing this
using build --install build --prefix ... does not actually change the value of
APP_PREFIX. This leads to the translations not being found when running
from a custom install location.
To combat this, the CMAKE_INSTALL_PREFIX env var is used to properly set
the APP_PREFIX. This requires CMAKE 3.29.
Alternatives:
-DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> which would not require a
newer CMAKE. This requires specifying the install dir twice.
to make running from the build directory possible.
ln -s "$INST_DIR/bin/translations" ../share/flameshot/translations