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

Portability guide for 0.28.0? #2630

Closed
0-wiz-0 opened this issue May 17, 2023 · 15 comments
Closed

Portability guide for 0.28.0? #2630

0-wiz-0 opened this issue May 17, 2023 · 15 comments
Labels
request feature request or any other kind of wish

Comments

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented May 17, 2023

0.28.0 broke a couple of exiv2 users. Please advise how the code should be updated.

kfilemetadata5 (https://github.com/KDE/kfilemetadata/blob/master/src/extractors/exiv2extractor.cpp)

[ 40%] Building CXX object src/extractors/CMakeFiles/kfilemetadata_taglibextractor.dir/__/kfilemetadata_debug.cpp.o
/scratch/sysutils/kfilemetadata5/work/kfilemetadata-5.98.0/src/extractors/exiv2extractor.cpp: In function 'QVariant {anonymous}::toVariantLong(const Exiv2::Value&)':
/scratch/sysutils/kfilemetadata5/work/kfilemetadata-5.98.0/src/extractors/exiv2extractor.cpp:78:31: error: 'const class Exiv2::Value' has no member named 'toLong'
   78 |         qlonglong val = value.toLong();
      |                               ^~~~~~
/scratch/sysutils/kfilemetadata5/work/kfilemetadata-5.98.0/src/extractors/exiv2extractor.cpp: In member function 'double KFileMetaData::Exiv2Extractor::fetchGpsAltitude(const Exiv2::ExifData&)':
/scratch/sysutils/kfilemetadata5/work/kfilemetadata-5.98.0/src/extractors/exiv2extractor.cpp:313:39: error: 'const class Exiv2::Value' has no member named 'toLong'
  313 |             auto altRef = it->value().toLong();
      |                                       ^~~~~~

geeqie (https://github.com/BestImageViewer/geeqie/blob/master/src/exiv2.cc)
geeqie.log

gexiv2 (https://gitlab.gnome.org/GNOME/gexiv2/-/issues/74)
gexiv2.log

Thanks!

@kmilos kmilos added request feature request or any other kind of wish help wanted labels May 17, 2023
@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 17, 2023

Here's the kfilemetadata5 issue for this:
https://bugs.kde.org/show_bug.cgi?id=469607

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 17, 2023

geeqie one: BestImageViewer/geeqie#1090

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue May 18, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue May 18, 2023
too much breakage, wait for fixes for geeqie, gexiv2, kfilemetadata5
Exiv2/exiv2#2630
@neheb
Copy link
Collaborator

neheb commented May 20, 2023

Use toInt64 instead

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 20, 2023

Thanks. Do you have suggestions for geeqie too?
(gexiv2 might be fixed in their git head.)

@neheb
Copy link
Collaborator

neheb commented May 20, 2023

@kevinbackhouse
Copy link
Collaborator

In 0.28.0, we have attempted to eliminate the long type because it is 32-bit on Windows and 64-bit on most other platforms. That's why toLong has been replaced by toInt64.

@kmilos
Copy link
Collaborator

kmilos commented May 23, 2023

Btw, this seems to be always true for me (MSYS2 UCRT64 GCC 12.2) now somehow for any type

#ifdef __cpp_if_constexpr
if constexpr (std::is_same_v<T, bool>) {
#else
if (std::is_same<T, bool>::value) {
#endif

so e.g. this darktable code now fails:
xmp[regionKey + "/mwg-rs:Area/stArea:w"] = XmpTextValue(std::to_string(w));
with

C:/msys64/opt/exiv2/include/exiv2/xmp_exiv2.hpp:403:36: error: could not convert 'value' from 'const
 Exiv2::XmpTextValue' to 'bool'
  403 |     setValue(Exiv2::toString(value ? "True" : "False"));
      |                              ~~~~~~^~~~~~~~~~~~~~~~~~
      |                                    |
      |                                    const Exiv2::XmpTextValue
compilation terminated due to -Wfatal-errors.

@kmilos
Copy link
Collaborator

kmilos commented May 23, 2023

Also had to change Exiv2::AnyError to Exiv2::Error, and DataBuf access.

@neheb
Copy link
Collaborator

neheb commented May 23, 2023

Thanks. Do you have suggestions for geeqie too?
(gexiv2 might be fixed in their git head.)

I have a patch for this.

@neheb
Copy link
Collaborator

neheb commented May 23, 2023

Btw, this seems to be always true for me (MSYS2 UCRT64 GCC 12.2) now somehow for any type

#ifdef __cpp_if_constexpr
if constexpr (std::is_same_v<T, bool>) {
#else
if (std::is_same<T, bool>::value) {
#endif

so e.g. this darktable code now fails: xmp[regionKey + "/mwg-rs:Area/stArea:w"] = XmpTextValue(std::to_string(w)); with

C:/msys64/opt/exiv2/include/exiv2/xmp_exiv2.hpp:403:36: error: could not convert 'value' from 'const
 Exiv2::XmpTextValue' to 'bool'
  403 |     setValue(Exiv2::toString(value ? "True" : "False"));
      |                              ~~~~~~^~~~~~~~~~~~~~~~~~
      |                                    |
      |                                    const Exiv2::XmpTextValue
compilation terminated due to -Wfatal-errors.

After speaking with ChatGPT, it seems if constexpr should be removed from headers.

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented May 26, 2023

Thanks. Do you have suggestions for geeqie too?
(gexiv2 might be fixed in their git head.)

I have a patch for this.

That's great! Is it for exiv2 or geeqie? If the latter, can you please share it with at BestImageViewer/geeqie#1090 ?

@kmilos
Copy link
Collaborator

kmilos commented Jun 8, 2023

Well, looks like Arch maintainers did it: https://archlinux.org/todo/exiv2-028/ 🚀

@neheb
Copy link
Collaborator

neheb commented Jun 8, 2023

Great

@neheb
Copy link
Collaborator

neheb commented Jul 18, 2023

geeqie is fixed. can this issue be closed?

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented Jul 18, 2023

Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request feature request or any other kind of wish
Projects
None yet
Development

No branches or pull requests

4 participants