libs/icu: fixed hardcoded path in icu-config
ClosedPublic

Authored by maxrd2 on Dec 2 2019, 5:30 PM.

Details

Summary

Current recipe generates bin/icu-config script that contains a hardcoded path inside:

default_prefix="/home/appimage/Craft/BinaryCache/linux-64-gcc"
if [ "x${prefix}" = "x" ]; then prefix="$default_prefix"; fi

Using icu-config without specifiying prefix fails. This patch solves that by replacing those lines with:

default_prefix="$KDEROOT"
if [ "x${prefix}" = "x" ]; then prefix="$default_prefix"; fi

Diff Detail

Repository
R877 Craft Blueprints for KDE
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
maxrd2 requested review of this revision.Dec 2 2019, 5:30 PM
maxrd2 created this revision.
maxrd2 added a comment.Dec 2 2019, 5:33 PM

bin/icu-config had prefix variable hardcoded like this:

default_prefix="/home/appimage/Craft/BinaryCache/linux-64-gcc"

now it's changed to following so it doesnt break dependant build scripts:

default_prefix="$KDEROOT"
maxrd2 updated this revision to Diff 70764.Dec 2 2019, 5:35 PM

corrected return value

maxrd2 added a reviewer: Craft.Dec 2 2019, 5:42 PM
maxrd2 edited the summary of this revision. (Show Details)Dec 6 2019, 10:40 AM

@vonreth added summary.. hope this describes the issue better.

maxrd2 updated this revision to Diff 71011.Dec 6 2019, 1:24 PM
maxrd2 edited the summary of this revision. (Show Details)

Using os.path.join and buildprefix instead of env var.

maxrd2 added a comment.EditedDec 6 2019, 1:35 PM

@vonreth I can't replace second parameter $KDEROOT with CraftCore.standardDirs.craftRoot() as that will just hardcode different path.
Should I replace $KDEROOT with $(realpath "$(dirname "$0")/..") if KDEROOT is not good?

But as the post install is done on every system that installs icu, it is supposed to work

maxrd2 added a comment.EditedDec 6 2019, 3:01 PM

Isn't prebuilt binary package downloaded from KDE cache?
In that case will it have hardcoded craft path from those build machines?

Yes but the post install step is is also executed on cached files

maxrd2 updated this revision to Diff 71019.Dec 6 2019, 3:23 PM

Replaced $KDEROOT with CraftCore.standardDirs.craftRoot()

maxrd2 added a comment.Dec 6 2019, 3:26 PM

ok then... thanks for the info.
Have updated the diff.

vonreth accepted this revision.Dec 6 2019, 5:00 PM
This revision is now accepted and ready to land.Dec 6 2019, 5:00 PM
maxrd2 added a comment.Dec 6 2019, 5:53 PM

I don't have permissions to land it

ah sry πŸ™ˆ

This revision was automatically updated to reflect the committed changes.

This change has broken deployment of ICU from the cached binaries, and causes a build from source to fail.

*** Action: cleanimage for libs/icu ***
*** Action: install for libs/icu ***
*** Action: post-install for libs/icu ***
File C:\Craft\CI-Qt513\windows-msvc2019_64-cl-debug\build\libs\icu\image-Debug-63.1\bin/icu-config not found.
Action: post-install for libs/icu:63.1 FAILED
*** Craft all failed: libs/icu after 2 minutes 19 seconds ***
fatal error: package libs/icu all failed
Command C:\Program Files\Python36\python.exe -u C:\Craft\CI-Qt513\windows-msvc2019_64-cl-debug\craft\bin\craft.py --option /.buildTests=False --list-file ../ci-tooling/craftmaster/packages.list failed with exit code: 1

On Windows, icu-config simply does not exist.

maxrd2 added a comment.Dec 7 2019, 1:17 AM

Have added check that file exists in https://phabricator.kde.org/D25799
It still works on linux... don't know how/where to test on windows