GNOME Bugzilla – Bug 314400
rsvg_handle_set_dpi_x_y( ) is required to build gimp-2.3.3
Last modified: 2005-12-19 12:38:13 UTC
Version details: CVS as of 10Aug05 Distribution/Version: YDL-4.0.1 The latest version of gimp, 2.3.3, will not link against the 10Aug05 CVS version of librsvg. I see from the CVS source code that the librsvg team is in the process of deciding what to do with an entire set of "RSVG_DISABLE_DEPRECATED" functions. In version 2.9.5, that set contained only two functions, #ifndef RSVG_DISABLE_DEPRECATED void rsvg_set_default_dpi (double dpi); void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); #endif rsvg_handle_set_dpi_x_y() was not[yet] a member of that set. As the librsvg API continues to evolve, it would be prudent to publish anticipated API changes well in advance of release so other development groups can incorporate them into their products.
librsvg has never had a promise of API/ABI stability. In light of that, I've marked those few functions as deprecated for the better part of 2 years. I've informed Sven and others that they're deprecated. So now in a *unreleased development version* of librsvg, I've removed the deprecated functions. As librsvg is in the midst of a major overhaul in terms of its API and what it's implemented in terms of (Cairo vs Libart), I don't think that's a problem. The HEAD version wasn't meant to be used against the GNOME 2.x series anyway. Use the GNOME-2-12 branch if you want the promised API/ABI stability, which I've left in. As far as I'm concerned, bug 314400 is a bug in the Gimp.
I'm going to retreat somewhat from my previous comment. I believe that this is a bug in Garnome's choice to package a CVS HEAD version of librsvg, which is undergoing massive internal and external changes. The CVS HEAD version of librsvg was not meant to be used with the GNOME 2.x series, and Paul made a mistake by assuming that it would be. As such, I'm going to close this bug as WONTFIX/INVALID. When mixing and matching unreleased, development versions of libraries, caveat emptor.
Hmm, I wouldn't mind if you reassigned the bug to gimp since otherwise someone else will run into the problem and then has to open a new report for it. It shouldn't be hard to workaround this issue in the gimp plug-in and if this happens soon, the change could even go into gimp 2.2.9 still. If on the other hand you are saying that noone should be using the CVS version of librsvg, then I don't see any need for action.
Created attachment 51331 [details] [review] handle API change in GIMP plug-in Would this patch work? Btw, it would have saved me a lot of time if the librsvg ChangeLog would mention releases and changes to the version numbers. Or if releases would at least be tagged...
Created attachment 51338 [details] [review] patch gimp-2.3.3 to build using librsvg-2.9.9 I want to thank everyone involved for bringing this issue to closure. librsvg-2.9.9, the 10Aug05 CVS version, may now be used to build GARNOME-2.11.92. -Joseph
Reopening for GIMP.
Joseph, in the patch you attached, why did you change the way the version number is being handled? The current CVS version of librsvg has LIBRSVG_MAJOR_VERSION=2 LIBRSVG_MINOR_VERSION=99 LIBRSVG_MICRO_VERSION=0 Your patch isn't going to work with that version. What was the reason to change the version check in the first place?
I looked at the source code for the version I have, 10Aug05. The minor version is not 99: #ifndef LIBRSVG_FEATURES_H #define LIBRSVG_FEATURES_H 1 #define LIBRSVG_MAJOR_VERSION (2) #define LIBRSVG_MINOR_VERSION (9) #define LIBRSVG_MICRO_VERSION (9) #define LIBRSVG_VERSION "" I took a look at the latest version, which is 2.11.0. #ifndef LIBRSVG_FEATURES_H #define LIBRSVG_FEATURES_H 1 #define LIBRSVG_MAJOR_VERSION (2) #define LIBRSVG_MINOR_VERSION (11) #define LIBRSVG_MICRO_VERSION (0) #define LIBRSVG_VERSION "" So the logic for the #if's needs to be tweaked. -Joseph
The question is, when was the API changed? What version should we check for if we want to use the newer form of rsvg_handle_set_dpi()? When was the deprecated variant removed?
Sven, I don't have a good understanding of the numbering system used by the librsvg team, so I would focus only on stable releases. Looking at the source code... The API changed after the 2.9.5 stable release. In the 2.9.5 release rsvg_handle_set_dpi_x_y() was present and was not a member of the set of "DEPRECATED" functions. In the 2.11.0 [stable???] release, rsvg_handle_set_dpi_x_y() has disappeared along with the other functions that were tagged as "DEPRECATED". In fact the word "DEPCRECATED" has disappeared completely from the 2.11.0 source. So... I would suggest changing the test to #if (LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION >= 11) rsvg_handle_set_dpi (handle, vals->resolution, vals->resolution); #else rsvg_handle_set_dpi_x_y (handle, vals->resolution, vals->resolution); #endif Thoughts??? -Joseph
I am actually hoping for a quick note from Dom, telling us what version number to check for...
Sven, the second patch looks good to me. It will nicely handle the case of CVS HEAD librsvg vs. the versions we mean to ship in the GNOME 2.12 release series and below.
Looks like "we" are collectively driving this issue to closure. This is a good thing. After all, the operative word in the open-source community is "we", not "I" [:-)] -Joseph
Dom, sorry, perhaps I am just overlooking something here. But the second patch checks for (LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION == 9 && LIBRSVG_MICRO_VERSION < 9). That isn't even going to work with the version that I have installed here (2.8.1). Do you perhaps meant to say that the change proposed in comment #10 looks good?
Sorry, Sven. I didn't read the patch well enough. I just saw that it seemed to handle both the old and new APIs. The approach in comment #10 looks good.
OK, applied to both branches. This whole thing will cause packagers some headaches but I don't see what we can do about that... 2005-08-26 Sven Neumann <sven@gimp.org> * plug-ins/common/svg.c: deal with SVG API change. This is a compile-time check and thus somewhat ugly because it requires a recompile of the plug-in when updating librsvg. Fixes bug #314400.
*** Bug 316719 has been marked as a duplicate of this bug. ***
Obviously (see bug #316719) the change was not correct, even though I explicitely asked the librsvg maintainer to check that it is :(
Reopening, arghh!
After comparing various states in CVS, I have settled on the following version and committed to both branches: #if (LIBRSVG_MAJOR_VERSION == 2 && LIBRSVG_MINOR_VERSION < 99) rsvg_handle_set_dpi_x_y (handle, vals->resolution, vals->resolution); #else rsvg_handle_set_dpi (handle, vals->resolution, vals->resolution); #endif It would have been a lot easier if librsvg CVS had tags for all releases and if Joseph and Dom would have paid a little more attention. I hope that the issue is solved now...
Huh??? -Joseph
Don't pass the buck, Sven. Review patches for your own product, and accept responsibility for them when things fail. I said that something like what was proposed would work ok ("The approach in comment #10 looks good."), and I said that librsvg 2.12 would NOT break API/ABI.
*** Bug 316799 has been marked as a duplicate of this bug. ***
gimp 2.2.9 failed with librsvg 2.7.1 and 2.13.2, both exited with "undefined reference to `rsvg_handle_set_dpi_x_y'" in svg.c:441 and svg.c:542. If I do a nm -s on the 2.13.2 archive of librsvg I find the rsvg_handle_set_dpi function but not the ..._x_y one. LIBRSVG_MINOR_VERSION is defined as 7 and 13 respectively. I've had noe problems with earlier gimp versions (ie <= 2.2.8). So, neither #10 nor #20 seems to work me, but #14 would. (But then again, I never tried the 2.9.x version of librsvg.)
*** Bug 320148 has been marked as a duplicate of this bug. ***
If I understood bug 320007 correctly, librsvg 2.13 is no stable release (taken from CVS HEAD?) and still changing. So it wouldn't be surprising that building against it fails.
There is one problem with the above. The suggested code: rsvg_handle_set_dpi (handle, vals->resolution, vals->resolution) which appears in gimp 2.2.9 (svg.c) is wrong. The rsvg_handle_set_dpi function only takes one dpi value, of course: rsvg_handle_set_dpi (handle, vals->resolution) Gimp 2.2.9 has a problem with the test and even if the test worked, the second branch (the set_dpi with two parameters) would not work. The wrong number of parameters in the set_dpi function is another item to watch out for.
Listen, I changed librsvg's API back to 1-arg set_dpi() and set_default_dpi() a while ago in order to placate everyone here and renamed the 2-arg versions to include an "x_y" in their name, just like before. There was 1 unstable release that had the 2-arg version of set_dpi(), and I wouldn't support that if I were you. Please act accordingly.
That is the information that I asked for two months ago. Oh well, is anyone willing to clean up this mess? I feel bored by this issue but will look into applying a patch if one is provided here.
I would like to get this sorted out for the 2.2.10 and 2.3.6 releases. Increasing priority accordingly.
Hm, when comparing comment 28 to comment 24 - why does this happen? According to comment 28, rsvg_handle_set_dpi_x_y should exists as we use it (and current CVS certainly works with all the 2.12.x I tried). Dom, are 2.13.2 and 2.7.1 (see comment 24) unstable and/or buggy versions?
If checking the actual version is so complicated, why don't we just write #ifdef svg_handle_set_dpi_x_y /* foo */ #else /* bar */ #endif ???
Why is this issue so hard to put to rest??? If the version numbering sheme used by librsvg is sequential in some deterministic manner, the resolution should be simple. -----------------------------------------------------------------------> TIME ... | | | ... | Vn Vn+1 Vn+2 Vp #if (VERSION < Vk) rsvg_handle_set_dpi_x_y (handle, vals->resolution, vals->resolution); #else rsvg_handle_set_dpi (handle, vals->resolution); #endif The problem is reduced to how to specify the inequality. The librsvg team has "surrendered" and has added support for both functions in its current version. -Joseph
So why would we have to change anything (except the check) if rsvg_handle_set_dpi_x_y is supported with 2 parameters from now on? Is rsvg_handle_set_dpi_x_y deprecated? I want a "yes" or "no" as an answer, and whoever answers it will be held liable if it turns out to be incorrect :)
librsvg-2.11.0.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.11.1.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.12.4.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.12.5.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.12.6.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.12.7.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.13.0.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.13.1.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.13.2.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi_x, double dpi_y); librsvg-2.13.3.tar.bz2 rsvg.h:void rsvg_handle_set_dpi (RsvgHandle * handle, double dpi); rsvg.h:void rsvg_handle_set_dpi_x_y (RsvgHandle * handle, double dpi_x, double dpi_y);
Created attachment 56064 [details] [review] Patch against 2.3.5 GIMP to work around for librsvg versions {2.11.0, 2.13.0, 2.13.1, 2.13.2} Please check my last post for the version specific function declarations. This's a source level fix. I am not sure if an ABI-compatible cross-version fix is even possible.
The patch doesn't have \ line endings for the multi-line macro, so please add them when you apply it.
Created attachment 56154 [details] [review] patch that I am going to apply to both branches
2005-12-19 Sven Neumann <sven@gimp.org> * plug-ins/common/svg.c: fixed handling of librsvg API change, based on a patch by S. Mukund (bug #314400).