Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Luckybackup, new recipe #3183
Luckybackup, new recipe #3183
Changes from all commits
8ac8c62
a64d58a
183aa98
20a230e
be21a47
bee0978
9339e68
9337421
5851498
75f2c14
22b56ff
45891d3
e5e4453
da5a621
5cba9dd
bbe6549
82b1509
7a32561
87d2dae
ecedbcb
7452f5c
2f91957
b325d46
098f6b1
091fd49
9a7c49c
6020b8c
9f998fd
19ce708
112362e
5907c98
9ced35b
4243ab5
06e713a
d0a22a7
c010553
3416276
b80dbc4
258de93
9644cf3
86773a6
ab40e08
a2bcd33
332c080
6bb682a
e2aff5d
52e2688
7061673
cd43d17
7c3c6af
9b4020d
6f1c39c
f8044a3
895ac42
e64910d
a0da9b4
932c999
735a43e
a2f42a8
7eb23b5
e7f7514
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fbrosson
Are there any reasons why these can't be changed into
@Template@
which then you cansed
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 usingfind_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 inglobal.cpp
would be unusable. That's why I believe the app does not change dir.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really,
/package/name-version/.self/
stay the same regardless of installation location. Also, there'sfind_path
if you want the fancy/boot/{home,system}
paths.