-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
retroarch: Install minimal assets (6MB) needed for xmb menu driver #2007
Conversation
Can be used to download specific files/folders from github repository without needing to fetch or store the entire repository.
dfa574e
to
25b9a13
Compare
* Download minimal xmb icon theme assets by default (6MB download and no .git storage necessary) * Ensure minimal assets are reinstalled after uninstalling full retroarch assets. * Add subversion to retroarch depends (for svnExport function)
Not sure it's worth installing/using subversion just for this (a helper function isn't needed as we don't use it elsewhere yet). It is easier to just archive up the minimal assets ourselves. I would prefer to be involved in the discussion before submitting PRs with changes like this (You only mentioned it 12 hours ago on the other PR and I haven't had a chance to comment.) |
I'll have a think anyway. |
I didn't think this was a major change due to the low footprint compared to our earlier expectations; subversion fetches just 2mb of debs. I added svnExport in case you could envision other uses to selectively fetch data for other modules. Anyhow, apologies, I didn't think this part was a major change requiring forum discussion, as the patch doesn't actually set xmb to the default. |
Also to note, whether you accept the PR or package assets manually, the icon assets seem duplicated in PNG and svg format. You might be able to trim down the size further by excluding one if you think 6mb is still too much. |
NP - I was mostly thinking it would be best to discuss first when you brought it up in the other PR as it could have saved some coding time etc. |
local dir="$configdir/all/retroarch/assets" | ||
local path="xmb/monochrome" | ||
# export files only if full assets are missing | ||
if [ ! -d "$dir/.git" ]; then |
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.
Just FYI we use bash [[
rather than [
local branch="$4" | ||
[[ -z "$branch" ]] && branch="trunk" | ||
|
||
runCmd svn export $repo/$branch/$path $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.
these would need quoting.
I made a couple of code comments, but I'm going to close this and think about implementing it differently (if I decide to include some assets by default - those who want to run xmb probably want to install the full assets anyway so I'm not convinced there is a big advantage of installing a minimal amount). I may add a quick option to switch to xmb, which would install assets and switch the config. |
No worries. In case you change your mind about implementing it yourself, I'll keep my branch alive for the time being so if you reopen, I can implement the changes you specified. The xmb menu driver seems to function perfectly well with just the icon theme assets; the problem with the full assets is that the .git storage is an additional 100MB (but of course, the advantage is that it can be fully updated with minimal bandwidth usage). I didn't look too closely, but if you're already packaging other raw files from repositories on files.retropie.org.uk for other modules, svnExport might help you save on your bandwidth costs. ;) |
Thanks. We don't generally package stuff up from github repositories, but we do package some assets and sourcecode (non github stuff). We are not billed based on bandwidth usage currently so it's not a problem to include some stuff on our server. We also host all the RPI binaries, so it wouldn't make much of a difference anyway :) |
BTW since SVN operates on URLs, there is no need to have in that function a split between $repo/$branch/$path etc. Just a url and a destination would suffice. |
Yep, I just split that up so that it could support similar params to gitPullorClone to keep things consistent. "svn export" is a bit odd in that you can't specify the master branch (git master maps to "trunk"), and branches need to be specified via "branches/{branchname}". |
SVN doesn't see a difference between branches / tags etc (tags and branches are the same - both are really just copies). github just choose to map it that way for their git repositories. We don't use svn enough in retropie to need it yet anyway - I prefer to keep the API as simple as posible and only add things that are used in multiple places etc. |
Quite clever how github implement both git and svn support. |
It is, but it would have been a lot easier if their https transport supported "git archive"; then we could do the same without relying on subversion mapping at all. |
For your consideration, two changes to enable fetching of minimal assets required for the xmb menu driver to function. The download and total occupied space is a mere 6MB for assets and a few extra megabytes (2MB packed) due to the additional subversion dependency.
"svn export" allows fetching of specific data from a folder of a git repository without downloading or storing the entire .git repository. This seems ideal for our purposes.
Note: "git archive" could probably achieve a similar result, but unfortunately, the github servers don't support this functionality over the https protocol.