added: housekeeping code for addons/packages #323

Merged
1 commit merged into from Oct 2, 2012

Conversation

Projects
None yet
6 participants
@ghost

ghost commented Aug 3, 2011

this does housekeeping, trying to never go above a given folder size (200MB by default).

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 4, 2011

@jmarshallnz; all your comments taken into consideration. ok to pull?

ghost commented Aug 4, 2011

@jmarshallnz; all your comments taken into consideration. ok to pull?

@CrystalP

View changes

xbmc/addons/AddonInstaller.cpp
+ for (std::map<CStdString,CFileItemList*>::iterator it = packs.begin();
+ it != packs.end();++it)
+ {
+ if (it->second.Size() > 1)

This comment has been minimized.

@CrystalP

CrystalP Aug 7, 2011

Contributor

build error, should be it->second->Size()

@CrystalP

CrystalP Aug 7, 2011

Contributor

build error, should be it->second->Size()

@CrystalP

View changes

xbmc/addons/AddonInstaller.cpp
+ {
+ it->second->Sort(SORT_METHOD_LABEL,SORT_ORDER_DESC);
+ for (int j=2;j<it->second->Size();++j)
+ items.Add(CFileItemPtr(new CFileItem(*it->second->Get(j))));

This comment has been minimized.

@CrystalP

CrystalP Aug 7, 2011

Contributor

That leaves 2 packages, not one as the comment says. I think it makes more sense with 2 as you did here, to always keep the last version and the previous one.

@CrystalP

CrystalP Aug 7, 2011

Contributor

That leaves 2 packages, not one as the comment says. I think it makes more sense with 2 as you did here, to always keep the last version and the previous one.

This comment has been minimized.

@ghost

ghost Aug 8, 2011

we can't invent packages for stuff we only have one for. granted, comment is probably a bit confusing as it's not what the code itself does so shall change it.

@ghost

ghost Aug 8, 2011

we can't invent packages for stuff we only have one for. granted, comment is probably a bit confusing as it's not what the code itself does so shall change it.

@CrystalP

View changes

xbmc/addons/AddonInstaller.cpp
+ it != packs.end();++it)
+ {
+ if (it->second.Size() > 1)
+ items.Add(CFileItemPtr(new CFileItem(*it->second->Get(1))));

This comment has been minimized.

@CrystalP

CrystalP Aug 7, 2011

Contributor

Shouldn't this be a c/p of lines 345-346 (except with "int j=1"), to catch ALL versions, except for the last one?
edit: nm, they've all been deleted in pass1.

@CrystalP

CrystalP Aug 7, 2011

Contributor

Shouldn't this be a c/p of lines 345-346 (except with "int j=1"), to catch ALL versions, except for the last one?
edit: nm, they've all been deleted in pass1.

@CrystalP

This comment has been minimized.

Show comment
Hide comment
@CrystalP

CrystalP Aug 7, 2011

Contributor

Regardless of disk space, I'd personally like to always preserve the two most recent versions per addon, to always have the ability to rollback. I'm comfortable enough that two broken releases in a row are quite unlikely.

One suggestion: if that's not done somewhere else already, use this function to delete the packages of uninstalled addons?

Contributor

CrystalP commented Aug 7, 2011

Regardless of disk space, I'd personally like to always preserve the two most recent versions per addon, to always have the ability to rollback. I'm comfortable enough that two broken releases in a row are quite unlikely.

One suggestion: if that's not done somewhere else already, use this function to delete the packages of uninstalled addons?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 8, 2011

hmm.. i sorta agree and sorta disagree. i guess we could change the space limit to be a 'desired max', while allowing more usage to keep two archives around of all.

i was pondering using it to nuke uninstalled addons. but sometimes quick re-adding is also nice. other opinions on that around? i'm fine with going with the consensus..

ghost commented Aug 8, 2011

hmm.. i sorta agree and sorta disagree. i guess we could change the space limit to be a 'desired max', while allowing more usage to keep two archives around of all.

i was pondering using it to nuke uninstalled addons. but sometimes quick re-adding is also nice. other opinions on that around? i'm fine with going with the consensus..

@vguerci

This comment has been minimized.

Show comment
Hide comment
@vguerci

vguerci Mar 4, 2012

Was guessing why a backup was big&slow, just figured addons/packages grew to 830MB over the time...
I'll give it a try and hope it will be merged some day :)
EDIT: Works perfectly, dropped to 166MB as expected, thanks @cptspiff

vguerci commented Mar 4, 2012

Was guessing why a backup was big&slow, just figured addons/packages grew to 830MB over the time...
I'll give it a try and hope it will be merged some day :)
EDIT: Works perfectly, dropped to 166MB as expected, thanks @cptspiff

@amet

This comment has been minimized.

Show comment
Hide comment
@amet

amet Sep 10, 2012

Contributor

@cptspiff will this go in at any point, my macmini would love you for it.. it has years of updates :)

Contributor

amet commented Sep 10, 2012

@cptspiff will this go in at any point, my macmini would love you for it.. it has years of updates :)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 10, 2012

i have done what I am willing to do but it was rejected as can be seen from the comments here.

ghost commented Sep 10, 2012

i have done what I am willing to do but it was rejected as can be seen from the comments here.

@MartijnKaijser

This comment has been minimized.

Show comment
Hide comment
@MartijnKaijser

MartijnKaijser Sep 10, 2012

Member

@cptspiff
I can't see any comments on why it couldn't be merged or am i missing something?
With the merging an small form HTPC like rpi and the android boxes this would be a very welcome addition to clean disk space

Member

MartijnKaijser commented Sep 10, 2012

@cptspiff
I can't see any comments on why it couldn't be merged or am i missing something?
With the merging an small form HTPC like rpi and the android boxes this would be a very welcome addition to clean disk space

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 10, 2012

crystal p is not satisfied.

ghost commented Sep 10, 2012

crystal p is not satisfied.

@jester-xbmc

This comment has been minimized.

Show comment
Hide comment
@jester-xbmc

jester-xbmc Sep 10, 2012

Maybe an ifdef for certain platforms? look at ATV2, lots of questions on the forum about having no space and us pointing out to clean up (it's on the wiki but still) (same goes for higher database revs on upgrade, the old ones stay behind and fill up fast on big databases, but thats off-topic)

Maybe an ifdef for certain platforms? look at ATV2, lots of questions on the forum about having no space and us pointing out to clean up (it's on the wiki but still) (same goes for higher database revs on upgrade, the old ones stay behind and fill up fast on big databases, but thats off-topic)

@NedScott

This comment has been minimized.

Show comment
Hide comment
@NedScott

NedScott Oct 1, 2012

Contributor

We NEED this for Frodo. Even if it defaults to off and is turned on in advancedsettings.xml

Contributor

NedScott commented Oct 1, 2012

We NEED this for Frodo. Even if it defaults to off and is turned on in advancedsettings.xml

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Oct 2, 2012

@CrystalP, popular demand.. can be revised later.

ghost commented Oct 2, 2012

@CrystalP, popular demand.. can be revised later.

ghost pushed a commit that referenced this pull request Oct 2, 2012

Arne Morten Kvarving
Merge pull request #323 from cptspiff/cleanpkgs
added: housekeeping code for addons/packages

@ghost ghost merged commit ed0641f into xbmc:master Oct 2, 2012

tru added a commit to plexinc/plex-home-theater-public that referenced this pull request Jan 11, 2013

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment