contrib: add meson build #2464

neheb · 2023-01-16T01:08:29Z

This is a fairly basic meson file that builds only the library. Useful for stuff like WrapDB.

Signed-off-by: Rosen Penev rosenp@gmail.com

codecov · 2023-01-16T01:18:13Z

Codecov Report

Merging #2464 (f1e54c9) into main (e309680) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2464   +/-   ##
=======================================
  Coverage   62.04%   62.04%           
=======================================
  Files         102      102           
  Lines       22764    22764           
  Branches    11205    11205           
=======================================
  Hits        14124    14124           
  Misses       6433     6433           
  Partials     2207     2207

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

piponazo · 2023-01-16T07:16:52Z

Do you think it is needed to provide some documentation explaining how this file is supposed to be used? Or developers experienced with Meson (I am not) will manage to find out easily?

neheb · 2023-01-16T10:17:00Z

meson is dead simple so probably not. unfortunately i broke my hand after this PR so any changes will have to wait.

piponazo · 2023-01-16T10:33:01Z

meson is dead simple so probably not. unfortunately i broke my hand after this PR so any changes will have to wait.

Sorry to hear that! Take care

ghost · 2023-01-26T01:22:25Z

👇 Click on the image for a new way to code review

Make big changes easier — review code in small groups of related files
Know where to start — see the whole change at a glance
Take a code tour — explore the change with an interactive tour
Make comments and review — all fully sync’ed with github

Try it now!

Legend

neheb · 2023-01-26T01:25:08Z

@piponazo Updated. One hand typing is hard :)

@eli-schwartz One issue I've noticed with the VSCode plugin is that it's not loading these ../../ files.

eli-schwartz · 2023-01-26T02:19:13Z

@eli-schwartz One issue I've noticed with the VSCode plugin is that it's not loading these ../../ files.

The build system isn't in the root of the project, it's in a subdirectory. The files that it isn't loading are files which are, as far as Meson knows, arbitrary files retrieved from the operating system -- not part of the project itself. So I'm not surprised that it mishandles this.

This is a fairly basic meson file that builds only the library. Useful for stuff like WrapDB.

This will not work, since the WrapDB -- and in general, use as a subproject -- requires there to be a meson.build file in the root of the project.

If it's a hard requirement to isolate the contributed meson.build files away from the project root, then it's possible to do what the zstd project does, and provide a minimal stub meson.build in contrib/meson/ and have that do a subdir() into the actual implementation files.

This will probably not fix VSCode, and it still cannot be used in the WrapDB directly, but the way zstd works is that the WrapDB provides its own minimal stub meson.build, installs it into the project root, and has that load the "actual implementation files". Since the only thing that needs to be duplicated in both places is the stub loader, that means the actual important bits are tracked in the project's own git repository and doesn't need to be duplicated.

Really though, that's sort of awkward. :/ Much simpler to simply place the meson.build in the project root, which also lets people use a wrapped subproject using git development versions of exiv2 for testing purposes.

neheb · 2023-01-26T02:25:41Z

sure, but the intention is not to replace cmake. It's not up to parity yet. From my experience, people like CMake better.

If no one has any objections, I'll move to the root directory.

eli-schwartz · 2023-01-26T02:33:29Z

sure, but the intention is not to replace cmake. It's not up to parity yet. From my experience, people like CMake better.

Sure, no problem. I'm simply observing that even for a build system which is documented as "unofficial, secondary, useful for embedding in other projects perhaps -- but should not be used by default for building the project", it probably still makes sense to launch that from the root directory.

It's "just" two files, and the root directory doesn't seem to be focusing on "lean and mean" -- there's more than 10 text files in the root directory, two png images, a whole bunch of dotfiles for tool configuration, and 3 directories containing various aspects of testing. Two more won't really hurt, and users will enter through the README file anyway (and there be told to type cmake).

In general I suspect that most software projects shouldn't have a steep tax on adding more files to the project root, as long as each file has a clear and distinct purpose. Unless the project specifically says otherwise. :)

kmilos · 2023-01-26T10:13:04Z

If no one has any objections, I'll move to the root directory.

Sure, as long as you include the disclaimer/note in the main README as well?

piponazo · 2023-01-26T20:02:00Z

Fine for me too to have the meson files in the root of the project. I will give it a try once I see them there ;)

eli-schwartz

READNE-meson

Slight typo in the name. :)

eli-schwartz · 2023-01-26T20:18:56Z

meson.build

+brotli_dep = dependency('libbrotlidec', disabler: true, required: false)
+if brotli_dep.found()
+  deps += brotli_dep
+endif
+
+curl_dep = dependency('libcurl', disabler: true, required: get_option('curl'))
+if curl_dep.found()
+  deps += curl_dep
+endif
+
+zlib_dep = dependency('zlib', disabler: true, required: get_option('png'))
+if zlib_dep.found()
+  deps += zlib_dep
+endif


As a matter of curiosity, why do these need to be disabler: true?

It only takes effect if the .found() method returns false, but everywhere you seem to first check for that anyway.

Aside: if you do not use disablers, then it is safe to append them unconditionally to deps +=. Not-found dependencies are no-ops by default and possess no compiler flags or linkable libraries, so they just do nothing when added... (that's why you also need to explicitly check .found() and set EXV_HAVE_*).

It’s mostly a hack to get the later set calls to not work and have the cmake and meson config files be more sinmilar.

The later set() calls are "working" and produce #undef.

the difference is

> /* #undef EXV_HAVE_LIBINTL_H */

vs

> #undef EXV_HAVE_LIBINTL_H

neheb · 2023-01-26T20:28:57Z

READNE-meson

Slight typo in the name. :)

Yeah I can’t type with one hand.

neheb · 2023-01-26T20:45:13Z

Added basic xmpsdk support.

neheb · 2023-01-26T23:25:28Z

With some MinGW testing, I found some bugs.

neheb · 2023-01-27T00:12:51Z

../include/exiv2/xmp_exiv2.hpp:401:28: warning: 'Exiv2::Xmpdatum::operator=' redeclared inline; 'dllimport' attribute ignored [-Wignored-attributes]
inline Xmpdatum& Xmpdatum::operator=(bool value) {

No idea how to fix this. Any ideas? ping @kmilos

neheb · 2023-01-27T00:22:51Z

@piponazo I see a warning about a bad function cast: https://github.com/Exiv2/exiv2/blob/main/src/basicio.cpp#L370

Is there a point to this block? I would think std::filesystem is enough.

kmilos · 2023-01-27T08:48:45Z

@neheb Need to add EXIV2API to both redeclarations on lines 401 and 406 I think.

Actually that might not work: https://stackoverflow.com/questions/11546403/importing-inline-functions-in-mingw so maybe it's better not to inline it at all?

neheb · 2023-01-27T14:01:44Z

But then it doesn't compile.

neheb · 2023-01-29T23:46:51Z

"cl" "-Iexiv2-1.dll.p" "-I." "-I.." "-I..\include\exiv2" "-I..\xmpsdk\include" "-I..\subprojects\expat-2.5.0\lib" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/Zc:__cplusplus" "/W2" "/EHsc" "/std:c++17" "/permissive-" "/Od" "/Zi" "-DEXIV2API=__declspec(dllexport)" "/Fdexiv2-1.dll.p\src_basicio.cpp.pdb" /Foexiv2-1.dll.p/src_basicio.cpp.obj "/c" ../src/basicio.cpp
../src/basicio.cpp(663): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(663): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(663): error C2059: syntax error: ')'
../src/basicio.cpp(831): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(831): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(831): error C2059: syntax error: ')'
../src/basicio.cpp(832): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(832): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(832): error C2059: syntax error: ')'
../src/basicio.cpp(1080): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1080): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(1080): error C2059: syntax error: ')'
../src/basicio.cpp(1110): warning C4003: not enough arguments for function-like macro invocation 'max'
../src/basicio.cpp(1110): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1110): error C2144: syntax error: 'unknown-type' should be preceded by ')'
../src/basicio.cpp(1110): error C2660: 'Exiv2::RemoteIo::Impl::getDataByRange': function does not take 0 arguments
../src/basicio.cpp(1032): note: see declaration of 'Exiv2::RemoteIo::Impl::getDataByRange'
../src/basicio.cpp(1110): note: while trying to match the argument list '()'
../src/basicio.cpp(1110): error C2144: syntax error: 'unknown-type' should be preceded by ';'
../src/basicio.cpp(1110): error C2059: syntax error: ')'
../src/basicio.cpp(1110): error C2059: syntax error: ')'
../src/basicio.cpp(1110): warning C4003: not enough arguments for function-like macro invocation 'max'
../src/basicio.cpp(1110): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1118): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1118): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(1118): error C2059: syntax error: ')'
../src/basicio.cpp(1243): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1243): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(1243): error C2059: syntax error: ')'
../src/basicio.cpp(1261): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1261): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(1261): error C2059: syntax error: ')'
../src/basicio.cpp(1457): warning C4003: not enough arguments for function-like macro invocation 'max'
../src/basicio.cpp(1457): warning C4003: not enough arguments for function-like macro invocation 'max'
../src/basicio.cpp(1457): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1457): error C2062: type 'unknown-type' unexpected
../src/basicio.cpp(1457): error C2589: '(': illegal token on right side of '::'
../src/basicio.cpp(1457): error C2059: syntax error: ')'
../src/basicio.cpp(1457): error C2143: syntax error: missing ';' before '{'
[13/94] Compiling C++ object exiv2-1.dll.p/src_convert.cpp.obj
[14/94] Compiling C++ object exiv2-1.dll.p/src_datasets.cpp.obj
ninja: build stopped: subcommand failed.
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: C:\ProgramData\Chocolatey\bin\ninja.EXE -C D:/a/exiv2/exiv2/build
Error: Process completed with exit code 1.

Seems I need a hack to undefine max,

.github/workflows/meson.yml

Pull request has been modified.

meson.build

neheb · 2023-01-31T23:09:03Z

Sure, it's doable, but IMHO it's completely legitimate to say we require C++17 and GCC 8 on main/next version in order to have a clean code base and reduce maintenance burden.

If requiring GCC8, it may make sense to also update to C++20. GCC8 supports that partially.

This is a fairly basic meson file that builds only the library. Useful for stuff like WrapDB. Signed-off-by: Rosen Penev <rosenp@gmail.com>

Pull request has been modified.

neheb marked this pull request as ready for review January 16, 2023 01:08

neheb force-pushed the meson branch from 7cee4cc to 5e1f88f Compare January 16, 2023 04:16

neheb force-pushed the meson branch from 5e1f88f to f8d9923 Compare January 26, 2023 01:21

neheb force-pushed the meson branch 2 times, most recently from a7e2199 to 373ca23 Compare January 26, 2023 01:26

neheb force-pushed the meson branch from 373ca23 to 4261427 Compare January 26, 2023 20:11

eli-schwartz reviewed Jan 26, 2023

View reviewed changes

neheb force-pushed the meson branch 2 times, most recently from 21a44e3 to 43d1844 Compare January 26, 2023 20:44

neheb force-pushed the meson branch from 43d1844 to 29eb388 Compare January 26, 2023 23:24

neheb force-pushed the meson branch from 29eb388 to 730a417 Compare January 26, 2023 23:35

neheb force-pushed the meson branch from 457c5a1 to 030ad61 Compare January 28, 2023 22:06

neheb force-pushed the meson branch 2 times, most recently from 215bb92 to 79e4d17 Compare January 29, 2023 21:58

neheb force-pushed the meson branch 3 times, most recently from 2a7b9e4 to 9537009 Compare January 30, 2023 01:02

piponazo previously approved these changes Jan 30, 2023

View reviewed changes

.github/workflows/meson.yml Outdated Show resolved Hide resolved

neheb force-pushed the meson branch from 9537009 to 3ea378d Compare January 31, 2023 05:36

neheb force-pushed the meson branch 9 times, most recently from 5779ec6 to ed83686 Compare January 31, 2023 09:57

kmilos reviewed Jan 31, 2023

View reviewed changes

meson.build Show resolved Hide resolved

neheb force-pushed the meson branch from ed83686 to a680fb5 Compare January 31, 2023 22:44

piponazo previously approved these changes Feb 6, 2023

View reviewed changes

neheb added 2 commits February 5, 2023 23:52

add meson build

f18fc31

This is a fairly basic meson file that builds only the library. Useful for stuff like WrapDB. Signed-off-by: Rosen Penev <rosenp@gmail.com>

add basic meson CI

f1e54c9

neheb force-pushed the meson branch from a680fb5 to f1e54c9 Compare February 6, 2023 07:54

piponazo approved these changes Feb 6, 2023

View reviewed changes

neheb merged commit 8d3e0d6 into Exiv2:main Feb 6, 2023

neheb deleted the meson branch February 6, 2023 21:45

nehaljwani mentioned this pull request May 8, 2023

Is a new release happening soon? #2406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contrib: add meson build #2464

contrib: add meson build #2464

neheb commented Jan 16, 2023

codecov bot commented Jan 16, 2023 •

edited

Loading

piponazo commented Jan 16, 2023

neheb commented Jan 16, 2023

piponazo commented Jan 16, 2023

ghost commented Jan 26, 2023 •

edited by ghost

Loading

neheb commented Jan 26, 2023

eli-schwartz commented Jan 26, 2023

neheb commented Jan 26, 2023

eli-schwartz commented Jan 26, 2023 •

edited

Loading

kmilos commented Jan 26, 2023

piponazo commented Jan 26, 2023

eli-schwartz left a comment

eli-schwartz Jan 26, 2023

neheb Jan 26, 2023

eli-schwartz Jan 26, 2023

neheb Jan 26, 2023

neheb commented Jan 26, 2023

neheb commented Jan 26, 2023

neheb commented Jan 26, 2023

neheb commented Jan 27, 2023

neheb commented Jan 27, 2023

kmilos commented Jan 27, 2023 •

edited

Loading

neheb commented Jan 27, 2023

neheb commented Jan 29, 2023

neheb commented Jan 31, 2023

contrib: add meson build #2464

contrib: add meson build #2464

Conversation

neheb commented Jan 16, 2023

codecov bot commented Jan 16, 2023 • edited Loading

Codecov Report

piponazo commented Jan 16, 2023

neheb commented Jan 16, 2023

piponazo commented Jan 16, 2023

ghost commented Jan 26, 2023 • edited by ghost Loading

Legend

neheb commented Jan 26, 2023

eli-schwartz commented Jan 26, 2023

neheb commented Jan 26, 2023

eli-schwartz commented Jan 26, 2023 • edited Loading

kmilos commented Jan 26, 2023

piponazo commented Jan 26, 2023

eli-schwartz left a comment

Choose a reason for hiding this comment

eli-schwartz Jan 26, 2023

Choose a reason for hiding this comment

neheb Jan 26, 2023

Choose a reason for hiding this comment

eli-schwartz Jan 26, 2023

Choose a reason for hiding this comment

neheb Jan 26, 2023

Choose a reason for hiding this comment

neheb commented Jan 26, 2023

neheb commented Jan 26, 2023

neheb commented Jan 26, 2023

neheb commented Jan 27, 2023

neheb commented Jan 27, 2023

kmilos commented Jan 27, 2023 • edited Loading

neheb commented Jan 27, 2023

neheb commented Jan 29, 2023

neheb commented Jan 31, 2023

codecov bot commented Jan 16, 2023 •

edited

Loading

ghost commented Jan 26, 2023 •

edited by ghost

Loading

eli-schwartz commented Jan 26, 2023 •

edited

Loading

kmilos commented Jan 27, 2023 •

edited

Loading