Revert "Lower deprecation versions to what was tested GIT_SILENT"

Authored by mlaurent on Apr 20 2020, 7:18 AM.

Description

Revert "Lower deprecation versions to what was tested GIT_SILENT"

Until now all compiles fine. I don't want to wait that a day we see
that there is a lot of deprecated method.
We are able to fix all compile error for the moment.

My rule is not "Waiting that we switch to qt6 for preparing to migrate
it as for other kde apps".

This reverts commit d09b4b2113d288d0dc958a82573dfe537dfe6575.

dfaure added a subscriber: dfaure.Apr 20 2020, 7:36 AM

@mlaurent we keep hitting compile failures in released KDE applications. We cannot keep shipping code with 0x060000.
My idea of testing for .git was not good enough, there are distros that package from git tags.

You can add a cmake option and set it for your build, but we can't ship code with builtin time bombs.

mlaurent added a comment.EditedApr 20 2020, 8:54 AM

if a distro decides to package an application which was not released based on git source it's the problem of distro not my problem.

Downgrade version when we release a version as we do in pim ok but for un-release apps not.

Adding a cmake option is not useful as nobody will activate it only me.

pino added a subscriber: pino.Apr 20 2020, 9:36 AM

if a distro decides to package an application which was not released based on git source it's the problem of distro not my problem.

There are also distros that keep the sources of packages in git repositories. In this case .git exists but it is not the upstream git repository. The assumption "it is a git repository, so it is the upstream one" is not correct.

Downgrade version when we release a version as we do in pim ok but for un-release apps not.

Then people will still hit the issue when they try to build from sources.

Adding a cmake option is not useful as nobody will activate it only me.

Look Laurent: I understand you care about removing deprecated functions, and that is a good thing. However, there are more things to consider:

  • having a deprecated function is not the worst thing to have, it's simply... a deprecated function, that will stay there until the next ABI break (so Qt6/KF6); in the meanwhile, that function keeps working as it did before the deprecation
  • switching away from a deprecated function can happen also in the medium/long term, no need to force people to do that immediately breaking any other pending work
  • not all the other people have removing deprecated functions as top priority: implemeting a new feature or fixing a bug may be simply more important for other people, and this gets in their way
  • in case you are not around for any reason for a long time period (vacation, sickness, etc), thins kind of hard block will prevent others from just building the application, with no real reason

if a distro decides to package an application which was not released based on git source it's the problem of distro not my problem.

There are also distros that keep the sources of packages in git repositories. In this case .git exists but it is not the upstream git repository. The assumption "it is a git repository, so it is the upstream one" is not correct.

As I wrote if distro decided to use git repo it's not my problem, they want to use unrelease code source, they accept the potential problem. Using an unrelease apps is never a good solution for a distro.

Downgrade version when we release a version as we do in pim ok but for un-release apps not.

Then people will still hit the issue when they try to build from sources.

So they will provide a patch/reporting a bug/downgrade it.

Adding a cmake option is not useful as nobody will activate it only me.

Look Laurent: I understand you care about removing deprecated functions, and that is a good thing. However, there are more things to consider:

  • having a deprecated function is not the worst thing to have, it's simply... a deprecated function, that will stay there until the next ABI break (so Qt6/KF6); in the meanwhile, that function keeps working as it did before the deprecation
  • switching away from a deprecated function can happen also in the medium/long term, no need to force people to do that immediately breaking any other pending work
  • not all the other people have removing deprecated functions as top priority: implemeting a new feature or fixing a bug may be simply more important for other people, and this gets in their way
  • in case you are not around for any reason for a long time period (vacation, sickness, etc), thins kind of hard block will prevent others from just building the application, with no real reason

As maintainer of application I can manage it no ?
I removed these macros in several kde apps, David did it too.
But for my apps I decide to keeping it and it's my choice.

My priority is to remove all deprecated methods before qt6.

pino added a comment.Apr 20 2020, 10:29 AM

if a distro decides to package an application which was not released based on git source it's the problem of distro not my problem.

There are also distros that keep the sources of packages in git repositories. In this case .git exists but it is not the upstream git repository. The assumption "it is a git repository, so it is the upstream one" is not correct.

As I wrote if distro decided to use git repo it's not my problem, they want to use unrelease code source, they accept the potential problem. Using an unrelease apps is never a good solution for a distro.

That is not what I wrote, let me explain it once more.

There are distributions that use git repositories to keep not only the packaging files (debian/ directory, .spec files, etc) but also the whole upstream sources of releases.
In that case, the git repository is not the upstream one, and assuming ".git exists so behave like a build of sources from upstream git" is wrong.

This has nothing to do with unreleased applications.

Downgrade version when we release a version as we do in pim ok but for un-release apps not.

Then people will still hit the issue when they try to build from sources.

So they will provide a patch/reporting a bug/downgrade it.

This is all extra work that you are forcing on other people for no gain.

Adding a cmake option is not useful as nobody will activate it only me.

Look Laurent: I understand you care about removing deprecated functions, and that is a good thing. However, there are more things to consider:

  • having a deprecated function is not the worst thing to have, it's simply... a deprecated function, that will stay there until the next ABI break (so Qt6/KF6); in the meanwhile, that function keeps working as it did before the deprecation
  • switching away from a deprecated function can happen also in the medium/long term, no need to force people to do that immediately breaking any other pending work
  • not all the other people have removing deprecated functions as top priority: implemeting a new feature or fixing a bug may be simply more important for other people, and this gets in their way
  • in case you are not around for any reason for a long time period (vacation, sickness, etc), thins kind of hard block will prevent others from just building the application, with no real reason

As maintainer of application I can manage it no ?
I removed these macros in several kde apps, David did it too.
But for my apps I decide to keeping it and it's my choice.

Sure, it is your choice, however you should also listen to what others say, especially when they are telling you this way is making other people's work more difficult. You are sort of forcing your own workflow (build everything with latest qt5/kf5 from git) on everybody else, and I would not be surprised if people do not contribute because this makes it too hard to even get things built.

Please think about others, Laurent.

My priority is to remove all deprecated methods before qt6.

Sure, and that is good for you: other people would really only build/use things though, and maybe do random fixes.

My idea of testing for .git was not good enough, there are distros that package from git tags.

The .git approach lays out another trap:
developer builds with .git present see other API than non-developer builds see. So there is the chance that implicit conversions or method overloads suddenly get triggered, resulting in failed builds or unexpected calls with broken runtime behaviour.

So this .git approach should better not be done IMHO in any case.

The best idea I had so far to support Laurent's workflow might be to have a CMake switch, like -DDISABLE_ALL_QTKF_DEPRECATED, to enable the fail-build-on-deprecated-Qt/KF-API:

option(DISABLE_ALL_QTKF_DEPRECATED "Usage of deprecated Qt/KF API fails build" OFF)
if (DISABLE_ALL_QTKF_DEPRECATED)
    add_definitions(
        -DQT_DISABLE_DEPRECATED_BEFORE=0x060000
        -DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x060000)
    )
else()
    add_definitions(
        -DQT_DISABLE_DEPRECATED_BEFORE=0x050e00
        -DQT_DEPRECATED_WARNINGS_SINCE=0x060000 # to enable warnings for non-disabled
        -DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x054400
        -DKF_DEPRECATED_WARNINGS_SINCE=0x060000 # to enable warnings for non-disabled
endif()

That way Laurent (and others who prefer that forced-fails) could have their way without conflicting with others.

What do you think? Perhaps we could also add some things to KDECompilerSettings with some macro, so all the boilerplate is hidden away, and all projects just would need one call like this:

ecm_disable_deprecated_api(QT 0x050e00 KF 0x054400))

Could be even enhance to allow passing normal version numbers, so things are less cryptic (especially now that minor version is >9.

I like the idea very much.

pino added a comment.Apr 20 2020, 11:11 AM

The best idea I had so far to support Laurent's workflow might be to have a CMake switch, like -DDISABLE_ALL_QTKF_DEPRECATED, to enable the fail-build-on-deprecated-Qt/KF-API:

Sadly Laurent already said he doesn't want it:

Adding a cmake option is not useful as nobody will activate it only me.

The point is to set this when compiling git master. Maybe in kdesrc-build's default setup (for kf5-qt5).

Adding a cmake option is not useful as nobody will activate it only me.

On the other hand everybody running into failed builds due to deprecated API hit rather went and changed/removed the *DISABLE_DEPRECATED_BEFORE* instead. And in the end it was mainly you doing all the porting work still, no?

So I think such a flag would still be a middle-ground, and allowing everyone to approach things for them as they prefer, without forcing each other into things.

if a distro decides to package an application which was not released based on git source it's the problem of distro not my problem.

There are also distros that keep the sources of packages in git repositories. In this case .git exists but it is not the upstream git repository. The assumption "it is a git repository, so it is the upstream one" is not correct.

As I wrote if distro decided to use git repo it's not my problem, they want to use unrelease code source, they accept the potential problem. Using an unrelease apps is never a good solution for a distro.

That is not what I wrote, let me explain it once more.

There are distributions that use git repositories to keep not only the packaging files (debian/ directory, .spec files, etc) but also the whole upstream sources of releases.
In that case, the git repository is not the upstream one, and assuming ".git exists so behave like a build of sources from upstream git" is wrong.

This has nothing to do with unreleased applications.

Downgrade version when we release a version as we do in pim ok but for un-release apps not.

Then people will still hit the issue when they try to build from sources.

So they will provide a patch/reporting a bug/downgrade it.

This is all extra work that you are forcing on other people for no gain.

no gain. For sure there is a gain, people helps me to compile against deprecated method.

Adding a cmake option is not useful as nobody will activate it only me.

Look Laurent: I understand you care about removing deprecated functions, and that is a good thing. However, there are more things to consider:

  • having a deprecated function is not the worst thing to have, it's simply... a deprecated function, that will stay there until the next ABI break (so Qt6/KF6); in the meanwhile, that function keeps working as it did before the deprecation
  • switching away from a deprecated function can happen also in the medium/long term, no need to force people to do that immediately breaking any other pending work
  • not all the other people have removing deprecated functions as top priority: implemeting a new feature or fixing a bug may be simply more important for other people, and this gets in their way
  • in case you are not around for any reason for a long time period (vacation, sickness, etc), thins kind of hard block will prevent others from just building the application, with no real reason

As maintainer of application I can manage it no ?
I removed these macros in several kde apps, David did it too.
But for my apps I decide to keeping it and it's my choice.

Sure, it is your choice, however you should also listen to what others say, especially when they are telling you this way is making other people's work more difficult. You are sort of forcing your own workflow (build everything with latest qt5/kf5 from git) on everybody else, and I would not be surprised if people do not contribute because this makes it too hard to even get things built.

Please think about others, Laurent.

My priority is to remove all deprecated methods before qt6.

Sure, and that is good for you: other people would really only build/use things though, and maybe do random fixes.

Adding a cmake option is not useful as nobody will activate it only me.

On the other hand everybody running into failed builds due to deprecated API hit rather went and changed/removed the *DISABLE_DEPRECATED_BEFORE* instead. And in the end it was mainly you doing all the porting work still, no?

So I think such a flag would still be a middle-ground, and allowing everyone to approach things for them as they prefer, without forcing each other into things.

Nope David did it when he compiled qt master with pim*
or other guys which fixed against qt5.14.1 new deprecated method.

Indeed I did 90% of porting but not all.
Without this flags nobody ported it

Well, if it's opt-in, I will set that flag on the computer where I build git master (and not on the one where I build the stable branch).
I'm sure you can convince Allen to do the same ;)
As I said, we can put it in kdesrc-build's default configuration. It's a different target audience than "everyone who builds this code".

kossebau added a comment.EditedApr 20 2020, 11:52 AM

Indeed I did 90% of porting but not all.
Without this flags nobody ported it

I know I contributed to porting deprecated API use in Okteta & KDevelop but also other places like KF modules without such forced-build-break setup ;)

And I wonder if David would not have also done some porting in either case. Quite some people care about warning-free builds, just do things when they are in mood or have the time for concentrated work.
Being forced to port deprecated API can also results in poor porting with low code quality, as the focus is on unbreaking the build, not on making proper use of the new recommended API (and I remember having seen a few cases in KDE where this was clearly the case).

Indeed I did 90% of porting but not all.
Without this flags nobody ported it

I know I contributed to porting deprecated API use in Okteta & KDevelop but also other places like KF modules without such forced-build-break setup ;)

And I wonder if David would not have also done some porting in either case. Quite some people care about warning-free builds, just do things when they are in mood or have the time for concentrated work.
Being forced to port deprecated API can also results in poor porting with low code quality, as the focus is on unbreaking the build, not on making proper use of the new recommended API (and I remember having seen a few cases in KDE where this was clearly the case).

But porting when we port during qt6 period will be better that porting as incremental during period before it will be better ?
I remember that we did it for porting to kde3-kde4 or kde4-kf5 and we made a lot of changes during one period (during qt switching) and we created a lot of porting bug.