optionally link the binaries against the dynamic library

[stefantalpalaru]

[CMake only]
The idea is to avoid compiling and installing more than one copy of the
common code between libgmic, gmic and gmic_gimp_gtk. The next step is to
do the same for the gmic-qt binaries.

Shorter compile times are relevant to Gentoo. Smaller installed size is
relevant to everybody.

[dtschump]

I've a problem with that idea. Basically when the file gmic.cpp is compiled for these different interfaces (cli, lib, plug-in,...), it uses different #define to manage what libraries have to be linked with each executable.
For instance, the cli interface depends on a lot of libraries, because it has to manage file input/output, whereas the plug-in for GIMP or Krita just gets input images from the host software.
I don't think having the plug-in depend on all input/output libraries requested by the CLI tool as libtiff, libjpeg, `opencv,... is a good idea. I don't think someone interested by the plug-in alone (most users) would be happy to see the monster OpenCV library gets installed on his system.

[dtschump]

I've a problem with that idea. Basically when the file gmic.cpp is compiled for these different interfaces (cli, lib, plug-in,...), it uses different #define to manage what libraries have to be linked with each executable.
For instance, the cli interface depends on a lot of libraries, because it has to manage file input/output, whereas the plug-in for GIMP or Krita just gets input images from the host software.
I don't think having the plug-in depend on all input/output libraries requested by the CLI tool as libtiff, libjpeg, `opencv,... is a good idea. I don't think someone interested by the plug-in alone (most users) would be happy to see the monster OpenCV library gets installed on his system.

[stefantalpalaru]

Dependencies are handled by packagers, like this: https://github.com/stefantalpalaru/gentoo-overlay/blob/534aa2f57d259ec7b2bab37a59931a4a76a5bd2a/media-gfx/gmic/gmic-2.1.5-r1.ebuild

all input/output libraries requested by the CLI tool as libtiff, libjpeg, `opencv,...

Those are actually optional. All a packager needs to do is make sure that when building the GTK+ GIMP plugin, the shared library supports png, fftw and X. Same for the gmic-qt binaries. If that shared library is also used by the CLI tool, add whatever else they need to it.

That's the whole point of shared libraries - put your common code in one place instead of compiling and shipping separate copies for each separate binary.

I don't think someone interested by the plug-in alone (most users) would be happy to see the monster OpenCV library gets installed on his system.

Nothing in this patch forces such a dependency or changes anything about the default behaviour.

[stefantalpalaru]

Dependencies are handled by packagers, like this: https://github.com/stefantalpalaru/gentoo-overlay/blob/534aa2f57d259ec7b2bab37a59931a4a76a5bd2a/media-gfx/gmic/gmic-2.1.5-r1.ebuild

all input/output libraries requested by the CLI tool as libtiff, libjpeg, `opencv,...

Those are actually optional. All a packager needs to do is make sure that when building the GTK+ GIMP plugin, the shared library supports png, fftw and X. Same for the gmic-qt binaries. If that shared library is also used by the CLI tool, add whatever else they need to it.

That's the whole point of shared libraries - put your common code in one place instead of compiling and shipping separate copies for each separate binary.

I don't think someone interested by the plug-in alone (most users) would be happy to see the monster OpenCV library gets installed on his system.

Nothing in this patch forces such a dependency or changes anything about the default behaviour.

[dtschump]

OK, I was misunderstanding the intentions.
I'll try to review this patch ASAP but I'm quite busy this week.

[dtschump]

OK, I was misunderstanding the intentions.
I'll try to review this patch ASAP but I'm quite busy this week.

[dtschump]

Just had a quick look to the proposed patch this morning and I think this confirms what I've said in my previous post. There seems to be a problem with extra dependencies (or non-enabled features).

Basically, the gmic::run() function is intended to behave differently depending on the compilation flags used (and thus, on dependencies on external libraries).
For instance, if I enable flag -Dcimg_use_opencv when I compile, then G'MIC command camera, parsed at line 6165 will be able to input frames from the webcam (as CImg method CImg<T>::get_load_camera() will use OpenCV features to do so). If I don't define -Dcimg_use_opencv then CImg<T>::get_load_camera() will throw an exception, leading to a G'MIC error.

What I mean is that -Dcimg_use_opencv ultimately makes a difference in the code of gmic::run(), and as the code is not exactly the same, it cannot be shared among the G'MIC interface. In one case, you have the feature enabled (and you need to link your code with the OpenCV libraries), on the other case, you don't have the feature enabled (and no linking with a monster like OpenCV is required).

This concerns a lot of features in G'MIC, not only the webcam input. OpenCV is used to read/write frames from video files, libpng, libjpeg, libtiff, are used to input/output image files, etc...
When the compiling flags -Dcimg_use_libpng, -Dcimg_use_libjpeg, -Dcimg_use_libtiff, ... are enabled, this changes the code defined in the CImg Library (file CImg.h), and at the end, the code of the gmic::run() function.

For these reasons, I think the code of gmic::run() cannot be easily shared between the different G'MIC interfaces. With your patch, gmic::run() would be compiled only once, and used for all interfaces
as it lies between the #define gmic_dynamic_linking...#endif directives.
To make this code shareable, the same define flags should be used for all G'MIC interfaces, which is not something desirable IMHO.

[dtschump]

Just had a quick look to the proposed patch this morning and I think this confirms what I've said in my previous post. There seems to be a problem with extra dependencies (or non-enabled features).

Basically, the gmic::run() function is intended to behave differently depending on the compilation flags used (and thus, on dependencies on external libraries).
For instance, if I enable flag -Dcimg_use_opencv when I compile, then G'MIC command camera, parsed at line 6165 will be able to input frames from the webcam (as CImg method CImg<T>::get_load_camera() will use OpenCV features to do so). If I don't define -Dcimg_use_opencv then CImg<T>::get_load_camera() will throw an exception, leading to a G'MIC error.

What I mean is that -Dcimg_use_opencv ultimately makes a difference in the code of gmic::run(), and as the code is not exactly the same, it cannot be shared among the G'MIC interface. In one case, you have the feature enabled (and you need to link your code with the OpenCV libraries), on the other case, you don't have the feature enabled (and no linking with a monster like OpenCV is required).

This concerns a lot of features in G'MIC, not only the webcam input. OpenCV is used to read/write frames from video files, libpng, libjpeg, libtiff, are used to input/output image files, etc...
When the compiling flags -Dcimg_use_libpng, -Dcimg_use_libjpeg, -Dcimg_use_libtiff, ... are enabled, this changes the code defined in the CImg Library (file CImg.h), and at the end, the code of the gmic::run() function.

For these reasons, I think the code of gmic::run() cannot be easily shared between the different G'MIC interfaces. With your patch, gmic::run() would be compiled only once, and used for all interfaces
as it lies between the #define gmic_dynamic_linking...#endif directives.
To make this code shareable, the same define flags should be used for all G'MIC interfaces, which is not something desirable IMHO.

[stefantalpalaru]

To make this code shareable, the same define flags should be used for all G'MIC interfaces, which is not something desirable IMHO.

It is, when installing them from the same package which is exactly my use case.

You don't seem to believe than the optional OpenCV support is just as optional for the CLI tool, the GTK+ GIMP plugin, the Qt5 GIMP plugin, the Krita plugin and the standalone GUI. I have all these installed on my system right now and I don't understand why would you want to compile them with different features enabled in gmic::run().

[stefantalpalaru]

To make this code shareable, the same define flags should be used for all G'MIC interfaces, which is not something desirable IMHO.

It is, when installing them from the same package which is exactly my use case.

You don't seem to believe than the optional OpenCV support is just as optional for the CLI tool, the GTK+ GIMP plugin, the Qt5 GIMP plugin, the Krita plugin and the standalone GUI. I have all these installed on my system right now and I don't understand why would you want to compile them with different features enabled in gmic::run().

[dtschump]

I understand your point. Even if I think that your case is not common.
Most G'MIC users (i.e. users of the plug-in) wouldn't like to install OpenCV on their system to have a working plug-in.

Anyway, I've made a set of commits today to ease the compilation of G'MIC interfaces with a shared library. I've added a new entry allshared: to the Makefile to show how this can be achieved
(see https://github.com/dtschump/gmic/blob/master/src/Makefile#L535). As you will see, it is now a bit simpler to handle, and with a minimum amount of modifications required in the source files.

Please tell me what you think about this.

[dtschump]

I understand your point. Even if I think that your case is not common.
Most G'MIC users (i.e. users of the plug-in) wouldn't like to install OpenCV on their system to have a working plug-in.

Anyway, I've made a set of commits today to ease the compilation of G'MIC interfaces with a shared library. I've added a new entry allshared: to the Makefile to show how this can be achieved
(see https://github.com/dtschump/gmic/blob/master/src/Makefile#L535). As you will see, it is now a bit simpler to handle, and with a minimum amount of modifications required in the source files.

Please tell me what you think about this.

[stefantalpalaru]

Please tell me what you think about this.

Honestly, I think you're taking the piss :-)

You removed "resources/CMakeLists.txt" which I was using to build my Gentoo package. As much as I hate CMake, it still has better structure and maintainability than your handcrafted Makefile where you do stuff like explicitly invoking "make" in a rule.

You're not supposed to write by hand such complex Makefiles, you're supposed to use Autotools. Failing that, at least use some other devil we know like CMake.

I'm reading your Makefile more carefully now and it's pure and utter madness. Checking for the existence of "/tmp/sk1" to decide whether pkg-config is available to get the libpng CFLAGS? Seriously?

There is no way to optionally disable support for any (optional in the code) dependency. Everything is automatically enabled if it is detected, even heavy stuff you were complaining about like OpenCV. No wonder you don't grasp the concept of configure-time dependency selection by passing arguments to a configuration program.

One other thing that makes no sense (besides the humorous OS detection based on "uname" where everything other than "Darwin" is labelled "Unix", each in individual "if" blocks) is using a Makefile in one Git repo to build software in 2 other separate Git repos (that are not submodules). What was the point of separating them in the first place? Now you rely on some fixed top dir names for the checked out repos and relative filesystem position in order to build binaries in all 3 of them. How is that sane?

You ended up reinventing the wheel for building the shared library without using Libtool. This is precisely why you're supposed to generate your Makefiles with Autotools and not write it by hand. But wait, there's more: you build the same shared library again in your new "_allshared" target. The same you already built for the "_lib" target.

You really thought this new and improved Makefile is a replacement for the CMakeLists.txt you deleted? That's sadder than your use of dots as commit comments (at least add some lines and write Morse code).

As you will see, it is now a bit simpler to handle, and with a minimum amount of modifications required in the source files.

I don't understand why set_variable() is used to get a variable's value, with a '.' operation that is ignored in the function body, but as far as I am concerned, anything that removes that "gmic_main" define check from the library and works for triggering CLI-specific code at run time instead of compile time does the job.

[stefantalpalaru]

Please tell me what you think about this.

Honestly, I think you're taking the piss :-)

You removed "resources/CMakeLists.txt" which I was using to build my Gentoo package. As much as I hate CMake, it still has better structure and maintainability than your handcrafted Makefile where you do stuff like explicitly invoking "make" in a rule.

You're not supposed to write by hand such complex Makefiles, you're supposed to use Autotools. Failing that, at least use some other devil we know like CMake.

I'm reading your Makefile more carefully now and it's pure and utter madness. Checking for the existence of "/tmp/sk1" to decide whether pkg-config is available to get the libpng CFLAGS? Seriously?

There is no way to optionally disable support for any (optional in the code) dependency. Everything is automatically enabled if it is detected, even heavy stuff you were complaining about like OpenCV. No wonder you don't grasp the concept of configure-time dependency selection by passing arguments to a configuration program.

One other thing that makes no sense (besides the humorous OS detection based on "uname" where everything other than "Darwin" is labelled "Unix", each in individual "if" blocks) is using a Makefile in one Git repo to build software in 2 other separate Git repos (that are not submodules). What was the point of separating them in the first place? Now you rely on some fixed top dir names for the checked out repos and relative filesystem position in order to build binaries in all 3 of them. How is that sane?

You ended up reinventing the wheel for building the shared library without using Libtool. This is precisely why you're supposed to generate your Makefiles with Autotools and not write it by hand. But wait, there's more: you build the same shared library again in your new "_allshared" target. The same you already built for the "_lib" target.

You really thought this new and improved Makefile is a replacement for the CMakeLists.txt you deleted? That's sadder than your use of dots as commit comments (at least add some lines and write Morse code).

As you will see, it is now a bit simpler to handle, and with a minimum amount of modifications required in the source files.

I don't understand why set_variable() is used to get a variable's value, with a '.' operation that is ignored in the function body, but as far as I am concerned, anything that removes that "gmic_main" define check from the library and works for triggering CLI-specific code at run time instead of compile time does the job.

[dtschump]

Please note that the CMakeFile has been removed because it doesn't seem to work anymore. I'm not the author of this CMakeFiles, I'd be happy to re-include it if someone makes something that works.
Most of the strange tests you mention and you've seen in the Makefile have been written by the MacOSX maintainer, so I don't endorse the responsibility for this :)

[dtschump]

Please note that the CMakeFile has been removed because it doesn't seem to work anymore. I'm not the author of this CMakeFiles, I'd be happy to re-include it if someone makes something that works.
Most of the strange tests you mention and you've seen in the Makefile have been written by the MacOSX maintainer, so I don't endorse the responsibility for this :)

[dtschump]

Of course, if you know how to use autotools for the project, you are welcome to submit something as well.
Most of the time when I use CMake or autotools to compile a project from scratch, it just doesn't work, so I cannot say these tools look very attractive.
But as a packager, you probably know better how to use them.

[dtschump]

Of course, if you know how to use autotools for the project, you are welcome to submit something as well.
Most of the time when I use CMake or autotools to compile a project from scratch, it just doesn't work, so I cannot say these tools look very attractive.
But as a packager, you probably know better how to use them.

[stefantalpalaru]

Please note that the CMakeFile has been removed because it doesn't seem to work anymore.

That's because you probably broke it. It worked just fine with my patch.

Of course, if you know how to use autotools for the project, you are welcome to submit something as well.

After what you did with this contribution painfully and carefully crafted to not change the default behaviour in any way?

Most of the time when I use CMake or autotools to compile a project from scratch, it just doesn't work, so I cannot say these tools look very attractive.

Do you develop on Windows?

I'm not trying to insult you, you're obviously a good researcher and a smart computer scientist, but you're also an amateur at this other stuff that looks more like architecture, engineering and plumbing than algorithm development.

[stefantalpalaru]

Please note that the CMakeFile has been removed because it doesn't seem to work anymore.

That's because you probably broke it. It worked just fine with my patch.

Of course, if you know how to use autotools for the project, you are welcome to submit something as well.

After what you did with this contribution painfully and carefully crafted to not change the default behaviour in any way?

Most of the time when I use CMake or autotools to compile a project from scratch, it just doesn't work, so I cannot say these tools look very attractive.

Do you develop on Windows?

I'm not trying to insult you, you're obviously a good researcher and a smart computer scientist, but you're also an amateur at this other stuff that looks more like architecture, engineering and plumbing than algorithm development.

[dtschump]

That's because you probably broke it. It worked just fine with my patch.

Your patch had some issues I've also fixed. The modifications you made on the source code were not acceptable as it.

After what you did with this contribution painfully and carefully crafted to not change the default behaviour in any way?

The default behavior is the way most people want it to work. I have to repeat it twice, but using a shared library bloated with all possible dependencies for all the interface is a crappy idea.
I've done some work to make it possible though, but don't think I'll use it in any way. Now, if you want to tweak the CMakeFile to make it work with this updated version, then go for it.
I think nobody use the CMakeFile anyway.

Do you develop on Windows?

Nope.

I'm not trying to insult you, you're obviously a good researcher and a smart computer scientist, but you're also an amateur at this other stuff that looks more like architecture, engineering and plumbing than algorithm development.

That is insulting actually.
This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request, with kindness for the project.

[dtschump]

That's because you probably broke it. It worked just fine with my patch.

Your patch had some issues I've also fixed. The modifications you made on the source code were not acceptable as it.

After what you did with this contribution painfully and carefully crafted to not change the default behaviour in any way?

The default behavior is the way most people want it to work. I have to repeat it twice, but using a shared library bloated with all possible dependencies for all the interface is a crappy idea.
I've done some work to make it possible though, but don't think I'll use it in any way. Now, if you want to tweak the CMakeFile to make it work with this updated version, then go for it.
I think nobody use the CMakeFile anyway.

Do you develop on Windows?

Nope.

I'm not trying to insult you, you're obviously a good researcher and a smart computer scientist, but you're also an amateur at this other stuff that looks more like architecture, engineering and plumbing than algorithm development.

That is insulting actually.
This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request, with kindness for the project.

[stefantalpalaru]

Your patch had some issues I've also fixed.

The proof of the pudding is in the eating. I am running 6 G'MIC binaries all linked dynamically to the same libgmic.so on my system using my 3 patches. There are no issues. The default case remains the one in which you compile and link the common code for each and every of those binaries. I tested it to make sure that nothing changed. There were no issues.

You'll understand, at this point, if I say that you don't know what you're talking about.

The default behavior is the way most people want it to work.

And that's why I made sure it remained unchanged.

I have to repeat it twice, but using a shared library bloated with all possible dependencies for all the interface is a crappy idea.

You still don't get the distinction between optional dependencies and separating common code in a shared library, but this is basic stuff in software architecture.

I've done some work to make it possible though, but don't think I'll use it in any way.

And that's your problem right there. You take the patch I'm actually using and replace it with some garbage that you yourself don't use, but it's somehow supposed to be better because you wrote it and that makes it "acceptable".

Now, if you want to tweak the CMakeFile to make it work with this updated version, then go for it.

My masochism has its limits.

This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request

Yes, of course.

[stefantalpalaru]

Your patch had some issues I've also fixed.

The proof of the pudding is in the eating. I am running 6 G'MIC binaries all linked dynamically to the same libgmic.so on my system using my 3 patches. There are no issues. The default case remains the one in which you compile and link the common code for each and every of those binaries. I tested it to make sure that nothing changed. There were no issues.

You'll understand, at this point, if I say that you don't know what you're talking about.

The default behavior is the way most people want it to work.

And that's why I made sure it remained unchanged.

I have to repeat it twice, but using a shared library bloated with all possible dependencies for all the interface is a crappy idea.

You still don't get the distinction between optional dependencies and separating common code in a shared library, but this is basic stuff in software architecture.

I've done some work to make it possible though, but don't think I'll use it in any way.

And that's your problem right there. You take the patch I'm actually using and replace it with some garbage that you yourself don't use, but it's somehow supposed to be better because you wrote it and that makes it "acceptable".

Now, if you want to tweak the CMakeFile to make it work with this updated version, then go for it.

My masochism has its limits.

This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request

Yes, of course.

[dtschump]

This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request
Yes, of course.

So that ends this discussion. Thanks for your efforts.

[dtschump]

This is free software, you are free to apply your own patch to the sources I provide.
I'm free to reject your pull request
Yes, of course.

So that ends this discussion. Thanks for your efforts.

[a17r]

Please reconsider this PR. Yes, the contributor should have been more diplomatic, but rejecting this completely, and moreover regressing to handcrafted Makefiles, is sad. As a packager, the latter makes me very reluctant to touch this.

[a17r]

Please reconsider this PR. Yes, the contributor should have been more diplomatic, but rejecting this completely, and moreover regressing to handcrafted Makefiles, is sad. As a packager, the latter makes me very reluctant to touch this.

[HJarausch]

It's probably my inability, but I couldn't get the Makefile working here.
Furthermore, the gmic-qt plugin does use Cmake and G'MIC itself does NOT use it currently.
This mixture doesn't make it easier for people like me.
So, please !!!, both of you calm down, and agree to some 'build system'.
A pure Makefile isn't flexible enough.
I don't like Cmake either. Some people (on Gentoo) seem to prefer 'meson' or 'ninja' or even 'scons'
but I have no idea which is best for this project.

Thanks for this wonderful software!
Helmut

[HJarausch]

It's probably my inability, but I couldn't get the Makefile working here.
Furthermore, the gmic-qt plugin does use Cmake and G'MIC itself does NOT use it currently.
This mixture doesn't make it easier for people like me.
So, please !!!, both of you calm down, and agree to some 'build system'.
A pure Makefile isn't flexible enough.
I don't like Cmake either. Some people (on Gentoo) seem to prefer 'meson' or 'ninja' or even 'scons'
but I have no idea which is best for this project.

Thanks for this wonderful software!
Helmut

[dtschump]

This PR won't be reconsidered as it is, because it is not compatible anymore with the current version of the G'MIC source code. I've done some modifications in the code structure to fulfill the initial request though : Now the G'MIC interfaces can be easily compiled to use the same shared version of libgmic
(see Makefile entry allshared: to see how it works).
These modifications also broke the CMakefiles.txt so I removed it as well, as I'm absolutely not the creator of this (old) file and I do not maintain it. So when it was not working anymore, I decided to remove it (I don't know how to fix it and don't want to have non-working stuffs in the current repo).

Of course, if someone is skilled enough (and is friendly at the same time), then he would be welcome to propose a working CMakefiles.txt if he can ensure he will maintain/update it in case of another change in the code structure.

Also if you experience issues with the Makefile please report them, I'm willing to fix those. I personally find the Makefile very easy, simple and efficient to use, a lot more than other solutions based on qmake, cmake or whatever.

[dtschump]

This PR won't be reconsidered as it is, because it is not compatible anymore with the current version of the G'MIC source code. I've done some modifications in the code structure to fulfill the initial request though : Now the G'MIC interfaces can be easily compiled to use the same shared version of libgmic
(see Makefile entry allshared: to see how it works).
These modifications also broke the CMakefiles.txt so I removed it as well, as I'm absolutely not the creator of this (old) file and I do not maintain it. So when it was not working anymore, I decided to remove it (I don't know how to fix it and don't want to have non-working stuffs in the current repo).

Of course, if someone is skilled enough (and is friendly at the same time), then he would be welcome to propose a working CMakefiles.txt if he can ensure he will maintain/update it in case of another change in the code structure.

Also if you experience issues with the Makefile please report them, I'm willing to fix those. I personally find the Makefile very easy, simple and efficient to use, a lot more than other solutions based on qmake, cmake or whatever.

[a17r]

Of course, if someone is skilled enough (and is friendly at the same time), then he would be welcome to propose a working CMakefiles.txt if he can ensure he will maintain/update it in case of another change in the code structure.

Frankly, your commit message style is making that impossible. If you work like that on your local clone to then squash several related commits into a meaningful message, fine, your business. Expecting collaboration on a git log like this: Not going to happen.

[a17r]

Of course, if someone is skilled enough (and is friendly at the same time), then he would be welcome to propose a working CMakefiles.txt if he can ensure he will maintain/update it in case of another change in the code structure.

Frankly, your commit message style is making that impossible. If you work like that on your local clone to then squash several related commits into a meaningful message, fine, your business. Expecting collaboration on a git log like this: Not going to happen.

[dtschump]

I understand your point.
I'm not familiar enough with cmake to create a maintain a Makefile for it, plus I don't really need it.
If someone else wants to create/maintain it, it's fine, otherwise never mind :)

[dtschump]

I understand your point.
I'm not familiar enough with cmake to create a maintain a Makefile for it, plus I don't really need it.
If someone else wants to create/maintain it, it's fine, otherwise never mind :)