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

Install FindLibRaw.cmake from the autotools build system too. #42

Closed
wants to merge 1 commit into from

Conversation

tasn
Copy link

@tasn tasn commented Oct 23, 2014

Helps in fixing #41.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

This may fix cmake, but will broke autotools.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Why? it just installs another file from autotools' build... I don't understand how it can possibly break autotools.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Have you even tested it? Because I have, and it's the same code I wrote for many other autotools projects that have had the same issue.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

There is no way to "test" on all distros and all possible user options (such as different prefix and include prefix).
So, rule is simple: if it not broken, do not fix it. Autotools are not broken.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

But it is broken, I already gave you the link to the Arch bug report in the ticket. You are not installing FindLibRaw.cmake, this is BROKEN.

Furthermore, this is a matter of installing an extra file, there's nothing "dangerous" here.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

Autotools are not broken

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Repeating that won't help you build a case. Autotools doesn't install all the files users of the library have learned to expect.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

So, it looks it is better to completely remove cmake scripts from LibRaw, these scripts creates too many headaches.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Do that if that's what you think is right. Having an unsupported build system is a bad idea anyway, you should choose one, autotools or cmake. However, this patch is still relevant even if you remove cmake completely, as this is for users of your library, it's like pkg-config but for cmake users.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

I agree, unsupported build system is bad.
Cmake support removed from master.
I do not think I need to release 0.16.1 with this change, BTW

@tasn
Copy link
Author

tasn commented Oct 23, 2014

That's great news. You don't need to release 0.16.1 with the change that removed cmake, you need to release 0.16.1 with this pull request, which fixes cmake for users. Especially since there's no harm in installing this extra file, and it only helps cmake users using the lib. It has zero effect on libraw as a lib.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

By the way, the other project that relies on FindLibRaw.cmake is:
libkdcraw kf5 framework

Just so you know, that people elsewhere already rely on this being installed. This is why we got into needing it in arch in the first place.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

LibRaw 0.16.1 definitely will not be released.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

OK. I'm done wasting my time. I wish I could argue with you, but it's impossible to argue with someone who doesn't read what you say, doesn't use facts or evidence and generally doesn't care.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

Thank you for your kind and polite words. Hope you enjoy using LibRaw.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Again, what I said is evidence based, nothing impolite:
Doesn't read what you say: "This may fix cmake, but will broke autotools." If you have read the patch you would have seen it has nothing to do with breaking autotools.
Doesn't use facts or evidence: "Autotools are not broken" and everything else you said wasn't backed by anything.
Doesn't care: "LibRaw 0.16.1 definitely will not be released." and your general attitude towards collaboration with other projects.

Please, maybe consider pointing out how I was impolite, especially since I spent a good hour or more trying to help you and your project, while you didn't even bother to try to understand the work I was doing to help you.

I am however quite irritated by this whole interaction, so I may have crossed the lines to impoliteness at some point, and if I have, I'm sorry.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

You ask me to backport some patch from upstream and release 0.16.1 (ASAP?).
I do this only for security patches. Point.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

That's what I wished you were going to do, but as I said in the other post, I get it if your process is different. This pull request has nothing to do with 0.16.1 (although it'll help).

If you include this pull request into master, this will at least be fixed in 0.17.0, and also, I was hoping this will be included in the branch for the future 0.16.1. I wish 0.16.1 was now, but I get it if it's not what you do, but even being in the branch staging there, is a first step towards knowing it's a supported fix that will be there in the future, and will make it possible for distros to assume it'll be there and build on top of it (patch it locally) until those versions are released.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

0.17 (master) is already fixed by removing cmake scripts I'm unable to support

@tasn
Copy link
Author

tasn commented Oct 23, 2014

This breaks external libraries and applications that use it. You only need to keep the file I shipped in this pull request (1 file, won't change, pretty short) in order to support those. It's the equivalent of a pkg-config file for the cmake world. Sec, I'll update my pull request.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Because this one is already close I had to open a new one. Please take a look at #43. It's not a lot to support, and I'm sure the past contributors to your cmake build system will help you if it ever breaks (it shouldn't). I can also be of any assistance if that ever happens.

@LibRaw
Copy link
Owner

LibRaw commented Oct 23, 2014

Yes, some changes sometimes breaks something. LibRaw 0.15 drops some api calls, this is more destructive than build system drop. That's life.

@tasn
Copy link
Author

tasn commented Oct 23, 2014

Yes, sometimes we have no choice but break things, and that is life, but we don't have to break if we can avoid it.

Your argument is the equivalent of people die, that's life, why even bother with having hospitals.

@LibRaw
Copy link
Owner

LibRaw commented Oct 24, 2014

LibRaw Cmake scripts are copied (without history) to separate repo: https://github.com/LibRaw/LibRaw-cmake
Contact me if you wish to contribute to this repo or support the scripts in any other way.

@tasn
Copy link
Author

tasn commented Oct 24, 2014

Thanks for the gesture, but it hardly helps.

I couldn't care less about the cmake build system, only about the one cmake
file in #43. However, if it's not in core, it's not going into Linux
distros who would need it, so it doesn't solve the issue I was referring to.

Though I guess having them out there for public reference is better than
nothing.

Tom.
On 24 Oct 2014 08:35, "LibRaw LLC" notifications@github.com wrote:

LibRaw Cmake scripts are copied (without history) to separate repo:
https://github.com/LibRaw/LibRaw-cmake
Contact me if you wish to contribute to this repo or support the scripts
in any other way.


Reply to this email directly or view it on GitHub
#42 (comment).

@LibRaw
Copy link
Owner

LibRaw commented Oct 24, 2014

Many packages requires additional scripting from linux distro builders, so Libraw is not an exclusion. So, the 'gesture' is not for linux distros (I do not care about their problems), but for developers who use libraw (usually not in linux)

@tasn
Copy link
Author

tasn commented Oct 24, 2014

I'm not aware of those many packages, and since you are not a Linux guy at all (by your own admission), I don't know where you got that from. I use Arch, a vanilla distro, they change as little as possible. Furthermore, the only reason why the Linux distros need it, is because other projects like Qt5 need it, and Qt5 is cross platform anyway, so this issue is across platforms.
Unless there's a major user of LibRAw on Windows/Mac that I'm unaware of, Linux has a lot of users of LibRaw, just because of the fact that it's depended upon by Enlightenment (unlike Qt5, we don't depend on FindLibRaw.cmake) and Qt5.

@LibRaw
Copy link
Owner

LibRaw commented Oct 24, 2014

Qt5 do not use LibRaw, I know it exactly because I use Qt4 and Qt5 in my projects.
Also, Qt5 uses qmake, not cmake for building.

Qmake .pro files are already in LibRaw repository. The files are good enough for casual Qt/QtCreator use (last time I checked it several days ago because I was asked how to use LibRaw in Qt Ctreator project). Also, these files are used to generate visual studio .vcproj files included into distribution to ease life of Windows developers.

@tasn
Copy link
Author

tasn commented Oct 24, 2014

Sorry, just re-checked, you are right, it's the "Gwenview KF5" that confused me.

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

Successfully merging this pull request may close these issues.

None yet

2 participants