PeteyMi / openjpeg

Automatically exported from code.google.com/p/openjpeg
Other
0 stars 0 forks source link

pkg-config files are only built on Unix targets #340

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. From Linux, cross-compile OpenJPEG for Windows:
cd openjpeg
cat > toolchain.cmake <<EOF
SET(CMAKE_SYSTEM_NAME Windows)
SET(CMAKE_C_COMPILER x86_64-w64-mingw32-gcc)
SET(CMAKE_RC_COMPILER x86_64-w64-mingw32-windres)
EOF
cmake -DCMAKE_TOOLCHAIN_FILE=toolchain.cmake -DCMAKE_INSTALL_PREFIX=/tmp/ojp2 .
make
make install

What is the expected output? What do you see instead?
OpenJPEG is correctly installed in /tmp/ojp2.  However, /tmp/ojp2/lib/pkgconfig 
should contain .pc files, but does not.

What version of the product are you using? On what operating system?
SVN trunk, r2838.  The build system is Fedora 20.

Please provide any additional information below.
CMakeLists.txt only installs the pkg-config files if(UNIX).  However, these 
files are also useful when cross-compiling for Windows from Linux or Cygwin: 
other libraries that use Autotools, and which are *also* being cross-compiled, 
may need the pkg-config files in order to link with OpenJPEG.

Original issue reported on code.google.com by bgilb...@backtick.net on 19 Apr 2014 at 4:54

GoogleCodeExporter commented 9 years ago
In

 Issue 68:  openjpeg2 pkg-config support

the 'if(UNIX)' problem has been mentioned on Jul 10, 2013.

The PC files had been backported to openjpeg-2.0 on Oct 28, 2013.

The 'if(UNIX)' problem has been - and will be - ignored. You must
do it yourself.

Place a doublecross in line 320 before

#if(UNIX) 

in in line 350 before

#endif()

of CMakeLists.txt .

winfried

Original comment by szukw...@arcor.de on 19 Apr 2014 at 5:51

GoogleCodeExporter commented 9 years ago
The man pages are currently installed on Windows, even though they are not 
useful there.  It seems to me that it would be safe to handle the pkg-config 
files the same way.

As an alternative, what about changing the test to "if(UNIX OR 
CMAKE_CROSSCOMPILING)"?

Original comment by bgilb...@backtick.net on 19 Apr 2014 at 6:46

GoogleCodeExporter commented 9 years ago
 if(UNIX OR CMAKE_CROSSCOMPILING)

seems to be an alternative.

The man-pages are installed only if at least

-DBUILD_DOC:bool=on

is set. The installation directory can be set with

-DOPENJPEG_INSTALL_DOC_DIR:path="PATH"

If not it is the default path.

winfried

Original comment by szukw...@arcor.de on 20 Apr 2014 at 2:17

GoogleCodeExporter commented 9 years ago
Well, I was wrong. But now I am right.

I have changed three 'CMakeLists.txt' files and added one new option
in the top 'CMakeLists.txt'.

Now with "cmake -DBUILD_DOC:bool=on -DBUILD_PKGCONFIG_FILES:bool=on"
I get the files created and installed. On LINUX:

-- Installing: /usr/local/CMAKE_INSTALL_TEST/lib/pkgconfig/libopenjp2.pc
-- Installing: /usr/local/CMAKE_INSTALL_TEST/lib/pkgconfig/libopenjpwl.pc
-- Installing: /usr/local/CMAKE_INSTALL_TEST/lib/pkgconfig/libopenjpip.pc
-- Installing: /usr/local/CMAKE_INSTALL_TEST/lib/pkgconfig/libopenjp3d.pc

-- Installing: /usr/local/CMAKE_INSTALL_TEST/share/man/man3/libopenjp2.3
etc.

Both variables are 'off 'in the top 'CMakeLists.txt' file. Perhaps
'BUILD_PKGCONFIG_FILES' should be 'on' by default.

But this does not seem to be all. The CMAKE variable

 CMAKE_CROSSCOMPILING

must be set by

 CMAKE_SYSTEM_NAME

And according to the wiki page

 http://www.cmake.org/Wiki/CMake_Cross_Compiling

some more rules might be necessary.

winfried

Original comment by szukw...@arcor.de on 20 Apr 2014 at 11:20

Attachments:

GoogleCodeExporter commented 9 years ago
pkg-config files should be created by default on Unix, otherwise downstream 
packagers will forget to enable them.  Also, in your patch, 
-DBUILD_PKGCONFIG_FILES=on won't work unless we are also cross-compiling or on 
Unix.

What about something like the attached?  On Unix it will always create 
pkg-config files.  On other platforms it will not, unless BUILD_PKGCONFIG is 
set.

Original comment by bgilb...@backtick.net on 20 Apr 2014 at 6:58

Attachments:

GoogleCodeExporter commented 9 years ago
Your 'conditional.diff' works on Win7 and LINUX.

winfried

Original comment by szukw...@arcor.de on 21 Apr 2014 at 10:56

GoogleCodeExporter commented 9 years ago
So, this is my final patch for the problem. Will this problem ever be solved?

winfried

Original comment by szukw...@arcor.de on 22 Apr 2014 at 1:02

Attachments:

GoogleCodeExporter commented 9 years ago
From your latest patch:

+#Build '.pc' files for pkg-config
+option(BUILD_PKGCONFIG_FILES "Build and install pkg-config files" OFF)

This call should be removed, since CMAKE_DEPENDENT_OPTION() internally calls 
option() when the condition ("NOT UNIX") is met.

Original comment by bgilb...@backtick.net on 22 Apr 2014 at 3:00

GoogleCodeExporter commented 9 years ago
So, 'CMAKE_DEPENDENT_OPTION()' is a replacement for 'option()'. I have seen it
now. Even if I write 'cmake -DBUILD_PKGCONFIG_FILES:bool=off' they are built
and installed. This line can be removed: it is ineffectual.

That is bad: If I do not want that PC files are installed, they are.

winfried

Original comment by szukw...@arcor.de on 22 Apr 2014 at 10:21

GoogleCodeExporter commented 9 years ago
Okay, different approach: always respect BUILD_PKGCONFIG_FILES, but default it 
to ON on Unix and OFF on other platforms.  This is based on your previous patch.

Original comment by bgilb...@backtick.net on 23 Apr 2014 at 3:58

Attachments:

GoogleCodeExporter commented 9 years ago
Yes. Now I can use '-DBUILD_PKGCONFIG_FILES:bool=off' to suppress
.PC files on LINUX. And with '-DBUILD_PKGCONFIG_FILES:bool=on' I
can build and install them.

Your last 'openjpeg-2.x-trunk-r2845-DOC-PKGCONFIG.diff' is OK.
Thank you. 

winfried

Original comment by szukw...@arcor.de on 26 Apr 2014 at 8:08

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r2860.

Original comment by mathieu.malaterre on 28 Apr 2014 at 7:34

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 28 Apr 2014 at 7:34