BitcoinArmory icon indicating copy to clipboard operation
BitcoinArmory copied to clipboard

Allow specifying custom path to lrelease binary

Open mplucinski opened this issue 8 years ago • 5 comments

This patch allows a user to specify custom path to lrelease tool during configuration phase. It's necessary e.g. in Fedora, where the tool is called "lrelease-qt4". When using the patch, in case if the user specifies LRELEASE_PATH, the search phase is skipped and it's assumed that the user-provided path is correct. If the variable isn't specified, the search should work like before.

mplucinski avatar Dec 20 '17 13:12 mplucinski

Hmmm. Is there another way to do this? I don't like the idea of forcing people to add LRELEASE_PATH to their configs. macOS users would have to do it. I can't speak for anyone else but suspect others would run into similar issues.

droark avatar Dec 20 '17 23:12 droark

lrelease is to package the translation stuff for Qt4. I'm wondering if it's not preferable to just skip the test if lrelease is missing.

goatpig avatar Dec 21 '17 01:12 goatpig

I'm sorry if I didn't write it clearly, but (AFAIK, I'm not in any way an autotools-expert) with my patch, the search for a binary should work like before, unless the variable is specified. I don't think it'd affect the people for whom the current setup works.

EDIT: I just checked, and it behaves like I described: in case if LRELEASE_PATH isn't specified in the environment, the search works like before, i.e. searches in $PATH for "lrelease".

mplucinski avatar Dec 21 '17 13:12 mplucinski

On macOS, this patch fails.

checking for lrelease... no
configure: error: missing lrelease in path, make sure qt4-linguist-tools is installed or specify LRELEASE_PATH

Granted, I am running this from the command line directly, which isn't how Armory is currently built on Macs. (I have a WIP patch to overhaul all that, and could try this patch against it.) I'm sure I could integrate LRELEASE_PATH into the Mac build script. I just don't like the idea of "intrusive" patches like these unless it's proven that, say, my system's an outlier, while virtually everybody else is dandy.

Have you tried this patch on Ubuntu, Debian, and perhaps Fedora? If they're happy, and maybe my WIP patch is happy, I suppose I don't have much of a leg to stand on.

droark avatar Dec 21 '17 17:12 droark

Maybe amend the PR it to just bypass the use of lrelease if it can't be detected nor is user specified?

goatpig avatar Dec 21 '17 17:12 goatpig