|
Notes |
|
|
(0026972)
|
|
David Cole
|
|
2011-06-26 08:23
|
|
|
Mathieu, can you take a look at this...? |
|
|
|
(0026973)
|
|
Modestas Vainius
|
|
2011-06-26 08:26
|
|
|
Could you tell me which commit? |
|
|
|
(0026974)
|
|
Modestas Vainius
|
|
2011-06-26 08:32
|
|
|
|
|
(0026975)
|
|
David Cole
|
|
2011-06-26 08:37
|
|
These are the commits in 'master' related to swig:
http://cmake.org/gitweb?p=cmake.git&a=search&h=HEAD&st=commit&s=swig [^]
03809b14 was the commit that I was hoping fixed this problem when I added that first note to this bug... but then I dug in and read the description, realized it's probably not fixed yet, and then sent the note to Mathieu instead... |
|
|
|
(0026977)
|
|
David Cole
|
|
2011-06-26 08:40
|
|
|
Looks like this is caused by the fix for 0004147 |
|
|
|
(0026986)
|
|
Mathieu Malaterre
|
2011-06-27 02:22
(edited on: 2011-06-27 02:37) |
|
Modestas, the only reason why you would see an issue with cmake 2.8.5 is something like this:
SWIG_ADD_MODULE(bla python bla.i)
where bla.i contains:
%{
#include "configure.h"
%}
*and* configure.h is configured by cmake *after* the cmake command SWIG_ADD_MODULE
----
In summary you have to do:
configure_file(configure.h.in configure.h)
SWIG_ADD_MODULE(bla python bla.i)
in that order now to let cmake compute dependencies correctly.
A quick hack would be to run cmake twice...
I do not believe this is a regression, this simply require all headers for swig module to be present now.
|
|
|
|
(0026987)
|
|
Modestas Vainius
|
|
2011-06-27 04:22
|
|
For some reason you assume that those headers must present at cmake time. But that's not necessarily true. In this particular case, src/hugin_script_interface/CMakeLists.txt generates them with custom command:
...
preprocess("${HUGIN_BASE_DIR}/panodata/PanoramaData.h"
"${CMAKE_CURRENT_BINARY_DIR}/hsi_PanoramaData.h"
"${HUGIN_BASE_DIR}/panodata/image_variables.h"
)
...
SET_SOURCE_FILES_PROPERTIES(hsi.i PROPERTIES CPLUSPLUS ON)
SWIG_ADD_MODULE(hsi python hsi.i)
SWIG_LINK_LIBRARIES(hsi ${PYTHON_LIBRARIES} ${common_libs})
...
MACRO(preprocess in out depends)
ADD_CUSTOM_COMMAND(OUTPUT ${out}
COMMAND ${HSI_C_PREPROCESS} ${in} > ${out}
DEPENDS ${in} ${depends}
)
SET_SOURCE_FILES_PROPERTIES(${out} GENERATED)
ENDMACRO(preprocess)
...
>I do not believe this is a regression, this simply require all headers for swig module to be present now.
In my opinion, hugin use case is a perfect one which cmake 2.8.5 rc2 broke for no good reason. While here that custom command could be replaced by execute_process() but what if custom command was actually built from source. In my opinion, the easiest solution would be to make dependency scanning optional and OFF by default. At very least, backwards compatibility was broken in undocumented way so you need to fix that either way. |
|
|
|
(0026988)
|
|
Mathieu Malaterre
|
|
2011-06-27 04:42
|
|
The benefit of proper dependencies is HUGE for anyone doing work with SWIG. Having a report of missing files is simply fantastic.
I would suggest you fill a bug report to hugin projet to fix there source code instead of relying on broken behavior. If a dependency is needed for a particular swig module, you need to depend on it... |
|
|
|
(0026989)
|
|
Modestas Vainius
|
|
2011-06-27 05:33
|
|
hugin is NOT broken and it's NOT relying on broken behaviour. It's new SWIG code that relies on broken assumption that all files must be available at cmake time.
The whole purpose of cmake is to generate scripts for building the source, not to actually build it. cmake is a mere source configuration tool. The real problem is that new SWIG code does not support nor take into account generated files (targets) while cmake itself goes through a great pain to do this.
With that said, I'm not saying you should throw away your code. For example, the easiest way would be to add SCANDEPENDS (or the like) option to SWIG_ADD_MODULE(). Another solution would be to make swig dependency scanner ignore missing include files errors (dunno if that's possible). However, IMHO the best (and the most complicated) solution would be to rewrite dependency scanner to do its work at build (make) time like cmake itself does ('Scanning dependencies of target' stage, depend.make files). Once again, I don't know how easy the latter is to do.
Don't get me wrong. I don't really particularly care about SWIG nor hugin. However, it's sad to see cmake losing flexibility it had before. It is wrong to add not-so-proper implementation as mandatory rather than optional even if it would probably work in most cases despite its flaws. At least make it disabable. |
|
|
|
(0026992)
|
|
Brad King
|
|
2011-06-27 13:07
|
|
|
|
|
(0026993)
|
|
Brad King
|
|
2011-06-27 13:40
|
|
|
|
|
(0027007)
|
|
Modestas Vainius
|
|
2011-07-04 13:14
|
|
|
Too bad this fix didn't make to 2.8.5~rc3. When will 2.8.5 final be released? |
|
|
|
(0038990)
|
|
Brad King
|
|
2015-06-26 11:08
|
|
This regression issue was resolved by the commit linked in 0012307:0026993. Issue 0004147 trackes the actual feature request separately.
|
|
|
|
(0040084)
|
|
Robert Maynard
|
|
2016-01-04 11:51
|
|
|
Closing resolved issues that have not been updated in more than 4 months. |
|