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

fix_enableBMFF_v1 #1554

Merged
merged 2 commits into from
Apr 14, 2021
Merged

fix_enableBMFF_v1 #1554

merged 2 commits into from
Apr 14, 2021

Conversation

clanmills
Copy link
Collaborator

In a team discussion on Element following RC2, we agreed that in Exiv2 v1.00, we would:

  1. Build the function Exiv2::enableBMFF(bool) into the library (even when -DEXIV2_ENABLE_BMFF=False)
  2. The bool returned by Exiv2::enableBMFF(bool) would indicate if the library was built with bmff support.
  3. The default for -DEXIV2_ENABLE_BMFF=On
  4. The documentation would be appropriated updated.

Built with cmake .. -DEXIV2_ENABLE_BMFF=On

517 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $ nm -g --demangle lib/libexiv2.dylib | grep enableBMFF
0000000000163c60 T Exiv2::enableBMFF(bool)
518 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $ make version_test | grep bmff

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
enable_bmff          1
519 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $

Built with cmake .. -DEXIV2_ENABLE_BMFF=Off

528 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $ nm -g --demangle lib/libexiv2.dylib | grep enableBMFF
00000000001632a0 T Exiv2::enableBMFF(bool)
529 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $ make version_test | grep bmff

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
enable_bmff          0
530 rmills@rmillsmm-local:~/gnu/github/exiv2/main/build $ 

@clanmills clanmills added this to the v1.00 milestone Apr 14, 2021
@clanmills clanmills requested review from 1div0 and hassec April 14, 2021 15:29
@clanmills clanmills self-assigned this Apr 14, 2021
hassec
hassec previously approved these changes Apr 14, 2021
Copy link
Member

@hassec hassec left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@1div0 1div0 left a comment

Choose a reason for hiding this comment

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

OK, makes sense.

@clanmills clanmills merged commit 64185f5 into main Apr 14, 2021
@clanmills clanmills deleted the fix_enableBMFF_v1 branch April 14, 2021 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants