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

cmake: Rename xmp to exiv2-xmp to avoid name conflicts #626

Merged
merged 1 commit into from
Jan 3, 2019

Conversation

cryptomilk
Copy link
Collaborator

There is already a libxmp file by the xmp project on https://xmp.sf.net.
To avoid issues prefix with exiv2.

Fixes #624

Signed-off-by: Andreas Schneider [email protected]

There is already a libxmp file by the xmp project on https://xmp.sf.net.
To avoid issues prefix with exiv2.

Fixes Exiv2#624

Signed-off-by: Andreas Schneider <[email protected]>
@codecov
Copy link

codecov bot commented Jan 2, 2019

Codecov Report

Merging #626 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #626   +/-   ##
=======================================
  Coverage   62.56%   62.56%           
=======================================
  Files         154      154           
  Lines       21109    21109           
=======================================
  Hits        13206    13206           
  Misses       7903     7903

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9843844...89509d7. Read the comment docs.

@clanmills
Copy link
Collaborator

Is this really needed? It has been this way since before I joined the project in 2008?

There is another request which is close to this concerning statics in Adobe's XMPsdk result in linking issues with users of XMPsdk or libexmpi. #459.

For v0.27, we can build Adobe XMPsdk 2016 Edition to be linked externally. This was done for a user in North Carolina who has a product that uses Adobe XMPsdk in addition to Exiv2. I suggested wrapping the statics, however he reported back that he couldn't get that to work. He fixed it with code change for which he didn't want to submit a PR (he described it as an ugly hack that we wouldn't accept). I suspect he renamed all the statics in Adobe's code.

#459 was reported when I was working long days to get v0.27.0 RC1 ready. I declined it as I didn't have the band-width to go into all of this for v0.27.

If you believe we must change the name of the library (it is a static library), perhaps we should also consider the namespace request for Adobe's statics.

@cryptomilk
Copy link
Collaborator Author

Before the libxmp.a was deleted in the rpm spec file. However this could lead to issues when linking using the cmake config file I guess. However I didn't look into it in detail.

However when it is installed it should not be named libxmp as there is already a package providing it!

@cryptomilk
Copy link
Collaborator Author

Why doesn't exiv2 use libexempi instead of the xmpsdk?

@clanmills
Copy link
Collaborator

I think it's historical. I believe the Adobe XMPsdk code was added about 2006 to Exiv2 (by a Microsoft Engineer). I can't remember what exempi is. It could be Adobe's code packaged to build with autotools. Although I worked at Adobe in San Jose, California, I had nothing to do with metadata or XMPsdk.

@cryptomilk
Copy link
Collaborator Author

It looks like libexempi is a maintained version of xmpsdk. See https://cgit.freedesktop.org/exempi/log/

It seems to have fixed a lot of compiler warning and security issues like buffer overflows and infinite loops. I'm not sure you have the same fixes in the fork. I think it is worth looking into to and thinking about migrating to it for v0.28.0. It seems to be available in distributions by default.

In the build directory you have files to build on Windows and MacOSX too.

@clanmills
Copy link
Collaborator

Replace XMPsdk with exempi? That's a tough sell to the team. We will have a team meeting in early March to discuss the objectives for v0.28. Luis and Dan started new jobs on December 1, so they have other priorities at the moment. Alison and I will be in the States for 4 weeks in February to visit family and escape the English winter.

The "big deal" for v0.28 is to "modernise" the code to C++11 and drop C++98 support. We'd like to rewrite the Tiff Parser to support BigTiff and Canon's new CR3 format. We also want to deprecate some features. Extended XMP support is on the list.

I will maintain v0.27 with "dot" releases for a couple of years. If we drop somebody's "must have feature", they can continue with v0.27 while we reconsider, or they prepare the restoration patch/PR. Starting with v0.27.1, you'll get compiler deprecation warnings when you compile anything that we intend to remove in v0.28.

Adobe published new editions of XMPsdk in 2013, 2014 and 2016. So they were on an 18 month update cycle. However it's been 30 months now. Perhaps they've lost interest.

Copy link
Collaborator

@piponazo piponazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a good contribution, since it will help to avoid name clashes between the external XMP library and the one compiled as part of exiv2.

Thanks!

@piponazo piponazo merged commit e27353a into Exiv2:master Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libxmp] Name conflict with xmp project
3 participants