Merge lp:~mc-return/compiz/compiz.merge-fix1172913-blacklist-composite-and-opengl into lp:compiz/0.9.10

[MC Return]

PLEASE DO NOT MERGE YET -> NEEDS INFORMATION

*CCSM, new feature:

Bring the ccsm_disable_unity_checkbox.patch inline, because it will not
hurt upstream Compiz in any way.
Also blacklist the plugins composite and opengl in Widgets.py, because
those are essential for Compiz to work.
This commit removes the checkboxes to enable/disable "OpenGL" and "Composite"
from the main CCSM screen. They can still be disabled in their respective
CCSM subscreens.

(LP: #1172913)

PLEASE DO NOT MERGE YET -> NEEDS INFORMATION

TODO: Evaluate with Sam if we should also remove the checkboxes from
      other plugins essential for minimal Compiz usage, like decor, move,
      place and resize for example... ?

Question: Will it be enough to bzr-remove the ccsm_disable_unity_checkbox.patch
          (it was just creating a blacklist for core and was appending unityshell)
          or do I have to do more for this to correctly work ?

[Sam Spilsbury (smspillaz)]

Hmm, not really a fan of this.

There are lots of development situations where one will need to run with unityshell disabled - also running without composite or opengl is a valid usecase (or at least it will be once https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1101026/+merge/159757 gets merged in, since at the moment there's a bug that causes those two to be loaded implicitly anyways). For example - in integration testing scenarios or where you want to run compiz as a normal window manager without compositing support. That was a specific design goal of the 0.9.x series.

review: Disapprove

[MC Return (mc-return)]

> Hmm, not really a fan of this.
>
TBH I am not a fan of removing options from the user as you probably know
either;)

> There are lots of development situations where one will need to run with
> unityshell disabled - also running without composite or opengl is a valid
> usecase (or at least it will be once https://code.launchpad.net/~compiz-
> team/compiz/compiz.fix_1101026/+merge/159757 gets merged in, since at the
> moment there's a bug that causes those two to be loaded implicitly anyways).

I did know that this would be possible...

> For example - in integration testing scenarios or where you want to run compiz
> as a normal window manager without compositing support. That was a specific
> design goal of the 0.9.x series.

Okay, let's drop that for now, in the long run we should elevate Compiz to such
a rock-solid stability level, that by enabling and disabling CCSM plugins manually
the user won't be able to crash Compiz anymore...

Then we can completely purge the ccsm_disable_unity_checkbox.patch.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

On Fri, Apr 26, 2013 at 3:15 PM, MC Return <email address hidden> wrote:
> Review: Needs Fixing
>
>> Hmm, not really a fan of this.
>>
> TBH I am not a fan of removing options from the user as you probably know
> either;)
>
>> There are lots of development situations where one will need to run with
>> unityshell disabled - also running without composite or opengl is a valid
>> usecase (or at least it will be once https://code.launchpad.net/~compiz-
>> team/compiz/compiz.fix_1101026/+merge/159757 gets merged in, since at the
>> moment there's a bug that causes those two to be loaded implicitly anyways).
>
> I did know that this would be possible...

Well, we already run without composite and opengl in some of the
integration tests in compiz anyways. The main problem at the moment
has been that plugins which may have used functionality from others on
an optional basis would cause those plugins to become implicitly
loaded and active by checking if they existed. Which has meant that we
haven't been able to do composite / opengl-less acceptance tests for
plugins which don't necessarily depend on that functionality.

>
>> For example - in integration testing scenarios or where you want to run compiz
>> as a normal window manager without compositing support. That was a specific
>> design goal of the 0.9.x series.
>
> Okay, let's drop that for now, in the long run we should elevate Compiz to such
> a rock-solid stability level, that by enabling and disabling CCSM plugins manually
> the user won't be able to crash Compiz anymore...
>
> Then we can completely purge the ccsm_disable_unity_checkbox.patch.

Well, most compiz plugins can deal with this just fine. In fact, the
last time I checked, pretty much all of them could.

The only problematic one was unityshell, and I believe that was due to
the fact that Unity pulls in Gtk+, Nux and a few others which have
lots of static data which isn't bound to any known names, making that
data impossible to deallocate and clean up properly. This usually
means that you'll get some strange behavior upon reloading those
libraries. That's fair at least, because those libraries were really
meant to be a base framework sitting underneath the application and
managing the application lifecycle as opposed to the other way around.

I believe that its a general goal to ensure that Unity is re-loadable
beneath the other plugins. So if its not working then its probably
worth filing bugs on that. Given its status as a legacy desktop, its
not really known whether or not those will be addressed given other
time constraints or whether or not you'll need to do that yourself.

As for why this distro-patch exists in the first place - I think the
reason was to prevent users in a unity session from disabling the very
thing that which made that session what it was. Which I think is
probably a fair ask. What might make sense is locking down some of the
other stuff within the scope of the distro-patch itself, so for
example, if we had a patch to prevent the disabling of composite,
opengl, move, resize, decor, place, scale, wall, grid, animation etc.

More broadly - one of the goals for compizconfig (which didn't happen
due to time constrain...

[MC Return (mc-return)]

>
> Well, we already run without composite and opengl in some of the
> integration tests in compiz anyways. The main problem at the moment
> has been that plugins which may have used functionality from others on
> an optional basis would cause those plugins to become implicitly
> loaded and active by checking if they existed. Which has meant that we
> haven't been able to do composite / opengl-less acceptance tests for
> plugins which don't necessarily depend on that functionality.
>
The tests do not have much to do with what the user of CCSM experiences.
We should not mix those issues as removing checkboxes won't change our
ability to test everything at all.

> >
> >> For example - in integration testing scenarios or where you want to run
> compiz
> >> as a normal window manager without compositing support. That was a specific
> >> design goal of the 0.9.x series.
> >
Removing a checkbox in CCSM main screen won't change that.

>
> Well, most compiz plugins can deal with this just fine. In fact, the
> last time I checked, pretty much all of them could.
>

It sucks a lot less now, yes ;)
Enabling/disabling works perfectly in 14 out of 15 times currently, I
would estimate...

> The only problematic one was unityshell, and I believe that was due to
> the fact that Unity pulls in Gtk+, Nux and a few others which have
> lots of static data which isn't bound to any known names, making that
> data impossible to deallocate and clean up properly. This usually
> means that you'll get some strange behavior upon reloading those
> libraries. That's fair at least, because those libraries were really
> meant to be a base framework sitting underneath the application and
> managing the application lifecycle as opposed to the other way around.
>
My testing shows that it is not really Unity-specific. You can turn
Unity on and off with roughly the same success rate like other plugins.

What I found out is that there seems to be some timeout when loading
plugins (from what I remember reading the code), we should investigate
if we need it.

Also Compiz seems to get slightly slower each time you fully restart it
(with setsid unity for example), but sometimes this heals itself again
and Compiz restarts fast again (in seldom cases)...

Furthermore holding spinners in CCSM makes it send the rapidly changing
value to Compiz immediately and thus this way the user is able to crash
Compiz easily also...
If you have a idea for some send-timeout or so, it would be cool.

So there is still some work to be done until we can say that CCSM is
stable.

The only plugin, where the user currently has the potential to crash
Compiz completely with, is Wizard. Once you change the Emitters list
Compiz will break immediately and won't start anymore.
I still need to find out what makes it do that and fix it
(it's wizards fault).

> I believe that its a general goal to ensure that Unity is re-loadable
> beneath the other plugins. So if its not working then its probably
> worth filing bugs on that. Given its status as a legacy desktop, its
> not really known whether or not those will be addressed given other
> time constraints or whether or not you'll need to do that yourself.
>
See ...

[MC Return (mc-return)]

I still think it would be a good move to remove the checkboxes from the main screen to help the unknowing user to identify essential plugins that need to stay enabled for Compiz to work.

Tests do not have much to do with what the user of CCSM experiences.
We should not mix those issues, as removing checkboxes from the main CCSM screen won't change our ability to test everything at all.

There is no real reason I can think of that justifies having those checkboxes in the main CCSM screen.
Additionally the checkboxes in the plugins' submenus are still available, so no functionality is removed from the user at all...

[MC Return (mc-return)]

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3672
http://jenkins.qa.ubuntu.com/job/compiz-ci/231/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/272/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-saucy-amd64-ci/44/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-saucy-armhf-ci/44/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-saucy-i386-ci/44/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/231/rebuild

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

The thing is that compiz, since 0.9.x is designed to be run without compositing support. It was broken for quite some time, but it should be working again now.

I don't see any good reason to remove the ability to do that in compizconfig-settings-manager. Remember that compizconfig-settings-manager is an advanced tool where you should be able to change everything and anything.

Unmerged revisions

3672. By MC Return on 2013-04-25

Bring the ccsm_disable_unity_checkbox.patch inline, because it will not
hurt upstream Compiz in any way
Also blacklist the plugins composite and opengl in Widgets.py, because
those are essential for Compiz to work
This removes the checkboxes to enable/disable "OpenGL" and "Composite"
from the main CCSM screen. They can still be disabled in their respective
CCSM subscreens.

TODO: Evaluate with Sam if we should also remove the checkboxes from
      other plugins essential for minimal Compiz usage, like decor, move,
      place and resize for example...

=== modified file 'compizconfig/ccsm/ccm/Widgets.py'
--- compizconfig/ccsm/ccm/Widgets.py	2012-09-04 08:43:45 +0000
+++ compizconfig/ccsm/ccm/Widgets.py	2013-04-25 21:25:30 +0000
@@ -1401,7 +1401,13 @@
         button.set_tooltip_text (plugin.LongDesc)
         button.add (box)
 
-        if plugin.Name != 'core':
+        blacklist_plugins = ['core']
+        blacklist_plugins.append('composite')
+        blacklist_plugins.append('opengl')
+        if os.getenv('DESKTOP_SESSION') == 'ubuntu':
+            blacklist_plugins.append('unityshell')
+
+        if plugin.Name not in blacklist_plugins:
             enable = gtk.CheckButton ()
             enable.set_tooltip_text(_("Enable %s") % plugin.ShortDesc)
             enable.set_active (plugin.Enabled)