Don't show the "Export > win32 vector print" extension on Platforms that don't support it

Bug #1307554 reported by Ryan Lerch on 2014-04-14
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Low
jazzynico

Bug Description

The "Export > win32 vector print" extension shows on Platforms that don't support it.

Is there a way to remove it from the extensions list for all platforms other than Windows?

su_v (suv-lp) on 2014-04-14
tags: added: extensions-plugins packaging
jazzynico (jazzynico) wrote :

> Is there a way to remove it from the extensions list for all platforms other than Windows?

The current extension system just list all files with .inx in the extension directory and load them (if valid, eg all dependencies are met for the extension) indistinctly. Thus we can't filter OS specific extensions.
I see two possibilities:
1. (ugly, but easy workaround) Exclude the extension from the appropriate Makefile.am file. If the INX file is not present in the extension installation folder, it is not loaded. The build system for Windows is a bit different and don't use the Makefile.am file, and thus the extension is not excluded.
2. (preferred) Add a new value for the type attribute of the dependency element (in the INX Schema, see http://wiki.inkscape.org/wiki/index.php/INX_extension_descriptor_format#DTD_XML_schema). The current types are"executable" (tries to find the dependency in a specified location) or "extension" (the id for the extension is looked up in the database). We could have a new "system" type to test the operating system.

I'm going to try solution 2...

Changed in inkscape:
assignee: nobody → jazzynico (jazzynico)
importance: Undecided → Low
milestone: none → 0.91
status: New → Triaged
jazzynico (jazzynico) on 2014-04-21
Changed in inkscape:
status: Triaged → In Progress
jazzynico (jazzynico) wrote :

The attached patch adds a new dependency type:

<dependency type="os">[win|notwin]</dependency>

I'm still not sure if the type name is well chosen (I hesitate between "os" and "system").

Note that it's a very basic implementation that solves the bug, with very limited modifications to the INX schema. We could also easily add other operating systems if needed.

Also note that including or excluding an OS from the element's value is not very clean, but using a different attribute would be a bit more difficult (and add confusion due to the existence of other dependency types...). IMHO for such a feature we should keep the schema changes as limited as possible.

jazzynico (jazzynico) wrote :
jazzynico (jazzynico) wrote :
Alvin Penner (apenner) wrote :

very interesting, looks like an excellent solution.
what is the default return value, in case no "os" parameter is specified? I was looking for a return TRUE somewhere but could not find it.

su_v (suv-lp) wrote :

<off-topic>
Such a change would be roughly the third breakage for backwards compatibility of script-based extensions supported by Inkscape 0.91 (none of the two example INX files are loaded by unpatched trunk, or by current stable - they just add an empty debug menu). Not an issue for extensions distributed with Inkscape, but more so for externally developed and maintained extensions (which in general is encouraged by the project). Are there any concerns or considerations to handle such breakages, or will extension developers have to make sure themselves to offer two versions of the extension in the near future (while 0.48 is still in use e.g. on LTS distros) if affected by incompatible changes in the INX file (dependency check) or in functions provided by inkex.py (translation, unit conversion)?

(I'm aware that such breakage has happened in the past too - AFAIU in 0.46, when migrating from PyXML to lxml)
</off-topic>

jazzynico (jazzynico) wrote :

Alvin > what is the default return value, in case no "os" parameter is specified?

The dependency test should thus be ignored. That's not the case with the attached patch, I'll have to rework it.

Alvin > I was looking for a return TRUE somewhere but could not find it.

TRUE is returned at the end of Dependency::check if none of the tests returns FALSE.

~suv> Are there any concerns or considerations to handle such breakages

I suggest we communicate on it as soon as the code is frozen so that extensions developers are aware of that changes.
That said, the platform dependent extensions patch can easily be backported to 0.48.x, and it may well be the case for the translation changes too (0.48.x Python files are partially translated, and we just need to be sure we don't break the strings that work - of course translating the new strings is out of the question for a minor revision). But I have no idea if it's possible to port the unit conversion changes.

su_v (suv-lp) wrote :

<off-topic cont.>
On 2014-04-22 15:12 +0100, jazzynico wrote:
> ~suv> Are there any concerns or considerations to handle such
> breakages
>
> I suggest we communicate on it as soon as the code is frozen so that
> extensions developers are aware of that changes. That said, the
> platform dependent extensions patch can easily be backported to
> 0.48.x, and it may well be the case for the translation changes too
> (0.48.x Python files are partially translated, and we just need to be
> sure we don't break the strings that work - of course translating the
> new strings is out of the question for a minor revision). But I have
> no idea if it's possible to port the unit conversion changes.

Backporting to 0.48.5 would make it even worse in my understanding (not all distros have the latest bug-fix release, thus potentially 3 versions of extensions would have to be maintained separately (two for the 0.48.x series, and one for 0.91).
</off-topic>

jazzynico (jazzynico) wrote :

<off-topic cont.>
~suv> thus potentially 3 versions of extensions would have to be maintained separately...

If all changes are backported to 0.48.5, that version and 0.91 would share the same extensions, thus we still have two versions, and then only one as soon as everyone has moved to the latest 0.48.x version or 0.91.

After a second thought, the only blocking point is the unit conversion:
* Translation changes can be ignored by external extensions devs (I didn't check, but there are chances that most of those extensions are not translatable at all), and thus their's no need to backport (very low risk of regression).
* OS dependency affects only one of the official extensions, and can be ignored as well, considering users won't use external extensions that don't work on their system . Additionally, I suggest we strongly encourage extensions devs to create multi-platform extensions (and thus discourage them to use the OS dependency feature. Again, no need to backport.

But still, I'm not sure backporting is a good idea at all, and would tend to accept that extensions are not backward compatible. Other applications that provide an extension system (e.g. Firefox, LibreOffice) regularly require changes in the external extensions, and devs constantly adapt their extensions (or abandon them). But contrary to us, the aforementioned applications have user friendly extensions managers that probably helps a lot when it comes to handle different extension versions depending on the application revision.

Does anyone feels that that subject should be debated in the devs list? Backward compatibility issues could affect other parts of the project (are we sure all SVG files created with 0.91 can be fully edited with 0.48.x?).
</off-topic>

jazzynico (jazzynico) wrote :

With the v2 patch, one can include (win) or exclude (notwin) extensions for Windows. All other values are ignored and the test just doesn't fail.

I'm still not sure it's a good idea. And even if we implement it I feel it shouldn't be made public (which is difficult since we can't prevent users from mimicking existing official extensions) or at least strongly discourage its use.

An alternative could be to create a win32 only extensions list in the C++ code directly (I'd see it in Extension::check(), from src/extension/extension.cpp). If the extension ID is com.vaxxine.print.win32, then we don't load it on Windows. Less flexible than the INX schema, but at least we keep control of the special cases, and we don't change the schema. Of course, If new extensions dedicated to a specific OS are added in the future, we could reconsider that option and work again on the INX schema.

As usual, comments are welcome.

jazzynico (jazzynico) wrote :

Attach is a patch implementing the alternative check (with no change in the INX schema).
Tested successfully on Crunchbang Waldorf (Win32 Vector Print extension hidden) and Windows XP (the extension shows in the export menu), Inkscape trunk revision 13312.

jazzynico (jazzynico) wrote :

Patch v3 committed revision 13316.

Changed in inkscape:
status: In Progress → Fix Committed
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers