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

EXIV2_TEST_VERSION is removed in 0.27 #555

Closed
busykai opened this issue Nov 19, 2018 · 10 comments
Closed

EXIV2_TEST_VERSION is removed in 0.27 #555

busykai opened this issue Nov 19, 2018 · 10 comments
Assignees
Milestone

Comments

@busykai
Copy link

busykai commented Nov 19, 2018

This commit removes EXIV2_TEST_VERSION macro along with deprecated EXIV2_CHECK_VERSION, seemingly by error.

It brakes gexiv2, for example. I'm not at all sure the intention was to leave no way to check the version.

@busykai busykai changed the title EXIV2_TEST_VERSION is removed EXIV2_TEST_VERSION is removed in 0.27 Nov 19, 2018
@clanmills clanmills added this to the v0.27 milestone Nov 19, 2018
@clanmills
Copy link
Collaborator

@busykai Thank You for bringing this to our attention.

I have submitted a change which I would like @piponazo to review.
@busykai, please test this with gexiv2 to confirm that it fixes your situation as I never built gexiv2.

commit 89375979d8c5b0ce667851cd2d7afb40fc844abf (HEAD -> 0.27-RC3)
Author: Robin Mills <robin@clanmills.com>
Date:   Mon Nov 19 11:38:26 2018 +0000

    https://github.com/Exiv2/exiv2/issues/555
    I have restored the Macro EXIV2_TEST_VERSION in include/exiv2/version.hpp
    I have added an option --version-test to exifprint.cpp to test/validate EXIV2_TEST_VERSION works as documented.
    Version strings in Exiv2 v0.27 and later have a fourth digit to indicate the pre-release number of the build.
    Pre-release builds should never be used for production purposes.

@piponazo
Copy link
Collaborator

Hi. I have to admit that I went to fast doing these changes. I did not see any usage of those macros and functions in our code and I thought they were some leftovers.

I just learned about the existence of gexiv2. Thanks for informing us about this issue, I will check in the future projects like gexiv2 or digikam before removing stuff from the API.

@clanmills It seems that you did not push that commit yet into github. The branch 0.27-RC3 is not there yet. I will review your changes when you push it.

clanmills added a commit that referenced this issue Nov 19, 2018
I have restored the Macro EXIV2_TEST_VERSION in include/exiv2/version.hpp
I have added an option --version-test to exifprint.cpp to test/validate EXIV2_TEST_VERSION works as documented.
Version strings in Exiv2 v0.27 and later have a fourth digit to indicate the pre-release number of the build.
Pre-release builds should never be used for production purposes.
@clanmills
Copy link
Collaborator

I've pushed a bit harder. I thought it was odd that GitHub didn't offer a PR. It is now. I have more changes I'd like to add this week.

@busykai
Copy link
Author

busykai commented Nov 19, 2018

Thank you, guys, for quick response. I can confirm it fixes the gexiv2 build. Some patching on gexiv2 side is still needed to #include <exiv2/version.hpp> explicitly (before it was already included via some other exiv2 header), not sure whether it needs addressing: explicit is better, IMO.

P.S. The rename of exiv2/xmp.hpp to exiv2/xmp_exiv2.hpp is also a breaking change in 0.27.

@clanmills
Copy link
Collaborator

clanmills commented Nov 19, 2018

Thanks for giving us feedback.

I want to discourage everybody from including individual header files. Using #include <exiv2/exiv2.hpp> is the correct way to use exiv2 and gives us the freedom to reorganise without breaking your build.

So, can we summarise this issue as one error each. Our error is to have removed EXIV2_TEST_VERSION and your error is to have broken the rule about #include <exiv2/exiv2.hpp>

I gave @piponazo encouragement to clean up relics in the code. I've added the use of EXIV2_TEST_VERSION to samples/exifprint.cpp to be sure that we'll break our own build if we remove the macro.

Good Start to the week.

@busykai
Copy link
Author

busykai commented Nov 20, 2018

Using #include <exiv2/exiv2.hpp> is the correct way to use exiv2

Thank you, good to know! I'm not contributing to gexiv2 though. I was just reporting my observations.

@clanmills
Copy link
Collaborator

@busykai Thanks for this update. Can you let the gexiv2 team know about our discussion, please.

@clanmills
Copy link
Collaborator

@phako
Copy link
Contributor

phako commented Nov 20, 2018

Thanks for letting me know!

gnomesysadmins pushed a commit to GNOME/gexiv2 that referenced this issue Nov 20, 2018
To improve compatibility with exiv2 0.27 as recommended in

Fixes #34

Exiv2/exiv2#555 (comment)
gnomesysadmins pushed a commit to GNOME/gexiv2 that referenced this issue Nov 20, 2018
To improve compatibility with exiv2 0.27 as recommended in

Fixes #34

Exiv2/exiv2#555 (comment)
@clanmills
Copy link
Collaborator

Wow. It's amazing how effective team-work can be. I think this is done/dusted/dead. Thanks Everybody. I'm going to close this.

uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 8, 2019
Import upstream patch to fix build against graphcis/exiv2-0.27:
Exiv2/exiv2#555 (comment)

PR:             235943


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@495064 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Mar 8, 2019
Import upstream patch to fix build against graphcis/exiv2-0.27:
Exiv2/exiv2#555 (comment)

PR:             235943
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this issue Mar 8, 2019
Import upstream patch to fix build against graphcis/exiv2-0.27:
Exiv2/exiv2#555 (comment)

PR:             235943


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@495064 35697150-7ecd-e111-bb59-0022644237b5
svmhdvn pushed a commit to svmhdvn/freebsd-ports that referenced this issue Jan 10, 2024
Import upstream patch to fix build against graphcis/exiv2-0.27:
Exiv2/exiv2#555 (comment)

PR:             235943
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

No branches or pull requests

4 participants