Rockbox

Tasklist

FS#11575 - RaaA should not be storing the real full path in config.cfg

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Sunday, 29 August 2010, 02:41 GMT
Last edited by Jonathan Gordon (jdgordon) - Tuesday, 07 December 2010, 00:29 GMT
Task Type Bugs
Category Settings
Status Closed
Assigned To Thomas Martitz (kugel.)
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

from my config.cfg
"
playlist catalog directory: /usr/local/share/rockbox/Playlis
"
/usr/local/share/rockbox should be added at runtime, and stored speratly or the actual path is cut off
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Tuesday, 07 December 2010, 00:29 GMT
Reason for closing:  Fixed
Additional comments about closing:  r28752
Comment by Nils Wallménius (nls) - Sunday, 29 August 2010, 06:55 GMT
is there any reason why we limit this path to 32 chars except to save some memory?
Comment by Jonathan Gordon (jdgordon) - Sunday, 29 August 2010, 07:09 GMT
nope, but then there is another more serious problem which wasnt obvious when I filed this... the Playlists directory should be in the user writable area so it should be ~/.config/rockbox.org/Playlists by default.
Comment by Thomas Martitz (kugel.) - Sunday, 29 August 2010, 10:18 GMT
I think it actually is, it's supposed to be anyways.

"/usr/local/share/rockbox/xxx" is translated into $HOME/.config/rockbox.org/xxx (if it exists) at even though it says /usr/local/share.

The path doesn't mean much anyway. Only the the basename of the file (without path and extensions) is actually stored in RAM. From there on it looks in $HOME/.config/rockbox.org/xxx, then /usr/local/share/rockbox for the files/folders.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 13 October 2010, 13:13 GMT
not entirely working version. wpsbuild.pl needs to be fixed so the cabbie theme uses the correct path prefix. I'm not sure the autoconf.h change is correct also.
Comment by Thomas Martitz (kugel.) - Wednesday, 13 October 2010, 13:17 GMT
What does that patch? I thought the bug this task is about was fixed.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 13 October 2010, 13:22 GMT
config.cfg storing real paths which it shouldnt. it should actually store them in the same way regular targets do (i.e wps: /.rockbox/wps/cabbiev2.wps) so you could take a .cfg from your dap and use it on raaa. right now its using /<Rb>/ but the prefix is irrelevant. the important thing is it shouldnt be storing /usr/local/....
Comment by Thomas Martitz (kugel.) - Wednesday, 13 October 2010, 13:25 GMT
As I said in my previous post, it doesn't actually use the path from the .cfg. It actually ignores the path (and extension) and only looks at the basefile name in the usual search path order.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 13 October 2010, 22:08 GMT
that isnt 100% true as I found out. that full path is currently used if the get_user_file_dir() isnt called correctly (like forgetting IS_FILE). and anyway that is more reason to not put the full path in the config.
Comment by Jonathan Gordon (jdgordon) - Thursday, 14 October 2010, 11:14 GMT
cleaner version which make ROCKBOX_DIR point back to the same as normal builds, ROCKBOX_SHARE_PATH is the old ROCKBOX_DIR. doing it this way means no scripts need to be fixed to work, and configs can be shared without any problem.

I find it a bit annoying that you really need 2 MAX_PATH buffers in the caller to make it work though.
Comment by Jonathan Gordon (jdgordon) - Monday, 18 October 2010, 12:59 GMT
This is more of a proof of concept than committable patch... remove all get_user_file_path() calls in apps/ and transparently change the filename in open() and opendir() to get the right path. sdl app only just now
Comment by Jonathan Gordon (jdgordon) - Sunday, 24 October 2010, 10:55 GMT
This changes the android path to /data/data/org.rockbox/.rockbox amd transparently changes file paths called with open() and diropen().
tested with android and sdl.

make reconf doesnt work though and that needs to be fixed before commiting
Comment by Jonathan Gordon (jdgordon) - Sunday, 24 October 2010, 11:34 GMT
this version fixes make reconf. please test
Comment by Jonathan Gordon (jdgordon) - Monday, 25 October 2010, 12:38 GMT
sync
Comment by Thomas Martitz (kugel.) - Sunday, 05 December 2010, 19:36 GMT
Sync and extended to wrap all IO functions that take a path.

Works very well, I think I'm going to commit in a day or two.

Loading...