Greetings,
Here's my implementation of your great idea. It adds
CompPluginRuleRequire, that allows a plugin to depend on a feature,
and two items in the plugin VTable, features and nFeatures. I know I
could have merged the features and the dependencies systems, but it
looks much cleaner (a feature is not actually a dep).
Plugins have been modified consequently (adding the new VTable items),
and Cube, Plane and Zoom plugins now use the new Features code.
I did my best to follow your coding style and indentation guidelines,
hopefully it'll fit your requirements.
As a side note, I'd suggest removing the "Can't activate plugin due to
dependency problems" fprintf, since it's just repeating the error
already printed in checkPluginDeps function.
Regards,
Guillaume Seguin
2006/10/7, Guillaume <[EMAIL PROTECTED]>:
Hello,
We already done something similar in beryl to fix these conflicting
plugins issues. What we did was to introduce two new rules,
CompPluginRuleConflicts and CompPluginRuleAfterAlt. The first one
prevents two incompatible plugin from being loaded at once, and the
later one allows a plugin to require only one plugin in a list of
alternatives.
Anyway, I really like your Feature idea, it's probably the best way to
handle such problems, and it'd be much more flexible than my silly
alternatives rule (since with Features one plugin may need several
Features).
About the implementation, I just wonder if it'd be better to extend
the current Dependencies system with two new rules
(CompPluginRuleProvideFeature and CompPluginRuleNeedFeature) or extend
the plugin API as you suggested. Both approachs sound acceptable.
If you're ok with that, I'll write a first draft tomorrow.
Regards,
Guillaume Seguin
2006/10/7, David Reveman <[EMAIL PROTECTED]>:
> The dependency checking currently provided is clearly not good enough.
> There's currently two issues with the plugins that exist in the compiz
> repository.
>
> plane plugin conflicts with cube and rotate plugin, zoom plugin should
> probably work with either cube or plane but currently only loads when
> cube plugin is used.
>
> Adding a
> { CompPluginRuleBefore, "cube" }
> to the plane plugin and a
> { CompPluginRuleBefore, "plane" }
> to the cube plugin will solve the conflict but it's not perfect as it
> means that all plugins sort of needs to be aware of all other plugins.
>
> Having the zoom plugin load if either the cube plugin or the plane
> plugin is loaded is currently not possible.
>
> I'm suggesting that we add some way to register and depend on features.
> E.g. the cube and plane plugins would both register a "largedesktop"
> feature. zoom plugin could depend on the "largedesktop" feature instead
> of the cube plugin and it wouldn't be possible to load two plugins that
> provide the same feature.
>
> Adding a feature list to the plugin vTable and a
> CompPluginRuleNeedFeature rule that can be used to depend on specific
> features should do it.
>
> -David
>
> _______________________________________________
> compiz mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/compiz
>
diff --git a/include/compiz.h b/include/compiz.h
index 9529a17..873c98b 100644
--- a/include/compiz.h
+++ b/include/compiz.h
@@ -26,7 +26,7 @@
#ifndef _COMPIZ_H
#define _COMPIZ_H
-#define ABIVERSION 20060927
+#define ABIVERSION 20061008
#include <stdio.h>
#include <sys/time.h>
@@ -2124,7 +2124,8 @@ typedef void (*FiniPluginProc) (CompPlug
typedef enum {
CompPluginRuleBefore,
- CompPluginRuleAfter
+ CompPluginRuleAfter,
+ CompPluginRuleRequire
} CompPluginRule;
typedef struct _CompPluginDep {
@@ -2132,6 +2133,10 @@ typedef struct _CompPluginDep {
char *plugin;
} CompPluginDep;
+typedef struct _CompPluginFeature {
+ char *name;
+} CompPluginFeature;
+
typedef struct _CompPluginVTable {
char *name;
char *shortDesc;
@@ -2158,6 +2163,9 @@ typedef struct _CompPluginVTable {
CompPluginDep *deps;
int nDeps;
+
+ CompPluginFeature *features;
+ int nFeatures;
} CompPluginVTable;
typedef CompPluginVTable *(*PluginGetInfoProc) (void);
@@ -2213,6 +2221,9 @@ CompPlugin *
findActivePlugin (char *name);
CompPlugin *
+findFeature (char *name);
+
+CompPlugin *
loadPlugin (char *plugin);
void
diff --git a/plugins/cube.c b/plugins/cube.c
index ab46c89..80bed2c 100644
--- a/plugins/cube.c
+++ b/plugins/cube.c
@@ -1738,6 +1738,10 @@ CompPluginDep cubeDeps[] = {
{ CompPluginRuleBefore, "switcher" }
};
+CompPluginFeature cubeFeatures[] = {
+ { "extendedDesktop" }
+};
+
static int
cubeGetVersion (CompPlugin *plugin,
int version)
@@ -1763,7 +1767,9 @@ CompPluginVTable cubeVTable = {
cubeGetScreenOptions,
cubeSetScreenOption,
cubeDeps,
- sizeof (cubeDeps) / sizeof (cubeDeps[0])
+ sizeof (cubeDeps) / sizeof (cubeDeps[0]),
+ cubeFeatures,
+ sizeof (cubeFeatures) / sizeof (cubeFeatures[0])
};
CompPluginVTable *
diff --git a/plugins/dbus.c b/plugins/dbus.c
index 46a7bcf..8000787 100644
--- a/plugins/dbus.c
+++ b/plugins/dbus.c
@@ -864,7 +864,9 @@ CompPluginVTable dbusVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/decoration.c b/plugins/decoration.c
index 0bac239..963491e 100644
--- a/plugins/decoration.c
+++ b/plugins/decoration.c
@@ -1368,7 +1368,9 @@ static CompPluginVTable decorVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
decorDeps,
- sizeof (decorDeps) / sizeof (decorDeps[0])
+ sizeof (decorDeps) / sizeof (decorDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/fade.c b/plugins/fade.c
index 5cccbea..941ab87 100644
--- a/plugins/fade.c
+++ b/plugins/fade.c
@@ -837,7 +837,9 @@ static CompPluginVTable fadeVTable = {
fadeGetScreenOptions,
fadeSetScreenOption,
fadeDeps,
- sizeof (fadeDeps) / sizeof (fadeDeps[0])
+ sizeof (fadeDeps) / sizeof (fadeDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/gconf-dump.c b/plugins/gconf-dump.c
index 35a2139..408f23a 100644
--- a/plugins/gconf-dump.c
+++ b/plugins/gconf-dump.c
@@ -841,7 +841,9 @@ CompPluginVTable gconfDumpVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
0,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/gconf.c b/plugins/gconf.c
index c60225c..ebf9c33 100644
--- a/plugins/gconf.c
+++ b/plugins/gconf.c
@@ -1065,7 +1065,9 @@ CompPluginVTable gconfVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
gconfDeps,
- sizeof (gconfDeps) / sizeof (gconfDeps[0])
+ sizeof (gconfDeps) / sizeof (gconfDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/minimize.c b/plugins/minimize.c
index c14b194..6e278a3 100644
--- a/plugins/minimize.c
+++ b/plugins/minimize.c
@@ -1065,7 +1065,9 @@ static CompPluginVTable minVTable = {
minGetScreenOptions,
minSetScreenOption,
minDeps,
- sizeof (minDeps) / sizeof (minDeps[0])
+ sizeof (minDeps) / sizeof (minDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/move.c b/plugins/move.c
index 399e399..5a017eb 100644
--- a/plugins/move.c
+++ b/plugins/move.c
@@ -742,7 +742,9 @@ CompPluginVTable moveVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/place.c b/plugins/place.c
index 1daeef6..3581acc 100644
--- a/plugins/place.c
+++ b/plugins/place.c
@@ -1164,7 +1164,9 @@ static CompPluginVTable placeVTable = {
placeGetScreenOptions,
placeSetScreenOption,
0,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/plane.c b/plugins/plane.c
index ad62497..6f81480 100644
--- a/plugins/plane.c
+++ b/plugins/plane.c
@@ -861,12 +861,8 @@ planeFini (CompPlugin *p)
freeDisplayPrivateIndex (displayPrivateIndex);
}
-CompPluginDep planeDeps[] = {
-#if 0
- /* We need a new CompPluginRuleConflicts */
- { CompPluginRuleConflicts, "rotate" },
- { CompPluginRuleConflicts, "cube" },
-#endif
+CompPluginFeature planeFeatures[] = {
+ { "extendedDesktop" }
};
static int
@@ -893,8 +889,10 @@ CompPluginVTable planeVTable = {
planeSetDisplayOption,
NULL, /* planeGetScreenOptions, */
NULL, /* planeSetScreenOption, */
- planeDeps,
- sizeof (planeDeps) / sizeof (planeDeps[0])
+ NULL, /* Deps */
+ 0, /* nDeps */
+ planeFeatures,
+ sizeof (planeFeatures) / sizeof (planeFeatures[0])
};
CompPluginVTable *
diff --git a/plugins/resize.c b/plugins/resize.c
index 3cfe22b..55ed4be 100644
--- a/plugins/resize.c
+++ b/plugins/resize.c
@@ -869,7 +869,9 @@ CompPluginVTable resizeVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/rotate.c b/plugins/rotate.c
index a383cb6..88bbf6b 100644
--- a/plugins/rotate.c
+++ b/plugins/rotate.c
@@ -2136,7 +2136,9 @@ CompPluginVTable rotateVTable = {
rotateGetScreenOptions,
rotateSetScreenOption,
rotateDeps,
- sizeof (rotateDeps) / sizeof (rotateDeps[0])
+ sizeof (rotateDeps) / sizeof (rotateDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/scale.c b/plugins/scale.c
index 69282bf..a301c75 100644
--- a/plugins/scale.c
+++ b/plugins/scale.c
@@ -1580,7 +1580,9 @@ CompPluginVTable scaleVTable = {
scaleGetScreenOptions,
scaleSetScreenOption,
0, /* Deps */
- 0 /* nDeps */
+ 0 /* nDeps */,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/screenshot.c b/plugins/screenshot.c
index 2fbd974..615af51 100644
--- a/plugins/screenshot.c
+++ b/plugins/screenshot.c
@@ -528,7 +528,9 @@ static CompPluginVTable shotVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/switcher.c b/plugins/switcher.c
index 981fc8b..9dcd381 100644
--- a/plugins/switcher.c
+++ b/plugins/switcher.c
@@ -2107,7 +2107,9 @@ CompPluginVTable switchVTable = {
switchGetScreenOptions,
switchSetScreenOption,
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/water.c b/plugins/water.c
index c64287e..a7ac861 100644
--- a/plugins/water.c
+++ b/plugins/water.c
@@ -1899,7 +1899,9 @@ static CompPluginVTable waterVTable = {
0, /* GetScreenOptions */
0, /* SetScreenOption */
NULL,
- 0
+ 0,
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/wobbly.c b/plugins/wobbly.c
index 6225a02..ddc8321 100644
--- a/plugins/wobbly.c
+++ b/plugins/wobbly.c
@@ -3127,7 +3127,9 @@ CompPluginVTable wobblyVTable = {
wobblyGetScreenOptions,
wobblySetScreenOption,
wobblyDeps,
- sizeof (wobblyDeps) / sizeof (wobblyDeps[0])
+ sizeof (wobblyDeps) / sizeof (wobblyDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/plugins/zoom.c b/plugins/zoom.c
index 9d58d37..004a1ac 100644
--- a/plugins/zoom.c
+++ b/plugins/zoom.c
@@ -884,7 +884,7 @@ zoomGetVersion (CompPlugin *plugin,
}
CompPluginDep zoomDeps[] = {
- { CompPluginRuleAfter, "cube" }
+ { CompPluginRuleRequire, "extendedDesktop" }
};
CompPluginVTable zoomVTable = {
@@ -905,7 +905,9 @@ CompPluginVTable zoomVTable = {
zoomGetScreenOptions,
zoomSetScreenOption,
zoomDeps,
- sizeof (zoomDeps) / sizeof (zoomDeps[0])
+ sizeof (zoomDeps) / sizeof (zoomDeps[0]),
+ NULL, /* features */
+ 0 /* nFeatures */
};
CompPluginVTable *
diff --git a/src/plugin.c b/src/plugin.c
index 425e575..6801234 100644
--- a/src/plugin.c
+++ b/src/plugin.c
@@ -331,6 +331,29 @@ findActivePlugin (char *name)
return 0;
}
+CompPlugin *
+findFeature (char *name)
+{
+ CompPlugin *p;
+ CompPluginFeature *features;
+ int nFeatures;
+
+ for (p = plugins; p; p = p->next)
+ {
+ features = p->vTable->features;
+ nFeatures = p->vTable->nFeatures;
+ while (nFeatures--)
+ {
+ if (strcmp (features->name, name) == 0)
+ return p;
+ features--;
+ }
+ }
+
+ return 0;
+}
+
+
void
unloadPlugin (CompPlugin *p)
{
@@ -387,10 +410,41 @@ checkPluginDeps (CompPlugin *p)
{
CompPluginDep *deps = p->vTable->deps;
int nDeps = p->vTable->nDeps;
+ CompPluginFeature *features;
+ int nFeatures;
+ CompPlugin *plugin;
+
+ deps = p->vTable->deps;
+ nDeps = p->vTable->nDeps;
+
+ features = p->vTable->features;
+ nFeatures = p->vTable->nFeatures;
+
+ while (nFeatures--)
+ {
+ if ((plugin = findFeature (features->name)))
+ {
+ fprintf (stderr, _("%s: '%s' plugin can't be loaded because it provides "
+ "'%s' feature which is already provided by '%s' plugin\n"),
+ programName, p->vTable->name, features->name,
+ plugin->vTable->name);
+
+ return FALSE;
+ }
+ }
while (nDeps--)
{
switch (deps->rule) {
+ case CompPluginRuleRequire:
+ if (findFeature (deps->plugin) == 0)
+ {
+ fprintf (stderr, _("%s: '%s' plugin requires '%s' "
+ "feature\n"), programName, p->vTable->name, deps->plugin);
+
+ return FALSE;
+ }
+ break;
case CompPluginRuleBefore:
if (findActivePlugin (deps->plugin))
{
_______________________________________________
compiz mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/compiz
