Port inkscape to librevenge framework for WPG, CDR and VSD imports

[su_v]

Forwarding from #inkscape (irc):
Fridrich Strba contributed diff to migrate Inkscape WPG, CDR and VSD imports to the librevenge framework.

[jazzynico (jazzynico)]

Targeting 0.91 might be a bit optimistic since librevenge is not available in the latest Debian [1] and Ubuntu [2] stable versions.

If there's no objection, I'd rather target 1.0.

[1] Available in the experimental repository only (https://packages.debian.org/search?suite=experimental&arch=any&searchon=names&keywords=librevenge)
[2] Should be available in Utopic (http://packages.ubuntu.com/search?searchon=sourcenames&keywords=librevenge)

[ScislaC (scislac)]

It looks like it will need to be handled conditionally and should be done for 0.91. Unfortunately these formats no longer work in Utopic because of changes in the most recent releases of the supporting libraries.

[ScislaC (scislac)]

I'm on Utopic and have the most recent libs for wpg, cdr, and visio (as well as revenge) and I tried a diff from that branch against trunk 13484 and after running autogen and configure it detects libwpg but not libcdr nor libvisio.

So, unfortunately it doesn't work as-is in my quick testing.

[su_v (suv-lp)]

@ScislaC - could you attach the config.log?

(it works for me on OS X - though I have git master versions of all new librevenge-based import libraries installed)

[ScislaC (scislac)]

Packaging issue on Ubuntu?

configure:20915: $PKG_CONFIG --exists --print-errors "libvisio-0.1 librevenge-0.0 librevenge-stream-0.0"
Package icu-i18n was not found in the pkg-config search path.
Perhaps you should add the directory containing `icu-i18n.pc'
to the PKG_CONFIG_PATH environment variable
Package 'icu-i18n', required by 'libvisio-0.1', not found

I have libicu52 installed, however, librevenge nor the filetype specific packages depend, recommend, or suggest the libicu-dev package. After installing it, the dependencies appear to be satisfied.

[Alex Valavanis (valavanisalex)]

Can I propose, first, that we drop support for old versions of libwpg (< 0.2)? If we don't, the conditional patch will become very messy to maintain... This is fine for windows devlibs and all supported versions of Ubuntu and Fedora, and presumably other popular distros.

Any objections?

[Alex Valavanis (valavanisalex)]

Here's a patch that performs a conditional build. I've dropped support for the now-obsolete libwpg-0.1, but it should work with everything since then.

I've tested it on Ubuntu Trusty (libwpg-0.2 etc), but I don't have a build system with librevenge handy, so I can't promise it'll work! Please feel free to test :)

[ScislaC (scislac)]

Alex: Tested on Utopic. Failed with the following.

../../src/extension/internal/wpg-input.cpp:85:16: error: ‘class librevenge::RVNGInputStream’ has no member named ‘isOLEStream’
     if (input->isOLEStream()) {

[Alex Valavanis (valavanisalex)]

Oops! Take 2...

[ScislaC (scislac)]

That one compiled fine. Now to hear if it is okay on other platforms.

[Alex Valavanis (valavanisalex)]

OK great... any thoughts about what to do with 0.48.x? Is there any appetite for a micro-release (0.48.5.1?!) to fix it or shall we just leave it up to downstream maintainers to patch it?

[su_v (suv-lp)]

There are two other reports which might make a micro-release more interesting (one of them a build failure):
bug #1357411 (has patch), bug #1345930 (needs triage)

[su_v (suv-lp)]

> Now to hear if it is okay on other platforms.

On OS X 10.7.5, trunk r13529:

1) environment with these ports installed:
  libcdr 0.0.14
  libvisio 0.0.27
  libwpg 0.2.2

Build succeeds:
(…)
checking for LIBWPG03... no
checking for LIBWPG02... yes
checking for LIBVISIO01... no
checking for LIBVISIO00... yes
checking for LIBCDR01... no
checking for LIBCDR00... yes
(…)
        Libwpg: yes
        Libvisio: yes
        Libcdr: yes
(…)

$ grep CDR build/config.h
#define WITH_LIBCDR00 1
/* #undef WITH_LIBCDR01 */
$ grep VISIO build/config.h
#define WITH_LIBVISIO00 1
/* #undef WITH_LIBVISIO01 */
$

but the input extensions based on Libvisio and Libcdr are not available at runtime (not listed among the file formats in the 'File > Open' dialog). WPG is ok.

2) environment with these ports installed (the versions can co-exist if the tools are installed with versioned suffix):

  libcdr 0.0.16
  libvisio 0.0.31
  libwpg 0.2.2

  librevenge 0.0.1 (git master)
  libcdr 0.1.0 (git master)
  libvisio 0.1.0 (git master)
  libwpg 0.3.0 (git master)

Build fails for WPG input (ok for Visio and CDR):
(…)
checking for LIBWPG03... yes
checking for LIBWPG02... yes
checking for LIBVISIO01... yes
checking for LIBVISIO00... yes
checking for LIBCDR01... yes
checking for LIBCDR00... yes
(…)
        Libwpg: yes
        Libvisio: yes
        Libcdr: yes
(…)
$ (cd build; make -k)
make all-recursive
Making all in src
make all-am
  CXX extension/internal/wpg-input.o
../../src/extension/internal/wpg-input.cpp:62:12: fatal error: 'librevenge-stream/librevenge-stream.h' file not found
  #include <librevenge-stream/librevenge-stream.h>
           ^
1 error generated.
make[3]: *** [extension/internal/wpg-input.o] Error 1
make[3]: Target `all-am' not remade because of errors.
make[2]: *** [all] Error 2

$ grep WPG build/config.h
#define WITH_LIBWPG 1
#define WITH_LIBWPG02 1
#define WITH_LIBWPG03 1

[Alex Valavanis (valavanisalex)]

Re comment #5: Missing libvisio dependency reported here: Bug #1359824

[Alex Valavanis (valavanisalex)]

@suv - I'm quite puzzled by this! If the cdr/visio input worked OK, then you must have the <revenge-stream/revenge-stream.h> header available. Maybe the LIBWPG_CFLAGS aren't being set correctly. Would you mind checking your config.log to see what you have? There should be something like:

LIBWPG02_CFLAGS=''
LIBWPG02_LIBS=''
LIBWPG03_CFLAGS='-I/usr/include/libwpg-0.3 -I/usr/include/libwpd-0.10 -I/usr/include/librevenge-0.0 '
LIBWPG03_LIBS='-lwpg-0.3 -lrevenge-0.0 -lrevenge-stream-0.0 '
LIBWPG_CFLAGS='-I/usr/include/libwpg-0.3 -I/usr/include/libwpd-0.10 -I/usr/include/librevenge-0.0 '
LIBWPG_LIBS='-lwpg-0.3 -lrevenge-0.0 -lrevenge-stream-0.0 '

If all has gone to plan:

(a) LIBWPG_CFLAGS should be identical to LIBWPG03_CFLAGS
(b) LIBWPG_CFLAGS should point to the folder containing the revenge-stream header

[su_v (suv-lp)]

1) This logic:
> dnl **************************************************************
> dnl Try using librevenge framework first. Fall back to old libs
> dnl if unavailable.
> dnl TODO: Drop subsequent tests once this is widespread in distros
> dnl **************************************************************
is not what happens: the fallback is not really conditional. AFAIU if both versions are installed, the test for the older libs overwrites the one for the newer. Either swap tests, or chain them with an 'else … if' construct?

2) For libcdr and libvisio, the config variable to enable the feature at all ('WITH_LIBCDR', 'WITH_LIBVISIO') is not set - neither for old nor for new versions of the libs. This basically disabled cdr and vsd input, independent of whether the requirements are satisfied or not.

3) There's a typo in the check for libcdr01:
- + PKG_CHECK_MODULES(LIBCDR01, libcdr-0.1 librevenge-0.0 librevenge-stream-0.0, with_cdr01=yes, with_libcdr01=no)
+ + PKG_CHECK_MODULES(LIBCDR01, libcdr-0.1 librevenge-0.0 librevenge-stream-0.0, with_libcdr01=yes, with_libcdr01=no)

Attached version of the diff was tested successfully with two setups (old libs installed, old and new libs installed). It was not tested with a setup where neither version of the libs is installed.

[Alex Valavanis (valavanisalex)]

Looks like a good fix; sorry for the hastily prepared patch! Tested on Ubuntu Utopic 64-bit with new versions of libs; all seems fine :)

[su_v (suv-lp)]

On 2014-08-21 24:32 , Alex Valavanis wrote:
> OK great... any thoughts about what to do with 0.48.x? Is there any
> appetite for a micro-release (0.48.5.1?!) to fix it or shall we just
> leave it up to downstream maintainers to patch it?

@Alex - fixes for bug #1357411, bug #1345930 have been committed to <lp:inkscape/0.48.x> in r10041 and r10042. Attaching backport of librevenge patch for LIBWPG to stable 0.48.x - please review (I was unsure whether dropping support for LIBWPG01 would be ok for stable too) and test on Linux …

Patch tested successfully with LIBWPG01, LIBWPG02 and LIBWG03 on OS X 10.7.5.

[su_v (suv-lp)]

Both patches need testing on Windows, too.

(see also http://sourceforge.net/p/inkscape/mailman/message/32793709/ )

[su_v (suv-lp)]

r13621

[su_v (suv-lp)]

@Fridrich - many thanks for the original librevenge patch: it has landed (with a few modifications) in Inkscape trunk just in time for the upcoming major release 0.91:
 http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/13621

[Victor (gaploid)]

Hi Guys,
Just found that windows archive 0.91 version still have libcdr 0.0.14. It looks like that there is a 0.1.1 already http://cgit.freedesktop.org/libreoffice/libcdr/ is it possible to update it?

Thank you in advance,
Victor

[ScislaC (scislac)]

Victor,

Please file a new report with your request so it can be tracked properly.

[su_v (suv-lp)]

@ScislaC, @Victor - we could unlink Victor's earlier report bug #1327170 (currently a duplicate), and reassign it to the windows devlibs repo.

[ScislaC (scislac)]

Ahhh, good thinking (I hadn't seen it). I went ahead and did that.