Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-04-06 Thread Alexander Neundorf
On Thursday 28 March 2013, Brad King wrote:
> On 03/27/2013 05:21 PM, Alexander Neundorf wrote:
> > Maybe instead of generating code which immediately results in failure at
> > the find_package() step, how about generating code which contains a list
> > of imported targets with missing dependencies/files/directories ?
> 
> This is blowing the proposed feature way out of scope.  Currently there
> will be no error until build time.  

... when using Config.cmake files, which are only starting to get commonly 
used, and I have seen plenty of mistakes in existing Config.cmake files. 
Usually it is already hard to explain to developers the fundamental difference 
between Find-modules and Config.cmake files.
Currently when using Find-modules, you get proper error reporting from 
find_package().

> With the proposed feature the error
> will be at CMake time when there could have been one at build time, but
> not extra errors at CMake time that would not have occurred at build
> time.
> 
> >if(Xxx_IMPORTED_TARGETS_WITH_MISSING_STUFF)
> >
> >   set(Xxx_${comp}_FOUND FALSE)
> >
> >endif()
> 
> A package config file and consuming projects should not be expected to
> adapt to broken installations of packages to use the "non-broken" parts.
> If an include dir is missing who knows what else is wrong.
> 
> This feature should be about making things simpler by erroring out early
> when we know the build is likely to break because something is missing.
> Your proposal will make it more complicated by trying to adapt to the
> breakage.
> 
> BTW, take a look at this problem:
> 
>  http://www.cmake.org/Bug/view.php?id=14041
> 
> triggered by some of the existing checks for missing files.


Yes, but this is something we have to take care of (thanks for doing that), if 
we want developers to like Config.cmake files.
What I like about Find-modules is that they give reliable information about 
the actual state of the system. If a Find-module tells me jpeglih.h can be 
found in some directory, it actually exists there, if it tells me the path to 
a libjpeg.so, I can rely on that this file actually exists.

If find_package() "only" reads some cmake script file, i.e. a Config.cmake 
file, this doesn't give me those guarantees as long as this file does not 
verify that the information it provides is actually correct, by checking that 
the files and directories it reports to me actually exist.
This is a step backwards compared to Find-modules.
Config.cmake files are much more powerful than Find-modules, but they are also 
a lot more complex, at least for simple cases.
We should not add more disadvantages to them (like not actually being a 
reliable source of information).

I had enough of bad experiences with libtool la files, I want Config.cmake to 
be much much better.
For me, this includes that if we do not use find_library()/find_path() to 
actually introspect the system, we need to make sure that the files we refer 
to actually do exist, so that the contents of the Config.cmake files will 
hopefully never be the cause for build debugging by developers.

Alex
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-28 Thread Brad King
On 03/27/2013 05:21 PM, Alexander Neundorf wrote:
> Maybe instead of generating code which immediately results in failure at the 
> find_package() step, how about generating code which contains a list of 
> imported targets with missing dependencies/files/directories ?

This is blowing the proposed feature way out of scope.  Currently there
will be no error until build time.  With the proposed feature the error
will be at CMake time when there could have been one at build time, but
not extra errors at CMake time that would not have occurred at build
time.

>if(Xxx_IMPORTED_TARGETS_WITH_MISSING_STUFF)
>   set(Xxx_${comp}_FOUND FALSE)
>endif()

A package config file and consuming projects should not be expected to
adapt to broken installations of packages to use the "non-broken" parts.
If an include dir is missing who knows what else is wrong.

This feature should be about making things simpler by erroring out early
when we know the build is likely to break because something is missing.
Your proposal will make it more complicated by trying to adapt to the
breakage.

BTW, take a look at this problem:

 http://www.cmake.org/Bug/view.php?id=14041

triggered by some of the existing checks for missing files.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-27 Thread Alexander Neundorf
On Wednesday 27 March 2013, Brad King wrote:
> On 03/26/2013 04:07 PM, Alexander Neundorf wrote:
> > there is a difference here in that the include directory properties are a
> > new feature, so this wouldn't break existing code.
> > It would force developers who make use of the new feature to split
> > exported targets into individual components and support components
> > properly in their Config.cmake files.
> 
> I don't want to force anyone to do that.  It is perfectly reasonable
> to have a monolithic targets file.

>From readme.txt:
"A package can provide sub-components.
Those components can be listed after the COMPONENTS (or REQUIRED)
or OPTIONAL_COMPONENTS keywords.  The set of all listed components will be
specified in a Xxx_FIND_COMPONENTS variable.
For each package-specific component, say Yyy, a variable Xxx_FIND_REQUIRED_Yyy
will be set to true if it listed after COMPONENTS and it will be set to false
if it was listed after OPTIONAL_COMPONENTS.
Using those variables a FindXxx.cmake module and also a XxxConfig.cmake 
package configuration file can determine whether and which components have 
been requested, and whether they were requested as required or as optional.
For each of the requested components a Xxx_Yyy_FOUND variable should be set
accordingly.
The per-package Xxx_FOUND variable should be only set to true if all requested
required components have been found. A missing optional component should not
keep the Xxx_FOUND variable from being set to true."

We should try to make that possible.

find_package(Foo REQUIRED COMPONENTS Blub)

If this does not error out, and if the result of that is Foo_FOUND = TRUE and 
Foo_Blub_FOUND = TRUE, a cmake user should be able to expect that component 
Blub of Foo works and can be used.

Maybe instead of generating code which immediately results in failure at the 
find_package() step, how about generating code which contains a list of 
imported targets with missing dependencies/files/directories ?
This can then be evaluated in the Config.cmake file and the per-component 
_FOUND variables can be set.
Does otherwise the author of a Config.cmake file have to write cmake code to 
query the imported targets for their include interface properties, and check  
whether the referenced directories and targets exist ?

With such a variable, the code in a Config.cmake file could look like this:

foreach(comp ${Xxx_FIND_COMPONENTS})
   include(${CMAKE_CURRENT_LIST_DIR}/${comp}Targets.cmake OPTIONAL
   RESULT_VARIABLE fileIncluded)

   set(Xxx_${comp}_FOUND TRUE)
   if(NOT fileIncluded)
  set(Xxx_${comp}_FOUND FALSE)
   endif()

   # now the suggested variable:
   if(Xxx_IMPORTED_TARGETS_WITH_MISSING_STUFF)
  set(Xxx_${comp}_FOUND FALSE)
   endif()
endforeach()


Alex
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-27 Thread Brad King
On 03/26/2013 04:07 PM, Alexander Neundorf wrote:
> there is a difference here in that the include directory properties are a new 
> feature, so this wouldn't break existing code.
> It would force developers who make use of the new feature to split exported 
> targets into individual components and support components properly in their 
> Config.cmake files.

I don't want to force anyone to do that.  It is perfectly reasonable
to have a monolithic targets file.

> I'm ok with it if the error occurs at cmake time, I'd be even more ok if a 
> successful find_package() would continue to tell me that everything I need 
> has 
> been found.
> This is the case with find-modules, I see this as a good thing.

find_package doesn't know everything you need because only after it
returns does the code start referencing some subset of the targets.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-27 Thread Stephen Kelly
Brad King wrote:
>> Additionally I've added a commit to
>> my clone in the error-on-exported-missing-include-dir branch, which
>> introduces a policy. That commit doesn't really have to be in CMake
>> 2.8.11. Currently the test doesn't pass, and I'm not sure why. The
>> cmake_policy() line in the test doesn't seem to have an effect.

> The switch on GetPolicyStatus does not use our standard convention.
> Usually we have WARN fall through to OLD and have NEW as its own case.
> Currently I think it is right but hard to follow.  Can you refactor?
> 
> I don't see at a glance why the test would fail.  You'll have to dig.

I solved this by recording the policy status when the target is created. The 
policy stack seems to be popped after the configuration step is done, so my 
new setting was being lost.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Alexander Neundorf
On Tuesday 26 March 2013, Brad King wrote:
> On 03/26/2013 03:24 PM, Alexander Neundorf wrote:
> > Probably I'm missing something obvious... can't the include-directories
> > be checked by generating code into the exports-file ?
> 
> No.  We went over that for a similar case in another thread recently:
> 
>  http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/6106/focu
> s=6138
> 
> The imported targets should not produce errors if they are not used.

there is a difference here in that the include directory properties are a new 
feature, so this wouldn't break existing code.
It would force developers who make use of the new feature to split exported 
targets into individual components and support components properly in their 
Config.cmake files.

I'm ok with it if the error occurs at cmake time, I'd be even more ok if a 
successful find_package() would continue to tell me that everything I need has 
been found.
This is the case with find-modules, I see this as a good thing.

Alex
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Brad King
On 03/26/2013 03:38 PM, Stephen Kelly wrote:
> Brad King wrote:
>> Alternatively one could solve this case with no additional CMake feature
>> by hiding the needed paths in $<1:...> generator expressions.  That is not
>> as explicit, but I would be okay with that as the recommended solution
>> over an awkwardly named property.
> 
> We could make it more explicit by adding a generator expression which 
> behaves the same as $<1:...> (like BUILD_INTERFACE already). This is also 
> more exact about what to exclude the check for:
> 
>  INTERFACE_INCLUDE_DIRECTORIES 
>"/foo/bar;$;/other/dir"

While this is not an edge case I think it is obscure enough that explaining
why the $ expression exists will be too much for now.
Let's just leave it as-is and we can add other solutions if $<1:...> is too
problematic in practice.

> Unrelated to this specific issue, I've added another commit to stage with 
> another check (for relative paths). Additionally I've added a commit to my 
> clone in the error-on-exported-missing-include-dir branch, which introduces 
> a policy. That commit doesn't really have to be in CMake 2.8.11. Currently 
> the test doesn't pass, and I'm not sure why. The cmake_policy() line in the 
> test doesn't seem to have an effect.

Yes, after 2.8.11.

This line in the test:

 +cmake_minimum_required(VERSION 2.8)

should not be needed because the test CMakeLists.txt has them.

The switch on GetPolicyStatus does not use our standard convention.
Usually we have WARN fall through to OLD and have NEW as its own case.
Currently I think it is right but hard to follow.  Can you refactor?

I don't see at a glance why the test would fail.  You'll have to dig.

Thanks,
-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Stephen Kelly
Brad King wrote:

> On 03/26/2013 02:53 PM, Robert Maynard wrote:
>> My concern with this feature is the use case of a directory not existing
>> at configure time, but which will exist at linking time. Primarily this
>> would occur when an imported library is the result of an
>> add_custom_command call which creates a new directory. While this might
>> be an edge case, I was wondering if we should have a way to skip the
>> verification on a target.

> Alternatively one could solve this case with no additional CMake feature
> by hiding the needed paths in $<1:...> generator expressions.  That is not
> as explicit, but I would be okay with that as the recommended solution
> over an awkwardly named property.

We could make it more explicit by adding a generator expression which 
behaves the same as $<1:...> (like BUILD_INTERFACE already). This is also 
more exact about what to exclude the check for:

 INTERFACE_INCLUDE_DIRECTORIES 
   "/foo/bar;$;/other/dir"


Unrelated to this specific issue, I've added another commit to stage with 
another check (for relative paths). Additionally I've added a commit to my 
clone in the error-on-exported-missing-include-dir branch, which introduces 
a policy. That commit doesn't really have to be in CMake 2.8.11. Currently 
the test doesn't pass, and I'm not sure why. The cmake_policy() line in the 
test doesn't seem to have an effect.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Brad King
On 03/26/2013 03:24 PM, Alexander Neundorf wrote:
> Probably I'm missing something obvious... can't the include-directories be 
> checked by generating code into the exports-file ?

No.  We went over that for a similar case in another thread recently:

 
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/6106/focus=6138

The imported targets should not produce errors if they are not used.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Alexander Neundorf
On Tuesday 26 March 2013, Brad King wrote:
> On 03/26/2013 02:53 PM, Robert Maynard wrote:
> > My concern with this feature is the use case of a directory not existing
> > at configure time, but which will exist at linking time. Primarily this
> > would occur when an imported library is the result of an
> > add_custom_command call which creates a new directory. While this might
> > be an edge case, I was wondering if we should have a way to skip the
> > verification on a target.
> 
> It is not an edge case.  The add_dependencies command explicitly supports
> adding "dependencies" of an imported target that then transitively become
> dependencies of real targets that use it.  The idea is to have a hand-made
> imported target that references files that will be produced at build time
> by something like ExternalProject.  The hand-made imported target would do
> 
>  add_dependencies(imp_tgt ep_tgt)
> 
> to get build-time dependencies right.  However, that will also mean that
> if one defines INTERFACE_INCLUDE_DIRECTORIES on imp_tgt the paths may not
> exist during CMake configuration.
> 
> I suggest that to support this case we allow the hand-made imported target
> to request that the include interface not be validated:
> 
>  set_property(TARGET imp_tgt PROPERTY
>INTERFACE_INCLUDE_DIRECTORIES_ALLOW_MISSING 1)
> 
> Ideas for a better name?
> 
> Alternatively one could solve this case with no additional CMake feature
> by hiding the needed paths in $<1:...> generator expressions.  That is not
> as explicit, but I would be okay with that as the recommended solution over
> an awkwardly named property.

Probably I'm missing something obvious... can't the include-directories be 
checked by generating code into the exports-file ?
That's done for the other referenced files too.

Alex
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Brad King
On 03/26/2013 02:53 PM, Robert Maynard wrote:
> My concern with this feature is the use case of a directory not existing at 
> configure time, but which will exist at linking time. Primarily this would 
> occur when an imported library is the result of an add_custom_command call 
> which creates a new directory. While this might be an edge case, I was
> wondering if we should have a way to skip the verification on a target. 

It is not an edge case.  The add_dependencies command explicitly supports
adding "dependencies" of an imported target that then transitively become
dependencies of real targets that use it.  The idea is to have a hand-made
imported target that references files that will be produced at build time
by something like ExternalProject.  The hand-made imported target would do

 add_dependencies(imp_tgt ep_tgt)

to get build-time dependencies right.  However, that will also mean that
if one defines INTERFACE_INCLUDE_DIRECTORIES on imp_tgt the paths may not
exist during CMake configuration.

I suggest that to support this case we allow the hand-made imported target
to request that the include interface not be validated:

 set_property(TARGET imp_tgt PROPERTY
   INTERFACE_INCLUDE_DIRECTORIES_ALLOW_MISSING 1)

Ideas for a better name?

Alternatively one could solve this case with no additional CMake feature
by hiding the needed paths in $<1:...> generator expressions.  That is not
as explicit, but I would be okay with that as the recommended solution over
an awkwardly named property.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-26 Thread Robert Maynard
Hi Stephen,

My concern with this feature is the use case of a directory not existing at
configure time, but which will exist at linking time. Primarily this would
occur when an imported library is the result of an add_custom_command call
which creates a new directory. While this might be an edge case, I was
wondering if we should have a way to skip the verification on a target.


On Mon, Mar 25, 2013 at 3:11 PM, Stephen Kelly  wrote:

> Brad King wrote:
>
> > On 03/25/2013 01:41 PM, Brad King wrote:
> >>> So will I squash these commits together and push to next?
> >>
> >> Yes, if this is intended for 2.8.11-rc2.
> >
> > I just looked at the change merged to next.  Can you please add
> > test cases for the error messages?
>
> Done now, thanks,
>
> Steve.
>
>
> --
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
>



-- 
Robert Maynard
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Stephen Kelly
Brad King wrote:

> On 03/25/2013 01:41 PM, Brad King wrote:
>>> So will I squash these commits together and push to next?
>> 
>> Yes, if this is intended for 2.8.11-rc2.
> 
> I just looked at the change merged to next.  Can you please add
> test cases for the error messages?

Done now, thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Brad King
On 03/25/2013 01:41 PM, Brad King wrote:
>> So will I squash these commits together and push to next?
> 
> Yes, if this is intended for 2.8.11-rc2.

I just looked at the change merged to next.  Can you please add
test cases for the error messages?

Thanks,
-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Brad King
On 03/25/2013 01:31 PM, Stephen Kelly wrote:
> it is right to error on "${CMAKE_SOURCE_DIR}/../bar/bat"

Yes because if the source dir doesn't exist later the path will
not be resolvable.

> So will I squash these commits together and push to next?

Yes, if this is intended for 2.8.11-rc2.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Stephen Kelly
Stephen Kelly wrote:

>> There are other places that we check for paths under the source/build
>> trees, such as:
>>
>>
> 
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmLocalGenerator.cxx;hb=v2.8.10.2#l2485
>>
> 
> Thanks, I'll look into that a bit later.
> 

I looked into this a bit and wrote an alternative patch and pushed to my 
clone.

A problem it still has is that if I create a build dir in the subdir, and 
then put "${CMAKE_BINARY_DIR}/../bar/bat" in the interface, it complains 
about the path being in the build dir. The '..' was not collapsed to see 
that it is in the source dir. 

I'm not certain whether this is a good or bad thing. Certainly the path is 
specified relative to the build dir, so it is good to error on it, just like 
it is right to error on "${CMAKE_SOURCE_DIR}/../bar/bat", even though the 
collapsed path is not in the source dir.

So will I squash these commits together and push to next?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Stephen Kelly
Brad King wrote:

> On 03/25/2013 05:57 AM, Stephen Kelly wrote:
>> Stephen Kelly wrote:
>>> I've pushed the error-on-exported-missing-include-dir branch to my
>>> clone.
> 
> Why does the test need CMAKE_OMIT_INCLUDES_CHECK?

Without it I get 

  Target "testSharedLibRequired" INTERFACE_INCLUDE_DIRECTORIES property
  contains path 
"/home/stephen/dev/src/cmake/Tests/ExportImport/build/Export"
  which is prefixed in the build directory.

because I was lazy when adding the test for it:

 set_property(TARGET testSharedLibRequired APPEND PROPERTY
   INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_BINARY_DIR}"
 "${CMAKE_CURRENT_SOURCE_DIR}" 
 )

I'm fine with declaring such lazyness out of scope, installing the files 
needed from those dirs instead, and removing the OMIT variable and 
functionality.

> 
>> I'm also not sure if the check for '..' in the path is the right way to
>> check this, whether symlinks need to be considered, what the result on
>> Windows would be if the two directories are on different drives, and
>> whether windows virtual drives need to be considered.
> 
> There are other places that we check for paths under the source/build
> trees, such as:
> 
>  
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmLocalGenerator.cxx;hb=v2.8.10.2#l2485
> 

Thanks, I'll look into that a bit later.

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Brad King
On 03/25/2013 05:57 AM, Stephen Kelly wrote:
> Stephen Kelly wrote:
>> I've pushed the error-on-exported-missing-include-dir branch to my clone.

Why does the test need CMAKE_OMIT_INCLUDES_CHECK?

> I'm also not sure if the check for '..' in the path is the right way to 
> check this, whether symlinks need to be considered, what the result on 
> Windows would be if the two directories are on different drives, and whether 
> windows virtual drives need to be considered.

There are other places that we check for paths under the source/build
trees, such as:

 
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmLocalGenerator.cxx;hb=v2.8.10.2#l2485

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Stephen Kelly
Stephen Kelly wrote:

> Stephen Kelly wrote:
>>> We already have similar errors in the exported "targets" files for
>>> things like missing library files.  The wording says something about
>>> possible missing or broken packages.
>> 
>> Right, I'll see if I can do something similar over the next few days.
> 
> I've pushed the error-on-exported-missing-include-dir branch to my clone.

I'm also not sure if the check for '..' in the path is the right way to 
check this, whether symlinks need to be considered, what the result on 
Windows would be if the two directories are on different drives, and whether 
windows virtual drives need to be considered.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES

2013-03-25 Thread Stephen Kelly
Stephen Kelly wrote:
>> We already have similar errors in the exported "targets" files for
>> things like missing library files.  The wording says something about
>> possible missing or broken packages.
> 
> Right, I'll see if I can do something similar over the next few days.

I've pushed the error-on-exported-missing-include-dir branch to my clone. I 
added the check of the include interface with a way to turn it off, because 
the ExportImport test semi-legitimately needs that. The variable for that 
probably needs a better name and does need documentation.

Any comments to that?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES (Was: CMake usage requirements in KDE Frameworks)

2013-03-22 Thread Stephen Kelly
Brad King wrote:

> On 03/22/2013 07:14 AM, Stephen Kelly wrote:
>> Would it be reasonable to issue an error at install(EXPORT)-time if the
>> INTERFACE_INCLUDE_DIRECTORIES of a target contains paths in the source
>> dir or binary dir, if the install prefix is not inside one of those
>> itself?
> 
> Yes, though it won't be able to work for all cases because some paths
> may have generator expressions which won't be evaluated until the
> consumer loads them.

Yes, exactly.

> 
>> Also, in cmTarget::GetIncludeDirectories, if a path in a
>> INTERFACE_INCLUDE_DIRECTORIES entry of an IMPORTED dependent does not
>> exist, that could be made an error too. This would sort of be showing the
>> error in the wrong place though (to the person using the package, rather
>> than the one creating it).
> 
> We already have similar errors in the exported "targets" files for
> things like missing library files.  The wording says something about
> possible missing or broken packages.

Right, I'll see if I can do something similar over the next few days.

Thanks,

Steve.



--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Issuing errors for faulty INTERFACE_INCLUDE_DIRECTORIES (Was: CMake usage requirements in KDE Frameworks)

2013-03-22 Thread Brad King
On 03/22/2013 07:14 AM, Stephen Kelly wrote:
> Would it be reasonable to issue an error at install(EXPORT)-time if the 
> INTERFACE_INCLUDE_DIRECTORIES of a target contains paths in the source dir 
> or binary dir, if the install prefix is not inside one of those itself?

Yes, though it won't be able to work for all cases because some paths
may have generator expressions which won't be evaluated until the
consumer loads them.

> Also, in cmTarget::GetIncludeDirectories, if a path in a 
> INTERFACE_INCLUDE_DIRECTORIES entry of an IMPORTED dependent does not exist, 
> that could be made an error too. This would sort of be showing the error in 
> the wrong place though (to the person using the package, rather than the one 
> creating it).

We already have similar errors in the exported "targets" files for
things like missing library files.  The wording says something about
possible missing or broken packages.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers