KDE/kdelibs

[Andreas Pakulat]

On 26.11.09 21:30:38, Sebastian Trueg wrote:
> Alexander Neundorf wrote:
> > Then, please post it to kde-buildsystem (k-c-d) is also ok, and wait for an 
> > explicit ok:
> > http://techbase.kde.org/Policies/CMake_Commit_Policy
> 
> here we go: the attached patch should integrate all your suggestions.
> Please have a look.

Comments on that below inline.
 
> > P.S. and please also explain what's up with FindKonto.cmake and 
> > FindKNepomuk.cmake.
> 
> They are both from pre-KDE 4.0 times. I have no idea why we still have
> them in there. I suspect that I simply forgot to remove them. After all
> what is Nepomuk today (and was so in 4.0) had at least two different
> names during development.
> 
> So I doubt anyone ever used the cmake macros. If so they never found
> anything. So I think they can safely be removed. But at least they can
> be cleaned out.

Well, for correctness sake they should then consist of a message(FATAL
"This module shouldn't be used, its only kept for backwards
compatibility, but won't work anymore") or similar (I think we have an
example in kdelibs/cmake/modules already for that. But this can wait
until after the beta release, so Alex can comment on it.
 
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt	(revision 1054513)
> +++ CMakeLists.txt	(working copy)
> @@ -96,7 +96,7 @@
>  macro_log_feature(SOPRANO_PLUGIN_REDLANDBACKEND_FOUND "Soprano Redland Backend" "Redland storage backend for Soprano" "http://soprano.sourceforge.net" FALSE "" "The Soprano redland backend is required to build the Nepomuk semantic desktop system.")
>  
>  macro_optional_find_package(SharedDesktopOntologies)
> -macro_log_feature(DESKTOP_ONTOLOGIES_FOUND "Shared desktop ontologies" "Desktop ontologies" "http://oscaf.sourceforge.net" FALSE "" "Ontologies necessary for the Nepomuk semantic desktop.")
> +macro_log_feature(SHAREDDESKTOPONTOLOGIES_FOUND "Shared desktop ontologies" "Desktop ontologies" "http://oscaf.sourceforge.net" FALSE "" "Ontologies necessary for the Nepomuk semantic desktop.")
>  
>  macro_optional_find_package(QCA2)
>  macro_log_feature(QCA2_FOUND "QCA2" "Qt Cryptographic Architecture" "http://delta.affinix.com/qca" FALSE "2.0.0" "Needed for the plasma remote widgets functionality.")
> @@ -227,10 +227,10 @@
>  
>  # Actually nepomuk is not optional, without it other KDE modules don't build,
>  # so this must be fixed. Alex
> -if(DESKTOP_ONTOLOGIES_FOUND)
> +if(Soprano_FOUND AND SHAREDDESKTOPONTOLOGIES_FOUND)
>    set(HAVE_NEPOMUK true)
>    add_subdirectory(nepomuk)
> -endif(DESKTOP_ONTOLOGIES_FOUND)
> +endif(Soprano_FOUND AND SHAREDDESKTOPONTOLOGIES_FOUND)

Mostly unrelated, so shouldn't be changed with this: if I understood the
discussions correctly, nepomuk is a defacto-dependency because other
modules (kdebase, kdepim) have it as a hard dependency. I'm all for
making things optional that not everybody needs, but if other modules
fail to build then and people need to rebuild kdelibs we should just
make it a hard dependency. Probably has to wait for KDE 4.5, but should
be kept in mind.
  
> +find_path (SHAREDDESKTOPONTOLOGIES_ROOT_DIR
> +  nie/nie.trig
> +  PATHS
> +  /usr/share
> +  /usr/local/share
> +  ${SHARE_INSTALL_PREFIX}
> +  ENV XDG_DATA_DIRS
> +  PATH_SUFFIXES ontology
> +)
> +if (SHAREDDESKTOPONTOLOGIES_ROOT_DIR)
> +  set (SHAREDDESKTOPONTOLOGIES_FOUND 1)
> +endif (SHAREDDESKTOPONTOLOGIES_ROOT_DIR)
> +
> +mark_as_advanced(SHAREDDESKTOPONTOLOGIES_ROOT_DIR)
> +
> +include(FindPackageHandleStandardArgs)
> +find_package_handle_standard_args(SharedDesktopOntologies DEFAULT_MSG SHAREDDESKTOPONTOLOGIES_ROOT_DIR)

If you use find_package_handle_standard_args you don't need the extra
set( XYZ_FOUND 1) above. So you can leave those 3 lines away.

> Index: cmake/modules/FindNepomuk.cmake
> ===================================================================
> --- cmake/modules/FindNepomuk.cmake	(revision 1054555)
> +++ cmake/modules/FindNepomuk.cmake	(working copy)
> @@ -22,11 +22,11 @@
>    macro_log_feature(Soprano_FOUND "Soprano" "Semantic Desktop Storing" "" FALSE "" "Soprano is needed for Nepomuk")
>  endif (NOT DEFINED Soprano_FOUND)
>  
> -if (NOT DEFINED DESKTOP_ONTOLOGIES_FOUND)
> +if (NOT DEFINED SHAREDDESKTOPONTOLOGIES_FOUND)
>    find_package(SharedDesktopOntologies)
>    include(MacroLogFeature)
> -  macro_log_feature(DESKTOP_ONTOLOGIES_FOUND "Shared desktop ontologies" "Desktop ontologies" "http://oscaf.sourceforge.net" FALSE "" "Ontologies necessary for the Nepomuk semantic desktop.")
> -endif (NOT DEFINED DESKTOP_ONTOLOGIES_FOUND)
> +  macro_log_feature(SHAREDDESKTOPONTOLOGIES_FOUND "Shared desktop ontologies" "Desktop ontologies" "http://oscaf.sourceforge.net" FALSE "" "Ontologies necessary for the Nepomuk semantic desktop.")
> +endif (NOT DEFINED SHAREDDESKTOPONTOLOGIES_FOUND)

If the sharedontologies are a requirement for nepomuk, then the
macro_log_feature call should pass TRUE instead of FALSE for the
"REQUIRED" argument. Else this will pass even if the ontologies aren't
there.
  
>  # Check for the following stuff independent from whether soprano has been found
>  # or not. This will give a better error message at the end.
> @@ -61,7 +61,7 @@
>  # easier for the user to see what was missing:
>  find_package_handle_standard_args(Nepomuk  DEFAULT_MSG 
>                                    NEPOMUK_LIBRARIES NEPOMUK_INCLUDE_DIR
> -                                  Soprano_FOUND SOPRANO_PLUGIN_RAPTORPARSER_FOUND SOPRANO_PLUGIN_REDLANDBACKEND_FOUND DESKTOP_ONTOLOGIES_FOUND
> +                                  Soprano_FOUND SOPRANO_PLUGIN_RAPTORPARSER_FOUND SOPRANO_PLUGIN_REDLANDBACKEND_FOUND SHAREDDESKTOPONTOLOGIES_FOUND
>                                    )

This would of course fail if the ontologies aren't found, but it
wouldn't be very precise in why it failed. So its better if the
macro_log_feature already fails so people are more aware which exact
requirement is missing.

Everything else looks fine, I'll leave it up to the windows developers
wether they'd like to see another default path being searched. I can't
think of any and SHARED_INSTALL_DIR should already catch most cases.

Andreas

-- 
Try to value useful qualities in one who loves you.