Skip to content

Commit

Permalink
Merge branch 'addon_dep_checking'
Browse files Browse the repository at this point in the history 
Currently we don't check whether dependencies are met when installing addons.

The commits here add support for this as follows:

   1. We change repository updates to occur all at once, prior to addons being installed. This ensures that the repositories are always up to date so we don't have issues with repoA being up to date, while addons in repoA depend on repoB which is not up to date.

   2. On repo update, we check dependencies and if not found (or not installable) we mark the addon as broken, rendering it uninstallable.

   3. In addition, we check dependencies on install (which handles install from zip for instance) and fail the install for an addon if a dependency can't be installed.

   4. Lastly, we ensure that CAddonDatabase::GetAddon() returns the latest version of the addon, so that we only ever attempt to install the latest in the case of more than one version being present in repositories.
  • Loading branch information
Jonathan Marshall committed Mar 16, 2011
2 parents ce9e2ac + 12165a3 commit 917b8f6
Show file tree
Hide file tree
Showing 26 changed files with 378 additions and 181 deletions.
2 changes: 2 additions & 0 deletions language/English/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,8 @@
<string id="24041">Install from zip file</string>
<string id="24042">Downloading %i%%</string>
<string id="24043">Available Updates</string>
<string id="24044">Dependencies not met</string>
<string id="24045">Add-on does not have the correct structure</string>

<string id="24050">Available Add-ons</string>
<string id="24051">Version:</string>
Expand Down
3 changes: 2 additions & 1 deletion xbmc/Application.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@
#include "dialogs/GUIDialogSlider.h"
#include "guilib/GUIControlFactory.h"
#include "dialogs/GUIDialogCache.h"
#include "addons/AddonInstaller.h"

#ifdef HAS_PERFORMANCE_SAMPLE
#include "utils/PerformanceSample.h"
Expand Down Expand Up @@ -4809,7 +4810,7 @@ void CApplication::ProcessSlow()
#endif

if (!IsPlayingVideo())
ADDON::CAddonMgr::Get().UpdateRepos();
CAddonInstaller::Get().UpdateRepos();
}

// Global Idle Time in Seconds
Expand Down
44 changes: 37 additions & 7 deletions xbmc/addons/Addon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ CStdString AddonVersion::Print() const
AddonProps::AddonProps(const cp_extension_t *ext)
: id(ext->plugin->identifier)
, version(ext->plugin->version)
, minversion(ext->plugin->abi_bw_compatibility)
, name(ext->plugin->name)
, path(ext->plugin->plugin_path)
, author(ext->plugin->provider_name)
Expand All @@ -270,6 +271,28 @@ AddonProps::AddonProps(const cp_extension_t *ext)
EMPTY_IF("noicon",icon)
EMPTY_IF("nochangelog",changelog)
}
BuildDependencies(ext->plugin);
}

AddonProps::AddonProps(const cp_plugin_info_t *plugin)
: id(plugin->identifier)
, version(plugin->version)
, minversion(plugin->abi_bw_compatibility)
, name(plugin->name)
, path(plugin->plugin_path)
, author(plugin->provider_name)
, stars(0)
{
BuildDependencies(plugin);
}

void AddonProps::BuildDependencies(const cp_plugin_info_t *plugin)
{
if (!plugin)
return;
for (unsigned int i = 0; i < plugin->num_imports; ++i)
dependencies.insert(make_pair(CStdString(plugin->imports[i].plugin_id),
make_pair(AddonVersion(plugin->imports[i].version), plugin->imports[i].optional != 0)));
}

/**
Expand All @@ -292,6 +315,18 @@ CAddon::CAddon(const cp_extension_t *ext)
m_userSettingsLoaded = false;
}

CAddon::CAddon(const cp_plugin_info_t *plugin)
: m_props(plugin)
, m_parent(AddonPtr())
{
m_enabled = true;
m_hasSettings = false;
m_hasStrings = false;
m_checkedStrings = true;
m_settingsLoaded = false;
m_userSettingsLoaded = false;
}

CAddon::CAddon(const AddonProps &props)
: m_props(props)
, m_parent(AddonPtr())
Expand Down Expand Up @@ -330,9 +365,9 @@ AddonPtr CAddon::Clone(const AddonPtr &self) const
return AddonPtr(new CAddon(*this, self));
}

const AddonVersion CAddon::Version()
bool CAddon::MeetsVersion(const AddonVersion &version) const
{
return m_props.version;
return m_props.minversion <= version && version <= m_props.version;
}

//TODO platform/path crap should be negotiated between the addon and
Expand Down Expand Up @@ -605,11 +640,6 @@ const CStdString CAddon::LibPath() const
return URIUtils::AddFileToFolder(m_props.path, m_strLibName);
}

ADDONDEPS CAddon::GetDeps()
{
return CAddonMgr::Get().GetDeps(ID());
}

/**
* CAddonLibrary
*
Expand Down
23 changes: 19 additions & 4 deletions xbmc/addons/Addon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,25 @@ class AddonVersion
bool operator<(const AddonVersion &rhs) const;
bool operator<=(const AddonVersion &rhs) const;
CStdString Print() const;
const CStdString str;
const char *c_str() const { return str.c_str(); };
private:
CStdString str;
};

class AddonProps
{
public:
AddonProps(const CStdString &id, TYPE type, const CStdString &versionstr)
AddonProps(const CStdString &id, TYPE type, const CStdString &versionstr, const CStdString &minversionstr)
: id(id)
, type(type)
, version(versionstr)
, minversion(minversionstr)
, stars(0)
{
}

AddonProps(const cp_extension_t *ext);
AddonProps(const cp_plugin_info_t *plugin);

bool operator==(const AddonProps &rhs)
{
Expand All @@ -82,6 +86,7 @@ class AddonProps
CStdString id;
TYPE type;
AddonVersion version;
AddonVersion minversion;
CStdString name;
CStdString parent;
CStdString license;
Expand All @@ -99,6 +104,8 @@ class AddonProps
CStdString broken;
InfoMap extrainfo;
int stars;
private:
void BuildDependencies(const cp_plugin_info_t *plugin);
};

typedef std::vector<class AddonProps> VECADDONPROPS;
Expand All @@ -108,6 +115,7 @@ class CAddon : public IAddon
public:
CAddon(const AddonProps &addonprops);
CAddon(const cp_extension_t *ext);
CAddon(const cp_plugin_info_t *plugin);
virtual ~CAddon() {}
virtual AddonPtr Clone(const AddonPtr& parent) const;

Expand Down Expand Up @@ -155,7 +163,8 @@ class CAddon : public IAddon
const CStdString Name() const { return m_props.name; }
bool Enabled() const { return m_enabled; }
virtual bool IsInUse() const { return false; };
const AddonVersion Version();
const AddonVersion Version() const { return m_props.version; }
const AddonVersion MinVersion() const { return m_props.minversion; }
const CStdString Summary() const { return m_props.summary; }
const CStdString Description() const { return m_props.description; }
const CStdString Path() const { return m_props.path; }
Expand All @@ -168,7 +177,13 @@ class CAddon : public IAddon
int Stars() const { return m_props.stars; }
const CStdString Disclaimer() const { return m_props.disclaimer; }
const InfoMap &ExtraInfo() const { return m_props.extrainfo; }
ADDONDEPS GetDeps();
const ADDONDEPS &GetDeps() const { return m_props.dependencies; }

/*! \brief return whether or not this addon satisfies the given version requirements
\param version the version to meet.
\return true if min_version <= version <= current_version, false otherwise.
*/
bool MeetsVersion(const AddonVersion &version) const;

protected:
CAddon(const CAddon&); // protected as all copying is handled by Clone()
Expand Down
94 changes: 67 additions & 27 deletions xbmc/addons/AddonDatabase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ bool CAddonDatabase::CreateTables()
m_pDS->exec("CREATE TABLE addon (id integer primary key, type text,"
"name text, summary text, description text, stars integer,"
"path text, addonID text, icon text, version text, "
"changelog text, fanart text, author text, disclaimer text)\n");
"changelog text, fanart text, author text, disclaimer text,"
"minversion text)\n");

CLog::Log(LOGINFO, "create addon index");
m_pDS->exec("CREATE INDEX idxAddon ON addon(addonID)");
Expand All @@ -62,6 +63,10 @@ bool CAddonDatabase::CreateTables()
CLog::Log(LOGINFO, "create addonextra index");
m_pDS->exec("CREATE INDEX idxAddonExtra ON addonextra(id)");

CLog::Log(LOGINFO, "create dependencies table");
m_pDS->exec("CREATE TABLE dependencies (id integer, addon text, version text, optional boolean)\n");
m_pDS->exec("CREATE INDEX idxDependencies ON dependencies(id)");

CLog::Log(LOGINFO, "create repo table");
m_pDS->exec("CREATE TABLE repo (id integer primary key, addonID text,"
"checksum text, lastcheck text)\n");
Expand Down Expand Up @@ -150,6 +155,15 @@ bool CAddonDatabase::UpdateOldVersion(int version)
{
m_pDS->exec("alter table addon add disclaimer text");
}
if (version < 13)
{
m_pDS->exec("CREATE TABLE dependencies (id integer, addon text, version text, optional boolean)\n");
m_pDS->exec("CREATE INDEX idxDependencies ON dependencies(id)");
}
if (version < 14)
{
m_pDS->exec("ALTER TABLE addon add minversion text");
}
}
catch (...)
{
Expand All @@ -171,16 +185,17 @@ int CAddonDatabase::AddAddon(const AddonPtr& addon,

CStdString sql = PrepareSQL("insert into addon (id, type, name, summary,"
"description, stars, path, icon, changelog, "
"fanart, addonID, version, author, disclaimer)"
"fanart, addonID, version, author, disclaimer, minversion)"
" values(NULL, '%s', '%s', '%s', '%s', %i,"
"'%s', '%s', '%s', '%s', '%s','%s','%s','%s')",
"'%s', '%s', '%s', '%s', '%s','%s','%s','%s','%s')",
TranslateType(addon->Type(),false).c_str(),
addon->Name().c_str(), addon->Summary().c_str(),
addon->Description().c_str(),addon->Stars(),
addon->Path().c_str(), addon->Props().icon.c_str(),
addon->ChangeLog().c_str(),addon->FanArt().c_str(),
addon->ID().c_str(), addon->Version().str.c_str(),
addon->Author().c_str(),addon->Disclaimer().c_str());
addon->ID().c_str(), addon->Version().c_str(),
addon->Author().c_str(),addon->Disclaimer().c_str(),
addon->MinVersion().c_str());
m_pDS->exec(sql.c_str());
int idAddon = (int)m_pDS->lastinsertid();

Expand All @@ -193,6 +208,12 @@ int CAddonDatabase::AddAddon(const AddonPtr& addon,
sql = PrepareSQL("insert into addonextra(id, key, value) values (%i, '%s', '%s')", idAddon, i->first.c_str(), i->second.c_str());
m_pDS->exec(sql.c_str());
}
const ADDONDEPS &deps = addon->GetDeps();
for (ADDONDEPS::const_iterator i = deps.begin(); i != deps.end(); ++i)
{
sql = PrepareSQL("insert into dependencies(id, addon, version, optional) values (%i, '%s', '%s', %i)", idAddon, i->first.c_str(), i->second.first.c_str(), i->second.second ? 1 : 0);
m_pDS->exec(sql.c_str());
}
return idAddon;
}
catch (...)
Expand All @@ -209,10 +230,28 @@ bool CAddonDatabase::GetAddon(const CStdString& id, AddonPtr& addon)
if (NULL == m_pDB.get()) return false;
if (NULL == m_pDS2.get()) return false;

CStdString sql = PrepareSQL("select id from addon where addonID='%s' order by version desc",id.c_str());
// there may be multiple addons with this id (eg from different repositories) in the database,
// so we want to retrieve the latest version. Order by version won't work as the database
// won't know that 1.10 > 1.2, so grab them all and order outside
CStdString sql = PrepareSQL("select id,version from addon where addonID='%s'",id.c_str());
m_pDS2->query(sql.c_str());
if (!m_pDS2->eof())
return GetAddon(m_pDS2->fv(0).get_asInt(),addon);

if (m_pDS2->eof())
return false;

AddonVersion maxversion("0.0.0");
int maxid = 0;
while (!m_pDS2->eof())
{
AddonVersion version(m_pDS2->fv(1).get_asString());
if (version > maxversion)
{
maxid = m_pDS2->fv(0).get_asInt();
maxversion = version;
}
m_pDS2->next();
}
return GetAddon(maxid,addon);
}
catch (...)
{
Expand Down Expand Up @@ -258,7 +297,8 @@ bool CAddonDatabase::GetAddon(int id, AddonPtr& addon)
{
AddonProps props(m_pDS2->fv("addonID" ).get_asString(),
TranslateType(m_pDS2->fv("type").get_asString()),
m_pDS2->fv("version").get_asString());
m_pDS2->fv("version").get_asString(),
m_pDS2->fv("minversion").get_asString());
props.name = m_pDS2->fv("name").get_asString();
props.summary = m_pDS2->fv("summary").get_asString();
props.description = m_pDS2->fv("description").get_asString();
Expand All @@ -281,6 +321,14 @@ bool CAddonDatabase::GetAddon(int id, AddonPtr& addon)
m_pDS2->next();
}

sql = PrepareSQL("select addon,version,optional from dependencies where id=%i", id);
m_pDS2->query(sql.c_str());
while (!m_pDS2->eof())
{
props.dependencies.insert(make_pair(m_pDS2->fv(0).get_asString(), make_pair(m_pDS2->fv(1).get_asString(), m_pDS2->fv(2).get_asBool())));
m_pDS2->next();
}

addon = CAddonMgr::AddonFromProps(props);
return NULL != addon.get();
}
Expand Down Expand Up @@ -352,6 +400,8 @@ void CAddonDatabase::DeleteRepository(int idRepo)
m_pDS->exec(sql.c_str());
sql = PrepareSQL("delete from addonextra where id in (select idAddon from addonlinkrepo where idRepo=%i)",idRepo);
m_pDS->exec(sql.c_str());
sql = PrepareSQL("delete from dependencies where id in (select idAddon from addonlinkrepo where idRepo=%i)",idRepo);
m_pDS->exec(sql.c_str());
sql = PrepareSQL("delete from addonlinkrepo where idRepo=%i",idRepo);
m_pDS->exec(sql.c_str());

Expand Down Expand Up @@ -549,7 +599,7 @@ void CAddonDatabase::SetPropertiesFromAddon(const AddonPtr& addon,
pItem->SetProperty("Addon.Type", TranslateType(addon->Type(),true));
pItem->SetProperty("Addon.intType", TranslateType(addon->Type()));
pItem->SetProperty("Addon.Name", addon->Name());
pItem->SetProperty("Addon.Version", addon->Version().str);
pItem->SetProperty("Addon.Version", addon->Version().c_str());
pItem->SetProperty("Addon.Summary", addon->Summary());
pItem->SetProperty("Addon.Description", addon->Description());
pItem->SetProperty("Addon.Creator", addon->Author());
Expand Down Expand Up @@ -616,29 +666,19 @@ bool CAddonDatabase::DisableAddon(const CStdString &addonID, bool disable /* = t
return false;
}

bool CAddonDatabase::BreakAddon(const CStdString &addonID, bool broken /* = true */, const CStdString& reason)
bool CAddonDatabase::BreakAddon(const CStdString &addonID, const CStdString& reason)
{
try
{
if (NULL == m_pDB.get()) return false;
if (NULL == m_pDS.get()) return false;

if (broken)
{
CStdString sql = PrepareSQL("select id from broken where addonID='%s'", addonID.c_str());
m_pDS->query(sql.c_str());
if (m_pDS->eof()) // not found
{
m_pDS->close();
sql = PrepareSQL("insert into broken(id, addonID, reason) values(NULL, '%s', '%s')", addonID.c_str(),reason.c_str());
m_pDS->exec(sql);
return true;
}
return false; // already disabled or failed query
}
else
{
CStdString sql = PrepareSQL("delete from broken where addonID='%s'", addonID.c_str());
CStdString sql = PrepareSQL("delete from broken where addonID='%s'", addonID.c_str());
m_pDS->exec(sql);

if (!reason.IsEmpty())
{ // broken
sql = PrepareSQL("insert into broken(id, addonID, reason) values(NULL, '%s', '%s')", addonID.c_str(),reason.c_str());
m_pDS->exec(sql);
}
return true;
Expand Down
9 changes: 4 additions & 5 deletions xbmc/addons/AddonDatabase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,12 @@ class CAddonDatabase : public CDatabase
bool HasDisabledAddons();

/*! \brief Mark an addon as broken
Sets a flag that this addon has been marked as broken in the repository.
Sets a flag that this addon has been marked as broken in the repository.
\param addonID id of the addon to mark as broken
\param broken whether to mark or not. Defaults to true
\param reason why it is broken. Defaults to blank
\param reason why it is broken - if non empty we take it as broken. Defaults to empty
\return true on success, false on failure
\sa IsAddonBroken */
bool BreakAddon(const CStdString &addonID, bool broken = true, const CStdString& reason="");
bool BreakAddon(const CStdString &addonID, const CStdString& reason="");

/*! \brief Check whether an addon has been marked as broken via BreakAddon.
\param addonID id of the addon to check
Expand All @@ -98,7 +97,7 @@ class CAddonDatabase : public CDatabase
protected:
virtual bool CreateTables();
virtual bool UpdateOldVersion(int version);
virtual int GetMinVersion() const { return 12; }
virtual int GetMinVersion() const { return 14; }
const char *GetBaseDBName() const { return "Addons"; }
};

Loading

0 comments on commit 917b8f6

Please sign in to comment.