Testing file offset bits?

[Stephen Kelly]

Alexander Neundorf wrote:

> There are two issues here:
> This change potentially breaks some applications, and not an in obvious
> way. This is bad in itself. We should only do that (should we actually
> ever do that ?) if there is a really good reason for it.
> I don't see any.
> What problems does keeping those defines create ?

Keeping them is 'not cleaning up' for me. I also just think it's better to 
use INTERFACE_COMPILE_DEFINITIONS so that using the correct define is not 
optional, in the way that using KDECompilerSettings.cmake is optional.

> And the second one is, that yes, it is responsibility of "KDE" to make the
> life of KDE developers easier, by providing sets of flags and settings
> which make development in some way easier.

I'd be surprised if this broke any KDE SC application, worked on by KDE 
Developers.

> Have a look at KDECMakeSettings.cmake, KDECompilerSettings.cmake and maybe
> also KDEInstallDirs.cmake.
> There is not much (is there anything at all ?) in those files which is
> required to build an application which uses some KDE libraries.
> Nobody has to use them, everybody can perfectly build his own application
> just using the Config.cmake files created by the frameworks libraries,
> they contain all the *necessary* information.

Yes. 

> 
> The purpose of those three files is to make life for (lazy ?) developers
> easier, so they don't have to care much about cmake or RPATH or compiler
> settings, but simply get a recommended set of flags which should make
> everything work as expected.

Yes. I'd say these files are not optional for 'KDE Applications' for 
uniformity. I can't imagine any 'KDE Developer' choosing not to use them 
anyway. People just copy and paste snippets of cmake code without knowing 
what they do. The use of those there files will be blindly copied around 
just like 

  find_package(KDE4 REQUIRED)
  include(KDE4Defaults)
  include(MacroLibrary)

is currently. I don't think this is a bad thing. I'm just making the point 
that for 'KDE Applications' these 3 files will practically always be used.

> 
> Nobody really needs the following lines to build a KDE application:
> if (... GNU)
>   ...
>   set(CMAKE_CXX_FLAGS_DEBUG  "-g -O2 -fno-reorder-blocks
>   -fno-schedule-insns -
> fno-inline")
>    ...
> 
> This is all just convenience.

Yes.

> 
> I'm very much for reverting that change.

Understood.

> 
>  
>> Particularly as we're creating a 'breaking release' (KF5), and generally
> 
> I thought the goal is to keep breakage and porting work as low as
> possible. As I said above, I don't think we need to add breakage which is
> not necessary.

Yes, one of the goals is to keep breakage and porting to a minimum. Another 
goal is to clean up.

I can't imagine this actually breaking any KDE SC code. KDE SC applications 
will likely use whatever target kde_file.h moves to, and they will get the 
same define as before.

> 
>> making our code more appropriate for ourselves, I don't think we need to
>> be concerned with non-conformant use of KDE4 libs and I don't think we
> 
> I am not aware of what "non-conformat use of KDE libs" is.
> Using open() is perfectly fine in my opinion, not everybody is writing
> cross- platform software.

Ok. If you use open(), I expect you to know how to use it and what you are 
doing, including knowing what compiler defines you need, without relying on 
the define coming from somewhere else.

>> > You may say that direct usage of open() etc. is considered bad in
>> > kdelibs or KDE SC, but it is valid POSIX API, and any 3rd party
>> > developer is free to use it and I'm far from saying that it is per se a
>> > bad thing.
>> 
>> kde_file.h says that its open() is a replacement, so it should be used
>> instead of the direct open().
>> 
>> My opinion is that we should confine defines to where we know they are
>> required (as I did in my commit),
> 
> As I said above, not much in those three files is "required", then we
> could remove them completely.

I don't think that's necessary.

I still think that having the define in the non-optional 
INTERFACE_COMPILE_DEFINITIONS is better. You can either look for other 
opinions, decide you feel stongly enough about it to revert, or leave it as 
is. I'm not going to complain either way, but you haven't convinced me to 
change my mind :).

Thanks,

Steve.