Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major ABI break in 7.1.1 #6144

Closed
remicollet opened this issue Mar 9, 2023 · 3 comments · Fixed by #6145
Closed

Major ABI break in 7.1.1 #6144

remicollet opened this issue Mar 9, 2023 · 3 comments · Fixed by #6145

Comments

@remicollet
Copy link
Contributor

ImageMagick version

7.1.1-0

Operating system

Linux

Operating system, version and so on

Fedora

Description

In 6b2ae4e

- VERS_10.0 {
+ VERS_10.1 {

This is a major ABI break as linker consider everything as new symbols.

Notice: in RPM packaging, such thing is track

$rpm -q --provides ImageMagick-libs
libMagickWand-7.Q16HDRI.so.10(VERS_10.0)(64bit)
...

Also change in enums (pixel.h) is a ABI breakage, as value defined at buildtime may change at runtime

@@ -49,8 +49,8 @@
   IndexChannel = 0x0020,             /* Color Index Table? */
   ReadMaskChannel = 0x0040,          /* Pixel is Not Readable? */
   WriteMaskChannel = 0x0080,         /* Pixel is Write Protected? */
-  MetaChannel = 0x0100,              /* not used */
-  CompositeMaskChannel = 0x0200,     /* SVG mask */
+  CompositeMaskChannel = 0x0100,     /* SVG mask */
+  MetaChannel = 0x0200,              /* not used */
   CompositeChannels = 0x001F,
   AllChannels = 0x7ffffff,
   /*
@@ -89,8 +89,8 @@
   IndexPixelChannel = 5,
   ReadMaskPixelChannel = 6,
   WriteMaskPixelChannel = 7,
-  MetaPixelChannel = 8,
-  CompositeMaskPixelChannel = 9,
+  CompositeMaskPixelChannel = 8,
+  MetaPixelChannel = 9,
   MetaPixelChannels = 10,
   IntensityPixelChannel = MaxPixelChannels,  /* ???? */
   CompositePixelChannel = MaxPixelChannels,  /* ???? */

IMHO, such breaking changes have no value...

DrawInfo struct also have change (haven't dig a lot to see if this may break things, allocation seems done in the library, not in the user app)

Steps to Reproduce

$ objdump -T libMagickWand-7.Q16HDRI.so.10.0.0
...
00000000000a0420 g    DF .text  00000000000001a2  VERS_10.0   MagickWriteImageFile
000000000008e6d0 g    DF .text  000000000000012f  VERS_10.0   MagickEdgeImage
000000000009ea10 g    DF .text  0000000000000183  VERS_10.0   MagickSpliceImage
000000000007eda0 g    DF .text  000000000000b8fc  VERS_10.0   ImportImageCommand
...
$ objdump -T libMagickWand-7.Q16HDRI.so.10.0.1
...
00000000000a0420 g    DF .text  00000000000001a2  VERS_10.1   MagickWriteImageFile
000000000008e6d0 g    DF .text  000000000000012f  VERS_10.1   MagickEdgeImage
000000000009ea10 g    DF .text  0000000000000183  VERS_10.1   MagickSpliceImage
000000000007eda0 g    DF .text  000000000000b8fc  VERS_10.1   ImportImageCommand
...

Images

No response

@remicollet
Copy link
Contributor Author

About map files, new entries may be added for new symbols (symbol versioning), but unchanged symbol have to stay in previous entry.

Of course ABI break are possible, but in such case, please bump all soname (MagickCore, MagickWand and Magick++)

@urban-warrior
Copy link
Member

urban-warrior commented Mar 9, 2023

The enum change was required to properly support multi-spectral images. We anticipated some ABI breakage which we tried to account for by bumping the library revision. We did add a member to DrawInfo, but it was added as the last member and should not disrupt the ABI.

@urban-warrior
Copy link
Member

To date we did not have proper support for an array of meta channels, e.g. meta channels 1-6. To add support we required that the meta channel be the last channel in the enum and additional meta-channels would follow, e.g., meta1, meta2, meta3, etc. Because of the ABI disruption, we think it is best to restore the previous enums so the ABI does not break and introduce a new enum to manage an array of meta channels. We'll commit your pull request, add a patch to refactor an array of meta channels, and issue a new release, hopefully within a day or two. Thanks for your assistance with this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants