Merge lp:~compiz-team/compiz-libcompizconfig/compiz-libcompizconfig.fix_874830 into lp:compiz-libcompizconfig

[Sam Spilsbury]

Bug 874830

[Tim Penhey (thumper)]

This is a clear place where we could have tests. There are stand alone methods that have defined behaviour.

Also another reason why I had boolean parameters:

- ccsSetString (setting, value, TRUE);
+ ccsSetString (setting, value, FALSE);

This tells the reviewer exactly nothing.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Should I include some sample files in a tests/ directory to demonstrate how the settings upgrade / key renames / default overrides ? They won't be installable at runtime (since we don't want users to accidentally create a profile which will have its key values changed by some testing code), but at least they could be there for example's sake.

> Also another reason why I had boolean parameters:

> - ccsSetString (setting, value, TRUE);
> + ccsSetString (setting, value, FALSE);

> This tells the reviewer exactly nothing.

Yes, that need to be changed to:

enum
{
    WriteSetting,
    SetTemporary
};

Though in reality, CCSSetting needs to be an abstracted class or something since this code really just abuses the structure into storing some temporary data.

Unmerged revisions

425. By Sam Spilsbury on 2011-10-15

Provide a generic method for system administrators and distributions
to handle renamed keys so that user settings will be transitioned to
the new key names and settings will not be lost.

Provide a file in DATADIR/compizconfig/transitions called your_item.transition
with the following format

oldPluginName oldSettingName newPluginName newSettingName
oldPluginName2 oldSettingName2 newPluginName2 newSettingName2

Keys will then be renamed appropriately.

Note that this requires backend support. If your backend does not support
reading from keys that don't have an installed schema, then those settings
will be erased as soon as the user upgrades since the schema data will be
lost. That includes GSettings at the moment.

=== modified file 'backend/src/ini.c'
--- backend/src/ini.c	2011-08-20 19:03:37 +0000
+++ backend/src/ini.c	2011-10-15 07:05:26 +0000
@@ -275,7 +275,7 @@
 	    if (ccsIniGetString (data->iniFile, setting->parent->name,
 				 keyName, &value))
 	    {
-		ccsSetString (setting, value, TRUE);
+		ccsSetString (setting, value, FALSE);
 		free (value);
 		status = TRUE;
 	    }
@@ -287,7 +287,7 @@
 	    if (ccsIniGetString (data->iniFile, setting->parent->name,
 				 keyName, &value))
 	    {
-		ccsSetMatch (setting, value, TRUE);
+		ccsSetMatch (setting, value, FALSE);
 		free (value);
 		status = TRUE;
 	    }
@@ -299,7 +299,7 @@
 	    if (ccsIniGetInt (data->iniFile, setting->parent->name,
 			      keyName, &value))
 	    {
-		ccsSetInt (setting, value, TRUE);
+		ccsSetInt (setting, value, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -310,7 +310,7 @@
 	    if (ccsIniGetBool (data->iniFile, setting->parent->name,
 			       keyName, &value))
 	    {
-		ccsSetBool (setting, (value != 0), TRUE);
+		ccsSetBool (setting, (value != 0), FALSE);
 		status = TRUE;
 	    }
 	}
@@ -321,7 +321,7 @@
 	    if (ccsIniGetFloat (data->iniFile, setting->parent->name,
 				keyName, &value))
 	    {
-		ccsSetFloat (setting, value, TRUE);
+		ccsSetFloat (setting, value, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -333,7 +333,7 @@
 	    if (ccsIniGetColor (data->iniFile, setting->parent->name,
 				keyName, &color))
 	    {
-		ccsSetColor (setting, color, TRUE);
+		ccsSetColor (setting, color, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -344,7 +344,7 @@
 	    if (ccsIniGetKey (data->iniFile, setting->parent->name,
 			      keyName, &key))
 	    {
-		ccsSetKey (setting, key, TRUE);
+		ccsSetKey (setting, key, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -355,7 +355,7 @@
 	    if (ccsIniGetButton (data->iniFile, setting->parent->name,
 				 keyName, &button))
 	    {
-		ccsSetButton (setting, button, TRUE);
+		ccsSetButton (setting, button, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -366,7 +366,7 @@
 	    if (ccsIniGetEdge (data->iniFile, setting->parent->name,
 				 keyName, &edges))
 	    {
-		ccsSetEdge (setting, edges, TRUE);
+		ccsSetEdge (setting, edges, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -377,7 +377,7 @@
 	    if (ccsIniGetBell (data->iniFile, setting->parent->name,
 			       keyName, &bell))
 	    {
-		ccsSetBell (setting, bell, TRUE);
+		ccsSetBell (setting, bell, FALSE);
 		status = TRUE;
 	    }
 	}
@@ -388,7 +388,7 @@
 	    if (ccsIniGetList (data->iniFile, setting->parent->name,
 			       keyName, &value, setting))
 	    {
-		ccsSetList (setting, value, TRUE);
+		ccsSetList (setting, value, FALSE);
 		ccsSettingValueListFree (value, TRUE);
 		status = TRUE;
 	    }
@@ -401,7 +401,7 @@
     if (!status)
     {
 	/* reset setting to default if it could not be read */
-	ccsResetToDefault (setting, TRUE);
+	ccsResetToDefault (setting, FALSE);
     }
 
     if (keyName)

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2010-05-21 02:55:08 +0000
+++ src/CMakeLists.txt	2011-10-15 07:05:26 +0000
@@ -24,6 +24,7 @@
     -DIMAGEDIR=\\\"${COMPIZ_IMAGEDIR}\\\"
     -DMETADATADIR=\\\"${COMPIZ_METADATADIR}\\\"
     -DSYSCONFDIR=\\\"${COMPIZ_SYSCONFDIR}\\\"
+    -DDATADIR=\\\"${COMPIZ_DATADIR}\\\"
     -DLIBDIR=\\\"${COMPIZ_LIBDIR}\\\"
 )
 

=== modified file 'src/ccs-private.h'
--- src/ccs-private.h	2011-08-20 19:03:37 +0000
+++ src/ccs-private.h	2011-10-15 07:05:26 +0000
@@ -72,7 +72,14 @@
     CCSSettingList replaceToValueSettings;
 } CCSSettingsUpgrade;
 
+typedef struct _CCSSettingsTransition
+{
+    char           *file;
+    char           *name;
+} CCSSettingsTransition;
+
 Bool ccsCheckForSettingsUpgrade (CCSContext *context);
+Bool ccsCheckForSettingsTransition (CCSContext *context);
 
 void ccsLoadPlugins (CCSContext * context);
 void ccsLoadPluginSettings (CCSPlugin * plugin);

=== modified file 'src/main.c'
--- src/main.c	2011-09-15 13:12:45 +0000
+++ src/main.c	2011-10-15 07:05:26 +0000
@@ -169,6 +169,9 @@
 
     ccsLoadPlugins (context);
 
+    /* Do settings transitions */
+    ccsCheckForSettingsTransition (context);
+
     /* Do settings upgrades */
     ccsCheckForSettingsUpgrade (context);
 
@@ -3356,7 +3359,21 @@
     free (uname);
 
     return 1;
-}    
+}
+
+static int
+transitionNameFilter (const struct dirent *name)
+{
+    int length = strlen (name->d_name);
+
+    if (length < 12)
+	return 0;
+
+    if (strncmp (name->d_name + length - 11, ".transition", 11))
+	return 0;
+
+    return 1;
+}
 
 /*
  * Process a filename into the properties
@@ -3422,6 +3439,22 @@
     return upgrade;
 }
 
+CCSSettingsTransition *
+ccsSettingsTransitionNew (char *path, char *name)
+{
+    CCSSettingsTransition *transition = calloc (1, sizeof (CCSSettingsTransition));
+    unsigned int          namelen = strlen (path);
+    unsigned int          pathlen = strlen (name);
+    unsigned int          fnlen = (pathlen + namelen + 1);
+
+    transition->file = calloc (fnlen, sizeof (char));
+    snprintf (transition->file, fnlen, "%s%s", path, name);
+
+    transition->name = strndup (name, pathlen - 10);
+
+    return transition;
+}
+
 Bool
 ccsCheckForSettingsUpgrade (CCSContext *context)
 {
@@ -3503,6 +3536,380 @@
 }
 
 Bool
+ccsProcessTransition (CCSContext            *context,
+		      CCSSettingsTransition *transition)
+{
+    FILE *fp =		   fopen (transition->file, "r");
+    char		   *ctBuffer;
+    unsigned int	   ctSize;
+    size_t		   ctReadSize;
+    unsigned int	   i, j;
+    char		   *tok;
+    char		   *buf;
+
+    if (!fp)
+    {
+	D (D_FULL, "[WARNING] failed to open transition file %s\n", transition->name);
+	return FALSE;
+    }
+
+    fseek (fp, 0, SEEK_END);
+    ctSize = ftell (fp);
+    rewind (fp);
+    ctBuffer = calloc (ctSize + 1, sizeof (char));
+
+    if (!ctBuffer)
+    {
+	D (D_FULL, "[WARNING] couldn't allocate transition file %s buffer\n", transition->name);
+	fclose (fp);
+	return FALSE;
+    }
+
+    ctReadSize = fread (ctBuffer, sizeof (char), ctSize, fp);
+
+    if (ctReadSize != ctSize)
+    {
+	D (D_FULL, "[WARNING] couldn't read completed buffer for transition %s\n", transition->name);
+	free (ctBuffer);
+	fclose (fp);
+	return FALSE;
+    }
+    ctBuffer[ctSize] ='\0';
+
+    /* Remove newlines */
+    i = 0;
+
+    while (i < ctSize)
+    {
+	if (strncmp (&(ctBuffer[i]), "\n", 1) == 0)
+	{
+	    (&(ctBuffer[i]))[0] = ' ';
+	}
+
+	i++;
+    }
+
+    /* Now break up the transition file into 4-sized chunks, with
+     * old-plugin old-setting new-plugin new-setting (newline)
+     * but first check to make sure that the number of tokens
+     * we have is a multiple of 4 */
+
+    buf = strdup (ctBuffer);
+    i = 0;
+
+    tok = strsep (&buf, " ");
+
+    while (tok)
+    {
+	i++;
+	tok = strsep (&buf, " ");
+    }
+
+    free (buf);
+
+    if ((i  - 1) % 4 != 0)
+    {
+	D (D_FULL, "[WARNING] provided upgrade file %s format not correct!\n", transition->name);
+	free (ctBuffer);
+	fclose (fp);
+	return FALSE;
+    }
+
+    buf = strdup (ctBuffer);
+
+    for (j = 0; j < (i / 4); j++)
+    {
+	char *oldPlugin;
+	char *newPlugin;
+	char *oldSetting;
+	char *newSetting;
+	CCSPlugin *cOldPlugin, *cNewPlugin;
+	CCSSetting *cOldSetting, *cNewSetting;
+	CCSPluginList p = NULL;
+	CCSSettingList s = NULL;
+	Bool           freeOldPlugin = FALSE, freeOldSetting = FALSE;
+
+	CONTEXT_PRIV (context);
+
+	oldPlugin = strsep (&buf, " ");
+	oldSetting = strsep (&buf, " ");
+	newPlugin = strsep (&buf, " ");
+	newSetting = strsep (&buf, " ");
+
+	if (!cPrivate->backend)
+	    return FALSE;
+
+	if (!cPrivate->backend->vTable->readSetting)
+	    return FALSE;
+
+	if (cPrivate->backend->vTable->readInit)
+	    if (!(*cPrivate->backend->vTable->readInit) (context))
+		return FALSE;
+
+	/* Find the new plugin and new setting, if neither can
+	 * be found, error out */
+
+	p = context->plugins;
+
+	while (p)
+	{
+	    if (strcmp (((CCSPlugin *) p->data)->name, newPlugin) == 0)
+		break;
+
+	    p = p->next;
+	}
+
+	if (!p)
+	{
+	    D (D_FULL, "[WARNING]: Could not find new plugin %s for transition %s\n", newPlugin, transition->name);
+	    continue;
+	}
+	else
+	{
+	    cNewPlugin = (CCSPlugin *) p->data;
+
+	    if (!((CCSPluginPrivate *) cNewPlugin->ccsPrivate)->loaded)
+		ccsLoadPluginSettings (cNewPlugin);
+
+	    s = ((CCSPluginPrivate *) cNewPlugin->ccsPrivate)->settings;
+
+	    while (s)
+	    {
+		if (strcmp (((CCSSetting *) s->data)->name, newSetting) == 0)
+		    break;
+
+		s = s->next;
+	    }
+
+	    if (!s)
+	    {
+		D (D_FULL, "[WARNING]: Could not find new setting %s for transition %s\n", newSetting, transition->name);
+		continue;
+	    }
+	    else
+		cNewSetting = (CCSSetting *) s->data;
+	}
+
+	/* Try and find the old plugin, if not, craft one
+	 * for the purposes of this function */
+
+	p = context->plugins;
+
+	while (p)
+	{
+	    if (strcmp (((CCSPlugin *) p->data)->name, oldPlugin) == 0)
+		break;
+
+	    p = p->next;
+	}
+
+	if (!p)
+	{
+	    cOldPlugin = calloc (1, sizeof (CCSPlugin));
+
+	    cOldPlugin->context = context;
+	    cOldPlugin->ccsPrivate = calloc (1, sizeof (CCSPluginPrivate));
+	    if (!cOldPlugin->ccsPrivate )
+	    {
+		free (cOldPlugin);
+		return FALSE;
+	    }
+	    ((CCSPluginPrivate *) cOldPlugin->ccsPrivate)->loaded = FALSE;
+
+	    cOldPlugin->name = strdup (oldPlugin);
+	    cOldPlugin->shortDesc = strdup (oldPlugin);
+	    cOldPlugin->longDesc  = strdup (oldPlugin);
+	    cOldPlugin->category  = strdup ("");
+	    cOldPlugin->refCount  = 1;
+
+	    freeOldPlugin = TRUE;
+	}
+	else
+	    cOldPlugin = (CCSPlugin *) p->data;
+
+	s = NULL;
+
+	if (!freeOldPlugin)
+	{
+	    /* Now try and find the old setting to transition from */
+
+	    s = ((CCSPluginPrivate *) cOldPlugin->ccsPrivate)->settings;
+
+	    while (s)
+	    {
+		if (strcmp (((CCSSetting *) s->data)->name, oldSetting) == 0)
+		    break;
+
+		s = s->next;
+	    }
+	}
+
+	if (!s)
+	{
+	     cOldSetting = calloc (1, sizeof (CCSSetting));
+
+	     cOldSetting = (CCSSetting *) calloc (1, sizeof (CCSSetting));
+	     if (!cOldSetting)
+		 return FALSE;
+
+	     cOldSetting->parent = cOldPlugin;
+	     cOldSetting->isDefault = TRUE;
+	     cOldSetting->name = strdup (oldSetting);
+	     cOldSetting->refCount = 1;
+
+	     cOldSetting->shortDesc = strdup (oldSetting);
+	     cOldSetting->longDesc  = strdup ("");
+	     cOldSetting->hints     = strdup ("");
+	     cOldSetting->group     = strdup ("");
+	     cOldSetting->subGroup  = strdup ("");
+
+	     cOldSetting->value = &cOldSetting->defaultValue;
+	     cOldSetting->defaultValue.parent = cOldSetting;
+	     cOldSetting->type = cNewSetting->type;
+
+	     switch (cOldSetting->type)
+	     {
+	     case TypeString:
+		cOldSetting->defaultValue.value.asString = strdup ("");
+		break;
+	     case TypeMatch:
+		cOldSetting->defaultValue.value.asMatch = strdup ("");
+		break;
+	     default:
+		break;
+	     }
+
+	     freeOldSetting = TRUE;
+	}
+	else
+	    cOldSetting = (CCSSetting *) s->data;
+
+	/* Now ask the backend for the value of the old setting */
+	(*cPrivate->backend->vTable->readSetting) (context, cOldSetting);
+	ccsSetValue (cNewSetting, cOldSetting->value, TRUE);
+	D (D_FULL, "Moved setting %s/%s to %s/%s", oldPlugin, oldSetting, newPlugin, newSetting);
+
+	if (freeOldSetting)
+	{
+	    free (cOldSetting->name);
+	    free (cOldSetting->shortDesc);
+	    free (cOldSetting->longDesc);
+	    free (cOldSetting->hints);
+	    free (cOldSetting->subGroup);
+
+	    switch (cOldSetting->type)
+	    {
+	     case TypeString:
+		free (cOldSetting->defaultValue.value.asString);
+		break;
+	     case TypeMatch:
+		free (cOldSetting->defaultValue.value.asMatch);
+		break;
+	     default:
+		break;
+	     }
+
+	    free (cOldSetting);
+	}
+
+	if (freeOldPlugin)
+	{
+	    free (cOldPlugin->ccsPrivate);
+	    free (cOldPlugin->name);
+	    free (cOldPlugin->shortDesc);
+	    free (cOldPlugin->longDesc);
+	    free (cOldPlugin->category);
+	    free (cOldPlugin);
+	}
+    }
+
+    D (D_FULL, "Completed transition %s\n", transition->name);
+    free (ctBuffer);
+    fclose (fp);
+
+    return TRUE;
+}
+
+Bool
+ccsCheckForSettingsTransition (CCSContext *context)
+{
+    struct dirent 	   **nameList;
+    int 	  	   nFile, i;
+    char	  	   *path = DATADIR "/compizconfig/transitions/";
+    char		   *dupath = NULL;
+    FILE		   *completedTransitions;
+    char		   *ctBuffer;
+    unsigned int	   ctSize;
+    size_t		   ctReadSize;
+    char		   *home = getenv ("HOME");
+
+    if (!home)
+	return FALSE;
+
+    asprintf (&dupath, "%s/.config/compiz-1/compizconfig/done_transitions", home);
+
+    completedTransitions = fopen (dupath, "a+");
+
+    if (!path)
+	return FALSE;
+
+    nFile = scandir (path, &nameList, transitionNameFilter, alphasort);
+    if (nFile <= 0)
+	return FALSE;
+
+    if (!completedTransitions)
+    {
+	fprintf (stderr, "[WARNING] Error opening done_transitions\n");
+	return FALSE;
+    }
+
+    fseek (completedTransitions, 0, SEEK_END);
+    ctSize = ftell (completedTransitions);
+    rewind (completedTransitions);
+
+    ctBuffer = calloc (ctSize + 1, sizeof (char));
+    ctReadSize = fread (ctBuffer, 1, ctSize, completedTransitions);
+
+    if (ctReadSize != ctSize)
+	D (D_FULL, "[WARNING] Couldn't read completed transitions file!\n");
+
+    ctBuffer[ctSize] = '\0';
+
+    for (i = 0; i < nFile; i++)
+    {
+	char		   *matched = strstr (ctBuffer, nameList[i]->d_name);
+	CCSSettingsTransition *transition;
+
+	if (matched)
+	{
+	    D (D_FULL, "Skipping transition %s\n", nameList[i]->d_name);
+	    continue;
+	}
+
+	transition = ccsSettingsTransitionNew (path, nameList[i]->d_name);
+
+	D (D_FULL, "Processing transition %s\n", nameList[i]->d_name);
+
+	ccsProcessTransition (context, transition);
+	ccsWriteChangedSettings (context);
+	ccsWriteAutoSortedPluginList (context);
+	D (D_FULL, "Completed transition %s\n", nameList[i]->d_name);
+	fprintf (completedTransitions, "%s\n", nameList[i]->d_name);
+	free (transition->file);
+	free (transition->name);
+	free (transition);
+	free (nameList[i]);
+    }
+
+    free (dupath);
+    fclose (completedTransitions);
+    free (ctBuffer);
+    free (nameList);
+
+    return TRUE;
+}
+
+Bool
 ccsImportFromFile (CCSContext *context,
 		   const char *fileName,
 		   Bool       overwriteNonDefault)