Luckybackup, new recipe #3183
Luckybackup, new recipe #3183
Conversation
merge master haikuports to my ports
get change from master
pull from master fork
pull from master
merge from master
|
LGTM, as reported the issues comming up are mostly gone when launched from Terminal in $appsDir, Maybe you could open an issue for the remaining ones? |
|
Sorry @mazbrili and @Begasus for this late reply. Well, I don't really know what to do with the missing icon. Someone would have to dig this topic... Shall we add a "help-wanted" label to this PR? |
|
Wrap it up best you can. Create a new issues for the open issues, and merge it. We can make the open issues into GCI tasks. |
There was a problem hiding this comment.
the issues comming up are mostly gone when launched from Terminal in
$appsDir
This is because the program looks for it's resources in the current working folder which is a huge bug and have to be fixed.
If we wanted to merge this, please disable the recipe on all architecture, then open an issue to fix it.
|
i'm found path is hardcoded on source .. need assist to fix it |
|
@mazbrili, in #3183 (comment) I was suggesting to patch the file Have you noticed that haikuporter generates a |
..thank you for your suggestion... i'm sure it can be done.. but need time to do that.. |
set current dir to app dir
add patch set currentdir to application dir
|
i've been make patch.. tested.. ok |
|
@mazbrili, I just pushed a tiny commit to remove a few unneeded lines in the patchset. Regarding the icon not being shown, this is a minor issue which can be fixed later. That said, there is no urgency, so if you would like to try to fix this before the merge, you are welcome. I think we'll probably merge before next tuesday in order to create a task for the Google Code-in contest it if the issue is not fixed yet. Regarding the Finally, regarding the installation directories, I'd like to suggest a few neutral changes. But I'll do so in a separate comment. |
|
I did some checking, but haven't found a sollution yet also (ps, not familiar with the wrapper.script yet, so I need some investigation on that part) :) |
|
Here are the changes I have in mind:
I believe these changes can be obtained by patching the - addResourcesToBinaries luckybackup.rdef "$appsDir"/LuckyBackup/LuckyBackup
+ addResourcesToBinaries luckybackup.rdef "$appsDir"/LuckyBackup
- addAppDeskbarSymlink "$appsDir"/LuckyBackup/LuckyBackup
+ addAppDeskbarSymlink "$appsDir"/LuckyBackupThe advantages of this layout are that:
That's why I think we should try to use the installation directories described above. EDIT: Updated comment to have "LuckyBackup" instead of "luckybackup". |
|
Please use LuckyBackup as a binary name. |
This comment has been minimized.
This comment has been minimized.
|
If the binary would end up in $binDir I would be ok to leave as is and only rename it in the DeskbarMenu, if it ends up in $appsDir I'm for using LuckyBackup |
|
@fbrosson your explanation is very good.. i'm not yet familiar with haiku system hierarchy.. that change is most suitable i think.. but right now i'm still learning to achieve like that.. |
| # copy translation | ||
| cp -rd ./translations/*.qm "$appsDir"/LuckyBackup/translations/ | ||
| # copy license | ||
| cp -rd ./license/* "$appsDir"/LuckyBackup/license/ |
There was a problem hiding this comment.
I was wondering if we could use make install and it turns out that we can do that if we patch the luckybackup.pro. I tried something and it allows to simplify INSTALL and switch to the layout I was suggesting... but the bad news is that the issue about the license not being displayed (Could not locate the license file!) is back...
And the "Help" > "LuckyBackup Handbook" menu displays an error:
ERROR
Could not locate the file: manual/index.html
(I did not check if it was OK without my changes.)
Anyway, this would require more digging...
So I don't know if this is worth the trouble.
Maybe we should just investigate about the settings file.
cmd:qmake$secondaryArchSuffix >= 5
"
+PATCH()
+{
+ sed -i \
+ -e "s|^\(documentation\.path =\).*|\1 $docDir|" \
+ -e "s|^\(translations\.path =\).*|\1 $dataDir/luckybackup|" \
+ -e "s|^\(manpage\.path =\).*|\1 $manDir/man8|" \
+ -e "s|^\(license\.path =\).*|\1 $docDir|" \
+ luckybackup.pro
+}
+
BUILD()
{
qmake luckybackup.pro
make $jobArgs
}
INSTALL()
{
- #create directories
- mkdir -p "$appsDir"/LuckyBackup/{manual,translations,license}
- #install
- install -T ./luckybackup "$appsDir"/LuckyBackup/LuckyBackup
-
- # copy manpages
- mkdir -p "$manDir"/man8
- cp -r ./manpage/*.8 "$manDir"/man8/
-
- # copy html documentation
- cp -r ./manual/* "$appsDir"/LuckyBackup/manual/
- # copy translation
- cp -rd ./translations/*.qm "$appsDir"/LuckyBackup/translations/
- # copy license
- cp -rd ./license/* "$appsDir"/LuckyBackup/license/
+ make install
+ install -d "$appsDir"
+ install -T luckybackup "$appsDir"/LuckyBackup
+ gunzip "$manDir"/man8/luckybackup.8.gz
local APP_SIGNATURE="application/x-vnd.LuckyBackup"
local MAJOR="`echo "$portVersion" | cut -d. -f1`"
local MIDDLE="`echo "$portVersion" | cut -d. -f2`"
local MINOR="`echo "$portVersion" | cut -d. -f3`"
local LONG_INFO="$SUMMARY"
sed \
-e "s|@APP_SIGNATURE@|$APP_SIGNATURE|" \
-e "s|@MAJOR@|$MAJOR|" \
-e "s|@MIDDLE@|$MIDDLE|" \
-e "s|@MINOR@|$MINOR|" \
-e "s|@LONG_INFO@|$LONG_INFO|" \
"$portDir"/additional-files/luckybackup.rdef.in > luckybackup.rdef
- addResourcesToBinaries luckybackup.rdef "$appsDir"/LuckyBackup/LuckyBackup
- addAppDeskbarSymlink "$appsDir"/LuckyBackup/LuckyBackup
+ addResourcesToBinaries luckybackup.rdef "$appsDir"/LuckyBackup
+ addAppDeskbarSymlink "$appsDir"/LuckyBackup
}
There was a problem hiding this comment.
Maybe change:
apps/LuckyBackup/license/➡️data/luckybackup/
into ?apps/LuckyBackup/license/➡️data/luckybackup/license/
Also with sed you used $docDir?
There was a problem hiding this comment.
src/global.cpp had everything we needed to fix most issues. 😄
|
The only remaining issue is the icon which is not being displayed. |
| -QString relativeManual = "manual/index.html"; | ||
| -QString systemManual = "/usr/share/doc/luckybackup/manual/index.html"; | ||
| +QString relativeManual = "../data/luckybackup/manual/index.html"; | ||
| +QString systemManual = "/boot/system/data/luckybackup/manual/index.html"; |
There was a problem hiding this comment.
Are there any reasons why these can't be changed into @Template@ which then you can sed in the recipe?
I'm a bit concerned about the "relative" paths, what exactly are they relative to? As seen last time, they might be relative to the current working dir, which may causes unintended content injection. I'd like to get rid of these relative paths instead.
There was a problem hiding this comment.
Yeah, we could use patterns and have INSTALL replace them with sed commands, but since this recipe will sooner or later get a much nicer patch using find_directory(), I thought that a few extra hard-coded paths were OK for now. (Adding a sed command that would probably go away during GCI 2018 would have been worse, imho, than using hard-coded paths, but I'm ok with anyone willing to rework this :)
Regarding the relative paths, I believe they are relative to /boot/system/apps/ because that's where the app "changes dir" at start-up (thanks to @mazbrili's patch.)
Regarding the unintended risk of injection, although I did not review the code to see if the app changes dir later, I'm confident that it does not, so the users are probably safe here. BTW, if the app did change dir after start-up to any directory other than that of argv[0] then all the relative paths present in global.cpp would be unusable. That's why I believe the app does not change dir.
There was a problem hiding this comment.
All apps will have to tolerate relative paths eventually, as they can get installed in HOME as well as /system/.
There was a problem hiding this comment.
Not really, /package/name-version/.self/ stay the same regardless of installation location. Also, there's find_path if you want the fancy /boot/{home,system} paths.
icon is not good.. i'm not artist