KDE/kdevelop/plugins/managers/cmake

[Andreas Pakulat]

On 11.02.09 00:29:38, Aleix Pol Gonzalez wrote:
> SVN commit 924530 by apol:
> 
> Fix libraries loop-up so that libraries are searched in a crossplatform way.
> Let Get_filename_component command to know the current directory. (Fixes macro_optional_add_subdirectory).
> --- trunk/KDE/kdevelop/plugins/managers/cmake/parser/cmakeprojectvisitor.cpp #924529:924530
> @@ -687,9 +687,8 @@
>      QStringList locationOptions = fpath->path()+fpath->hints();
>      QStringList path, files=fpath->filenames();
>  
> -    if(!fpath->noDefaultPath()) {
> -        locationOptions += m_defaultPaths;
> -    }
> +    if(!fpath->noDefaultPath())
> +        locationOptions += m_vars->value("CMAKE_SYSTEM_PREFIX_PATH");

I guess I should've checked where the variable is used. Sorry.
  
> @@ -737,8 +736,8 @@
>  
>      if(!flib->noDefaultPath())
>      {
> -        locationOptions += m_defaultPaths;
> -        locationOptions += m_vars->value("CMAKE_INSTALL_PREFIX").join(QString())+"/lib";
> +        foreach(const QString& s, m_vars->value("CMAKE_SYSTEM_PREFIX_PATH"))
> +            locationOptions.append(s+"/lib");

Shouldn't this also use CMAKE_LIBRARY_PATH, CMAKE_FRAMEWORK_PATH,
CMAKE_SYSTEM_FRAMEWORK_PATH and CMAKE_SYSTEM_LIBRARY_PATH? According to man
cmake those get searched for find_library as well. Similar for the
find_file, find_path and find_program support...

If you don't have time in the near future I'll either to it tonight myself
or file a bugreport so we don't forget about it.

>      }
>  
>      foreach(const QString& p, files)
> @@ -1356,16 +1355,19 @@
>  
>  int CMakeProjectVisitor::visit(const GetFilenameComponentAst *filecomp)
>  {
> +    QString dir;
> +    if(m_vars->contains("CMAKE_CURRENT_SOURCE_DIR"))
> +        dir=m_vars->value("CMAKE_CURRENT_SOURCE_DIR").first();
> +    QFileInfo fi(dir, filecomp->fileName());

This doesn't seem correct. The CMake documentation doesn't say anything
about what happens when the given filename is not absolute. But we
shouldn't always add the current source dir, because the given filename
might be an absolute path.

>      switch(filecomp->type())
>      {
>          case GetFilenameComponentAst::PATH:
>              val=fi.absolutePath();
>              break;
>          case GetFilenameComponentAst::ABSOLUTE:
> -            val=fi.canonicalFilePath();
> +            val=fi.absoluteFilePath();

This is definetly incorrect. According to the cmake documentation ABSOLUTE
returns the path without any symlinks, so the QFileInfo equivalent is
canonicalFilePath (which will resolve all symlinks and retrieve the actual
path to the file in the filesystem).

Andreas

-- 
You can rent this space for only $5 a week.