Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Jason Pleau]

Hi

On 05/19/2016 07:10 AM, Manuel A. Fernandez Montecelo wrote:
> Hi!
> 
> 2016-05-01 15:42 Manuel A. Fernandez Montecelo:
>>
>> If it needs this change, I think that my solution is not very robust
>> and that the assumptions made don't work and can fail in other cases.
> 
> So I've been fighting with this for a while this morning and couldn't
> get it to work as originally intended.  In the end, I uploaded a new
> version of the package with the patch stripping only the SDL2_LIBRARIES
> variable.
> 
> If other vars contain whitespace in the future it will not work, but
> that's fundamentally a problem with upstream...  I submitted the patch
> for their consideration in any case.
> 
> It would be great if you can verify that this works fine in your system,
> but I am quite confident that it does so I published the new release to
> the archive already -- lest it slips through the cracks and it's delayed
> for a few more weeks.

Seems to work here, thanks for the fix and upload!

> 
> 
> Cheers.

-- 
Jason Pleau

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Manuel A. Fernandez Montecelo]

Hi!

2016-05-01 15:42 Manuel A. Fernandez Montecelo:


If it needs this change, I think that my solution is not very robust
and that the assumptions made don't work and can fail in other cases.


So I've been fighting with this for a while this morning and couldn't
get it to work as originally intended.  In the end, I uploaded a new
version of the package with the patch stripping only the SDL2_LIBRARIES
variable.

If other vars contain whitespace in the future it will not work, but
that's fundamentally a problem with upstream...  I submitted the patch
for their consideration in any case.

It would be great if you can verify that this works fine in your system,
but I am quite confident that it does so I published the new release to
the archive already -- lest it slips through the cracks and it's delayed
for a few more weeks.


Cheers.
--
Manuel A. Fernandez Montecelo 

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Manuel A. Fernandez Montecelo]

2016-04-30 2:35 GMT+01:00 Jason Pleau :
> On 04/29/2016 06:44 AM, Manuel A. Fernandez Montecelo wrote:
>> Jason, if you want to test it to see if it works fine, then I can
>> submit both to upstream's bug report for them to decide.
>
> It looks like ${prefix} is not set when defining ${stripped_prefix}. So
> for example SDL2_INCLUDE_DIRS is set to /include/SDL2.

Thanks for testing!


> I got it to work with the following:
>
> # sdl2 cmake project-config input for ./configure scripts
>
> string(STRIP "@prefix@" prefix)
> string(STRIP "@exec_prefix@" stripped_exec_prefix)
> string(STRIP "@libdir@" stripped_libdir)
> string(STRIP "@includedir@" stripped_includedir)
> string(STRIP "-L${SDL2_LIBDIR} @SDL_RLD_FLAGS@ @SDL_LIBS@"
> stripped_SDL2_LIBRARIES)
>
> set(exec_prefix "${stripped_exec_prefix}")
> set(libdir "${stripped_libdir}")
> set(SDL2_PREFIX "${stripped_prefix}")
> set(SDL2_EXEC_PREFIX "${stripped_prefix}")
> set(SDL2_LIBDIR "${stripped_libdir}")
> set(SDL2_INCLUDE_DIRS "${stripped_includedir}/SDL2")
> set(SDL2_LIBRARIES "${stripped_SDL2_LIBRARIES}")

So you only changed the first line, compared to mine?

If it needs this change, I think that my solution is not very robust
and that the assumptions made don't work and can fail in other cases.


Cheers.
-- 
Manuel A. Fernandez Montecelo 

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Jason Pleau]

Hi (removed the SDL maintainers/developers from this email, don't want
to spam..)

On 04/29/2016 06:44 AM, Manuel A. Fernandez Montecelo wrote:

... [snip] ...

> 
> At a first glance, it looks very odd for cmake to be so fussy about
> whitespace, but I guess that they have a good reason.
> 
> I've been looking into it and a more definitive/robust fix could be to
> trim the strings coming from configure, so the same problem is avoided
> in other systems or in future changes to other variables.  (Patch
> attached).
> 
> Since "string(STRIP input output)" does not return the stripped
> string, instead of using the string() function in the same spot where
> the original variable is used, one has to use an "output variable" and
> then use the variable as substitute in the original command.
> 
> So the result looks a bit ugly and duplicates the number of lines, and
> everything is a bit cumbersome, but I think that it works.
> 
> Jason, if you want to test it to see if it works fine, then I can
> submit both to upstream's bug report for them to decide.

It looks like ${prefix} is not set when defining ${stripped_prefix}. So
for example SDL2_INCLUDE_DIRS is set to /include/SDL2.

I got it to work with the following:

# sdl2 cmake project-config input for ./configure scripts

string(STRIP "@prefix@" prefix)
string(STRIP "@exec_prefix@" stripped_exec_prefix)
string(STRIP "@libdir@" stripped_libdir)
string(STRIP "@includedir@" stripped_includedir)
string(STRIP "-L${SDL2_LIBDIR} @SDL_RLD_FLAGS@ @SDL_LIBS@"
stripped_SDL2_LIBRARIES)

set(exec_prefix "${stripped_exec_prefix}")
set(libdir "${stripped_libdir}")
set(SDL2_PREFIX "${stripped_prefix}")
set(SDL2_EXEC_PREFIX "${stripped_prefix}")
set(SDL2_LIBDIR "${stripped_libdir}")
set(SDL2_INCLUDE_DIRS "${stripped_includedir}/SDL2")
set(SDL2_LIBRARIES "${stripped_SDL2_LIBRARIES}")


(patch format attached)


> 
> (If anybody wants to use the attached patch as a basis for better
> patches, feel free to do it.  Also anybody feel free to submit the
> patches upstream, but better if tested first).
> 
> 
> Cheers.
> 

Thanks !

-- 
Jason Pleau
--- a/sdl2-config.cmake.in
+++ b/sdl2-config.cmake.in
@@ -1,10 +1,15 @@
 # sdl2 cmake project-config input for ./configure scripts
 
-set(prefix "@prefix@") 
-set(exec_prefix "@exec_prefix@")
-set(libdir "@libdir@")
-set(SDL2_PREFIX "@prefix@")
-set(SDL2_EXEC_PREFIX "@prefix@")
-set(SDL2_LIBDIR "@libdir@")
-set(SDL2_INCLUDE_DIRS "@includedir@/SDL2")
-set(SDL2_LIBRARIES "-L${SDL2_LIBDIR} @SDL_RLD_FLAGS@ @SDL_LIBS@")
+string(STRIP "@prefix@" prefix)
+string(STRIP "@exec_prefix@" stripped_exec_prefix)
+string(STRIP "@libdir@" stripped_libdir)
+string(STRIP "@includedir@" stripped_includedir)
+string(STRIP "-L${SDL2_LIBDIR} @SDL_RLD_FLAGS@ @SDL_LIBS@" stripped_SDL2_LIBRARIES)
+
+set(exec_prefix "${stripped_exec_prefix}")
+set(libdir "${stripped_libdir}")
+set(SDL2_PREFIX "${stripped_prefix}")
+set(SDL2_EXEC_PREFIX "${stripped_prefix}")
+set(SDL2_LIBDIR "${stripped_libdir}")
+set(SDL2_INCLUDE_DIRS "${stripped_includedir}/SDL2")
+set(SDL2_LIBRARIES "${stripped_SDL2_LIBRARIES}")

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Manuel A. Fernandez Montecelo]

Control: forwarded -1 https://bugzilla.libsdl.org/show_bug.cgi?id=3295


2016-04-27 22:41 GMT+01:00 Manuel A. Fernandez Montecelo
:
> Hi,
>
> 2016-04-27 14:08 GMT+01:00 Gianfranco Costamagna :
>> Hi Manuel and libsdl2 developers!
>>
>>
>> How do you feel about the patch below?
>> I think we should apply it, to avoid cmake breakages in linux systems.
>
> It looks a bit odd to have to play with whitespace in this way.
>
> I'm fine with it in general, though; but since this is not Debian
> specific I prefer if it's applied (or at least blessed) by upstream
> first.  If it's failing for Jason, it should affect more people using
> other distros or the code directly from SDL repos.

This has been reported in upstream's bugzilla already (by somebody
else), with no fixes so far -- setting "forwarded" accordingly to
track the evolution upstream.


At a first glance, it looks very odd for cmake to be so fussy about
whitespace, but I guess that they have a good reason.

I've been looking into it and a more definitive/robust fix could be to
trim the strings coming from configure, so the same problem is avoided
in other systems or in future changes to other variables.  (Patch
attached).

Since "string(STRIP input output)" does not return the stripped
string, instead of using the string() function in the same spot where
the original variable is used, one has to use an "output variable" and
then use the variable as substitute in the original command.

So the result looks a bit ugly and duplicates the number of lines, and
everything is a bit cumbersome, but I think that it works.

Jason, if you want to test it to see if it works fine, then I can
submit both to upstream's bug report for them to decide.

(If anybody wants to use the attached patch as a basis for better
patches, feel free to do it.  Also anybody feel free to submit the
patches upstream, but better if tested first).


Cheers.
-- 
Manuel A. Fernandez Montecelo 


sdl2-config.cmake.in
Description: Binary data

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Manuel A. Fernandez Montecelo]

Hi,

2016-04-27 14:08 GMT+01:00 Gianfranco Costamagna :
> Hi Manuel and libsdl2 developers!
>
>
> How do you feel about the patch below?
> I think we should apply it, to avoid cmake breakages in linux systems.

It looks a bit odd to have to play with whitespace in this way.

I'm fine with it in general, though; but since this is not Debian
specific I prefer if it's applied (or at least blessed) by upstream
first.  If it's failing for Jason, it should affect more people using
other distros or the code directly from SDL repos.


Cheers.
-- 
Manuel A. Fernandez Montecelo 

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Gianfranco Costamagna]

Hi Manuel and libsdl2 developers!


How do you feel about the patch below?
I think we should apply it, to avoid cmake breakages in linux systems.

thanks for considering it,

--- a/configure.in
+++ b/configure.in
@@ -97,7 +97,7 @@
 
 if test x$have_no_cygwin = xyes; then
 BASE_CFLAGS="-mno-cygwin"
-BASE_LDFLAGS="-mno-cygwin"
+BASE_LDFLAGS=" -mno-cygwin"
 fi
 BASE_CFLAGS="$BASE_CFLAGS -I/usr/include/mingw"
 ;;
@@ -123,7 +123,7 @@
 #fi
 #done
 SDL_CFLAGS="$BASE_CFLAGS"
-SDL_LIBS="-lSDL2 $BASE_LDFLAGS"
+SDL_LIBS="-lSDL2$BASE_LDFLAGS"
 CPPFLAGS="$CPPFLAGS $EXTRA_CFLAGS"
 CFLAGS="$CFLAGS $EXTRA_CFLAGS"
 LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS"

(probably also BASE_CFLAGS needs some similar patching)

cheers,


Gianfranco


Il Domenica 24 Aprile 2016 19:22, Jason Pleau  ha scritto:
Hi Gianfranco,

On Fri, 22 Apr 2016 14:23:53 +0200 Gianfranco Costamagna
 wrote:
> Hi,
> > set(SDL2_LIBRARIES "-L${SDL2_LIBDIR}  -lSDL2 ")
> 
> I know where that space comes from:
> 
> quoting configure.in
> 
> SDL_LIBS="-lSDL2 $BASE_LDFLAGS"
> 
> 
> changing to 
> SDL_LIBS="-lSDL2$BASE_LDFLAGS"
> 
> fixes the issue, because in our case BASE_LDFLAGS is undefined.
> 
> I'm not sure why the cmake build is not even creating it, and I'm not sure how
> to best fix the issue.
> 
> I think a patch would be welcome, so the maintainers can apply it if needed :)

I attached a patch, would something like this be an acceptable solution
? If yes I think we should forward upstream

> 
> thanks for the bug report,
> 
> Gianfranco
> 

-- 
Jason Pleau

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Jason Pleau]

Hi Gianfranco,

On Fri, 22 Apr 2016 14:23:53 +0200 Gianfranco Costamagna
 wrote:
> Hi,
> > set(SDL2_LIBRARIES "-L${SDL2_LIBDIR}  -lSDL2 ")
> 
> I know where that space comes from:
> 
> quoting configure.in
> 
> SDL_LIBS="-lSDL2 $BASE_LDFLAGS"
> 
> 
> changing to 
> SDL_LIBS="-lSDL2$BASE_LDFLAGS"
> 
> fixes the issue, because in our case BASE_LDFLAGS is undefined.
> 
> I'm not sure why the cmake build is not even creating it, and I'm not sure how
> to best fix the issue.
> 
> I think a patch would be welcome, so the maintainers can apply it if needed :)

I attached a patch, would something like this be an acceptable solution
? If yes I think we should forward upstream

> 
> thanks for the bug report,
> 
> Gianfranco
> 

-- 
Jason Pleau
>From bdab9ab551bd388f3a985909a5534535899048cc Mon Sep 17 00:00:00 2001
From: Jason Pleau 
Date: Sun, 24 Apr 2016 13:13:35 -0400
Subject: [PATCH] patch to remove trailing whitespace in sdl2-config.cmake

---
 debian/changelog   |  5 +++-
 .../patches/remove_trailing_whitespace_cmake.patch | 27 ++
 debian/patches/series  |  1 +
 3 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 debian/patches/remove_trailing_whitespace_cmake.patch

diff --git a/debian/changelog b/debian/changelog
index 3c85867..59ff8a5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -22,7 +22,10 @@ libsdl2 (2.0.4+dfsg2-1) UNRELEASED; urgency=medium
   this repack can be removed in the next upstream tarball import.
   (Closes: #814276).
 
- -- Manuel A. Fernandez Montecelo   Sun, 21 Feb 2016 19:08:44 +
+  [ Jason Pleau ]
+  * patch to remove trailing whitespace in sdl2-config.cmake (Closes: #822210)
+
+ -- Jason Pleau   Sun, 24 Apr 2016 13:08:58 -0400
 
 libsdl2 (2.0.4+dfsg1-2) unstable; urgency=low
 
diff --git a/debian/patches/remove_trailing_whitespace_cmake.patch b/debian/patches/remove_trailing_whitespace_cmake.patch
new file mode 100644
index 000..c9d5f0a
--- /dev/null
+++ b/debian/patches/remove_trailing_whitespace_cmake.patch
@@ -0,0 +1,27 @@
+Description: remove the trailing whitespace in sdl2-config.cmake
+ CMake complains that there is trailing whitespace for the SDL2_LIBARIES
+ directive. This patch removes the space before $BASE_LDFLAGS in configure.in
+ which results in no extra whitespace in sdl2-config.cmake.
+Author: Jason Pleau 
+Bug-Debian: https://bugs.debian.org/822210
+
+--- a/configure.in
 b/configure.in
+@@ -97,7 +97,7 @@
+ 
+ if test x$have_no_cygwin = xyes; then
+ BASE_CFLAGS="-mno-cygwin"
+-BASE_LDFLAGS="-mno-cygwin"
++BASE_LDFLAGS=" -mno-cygwin"
+ fi
+ BASE_CFLAGS="$BASE_CFLAGS -I/usr/include/mingw"
+ ;;
+@@ -123,7 +123,7 @@
+ #fi
+ #done
+ SDL_CFLAGS="$BASE_CFLAGS"
+-SDL_LIBS="-lSDL2 $BASE_LDFLAGS"
++SDL_LIBS="-lSDL2$BASE_LDFLAGS"
+ CPPFLAGS="$CPPFLAGS $EXTRA_CFLAGS"
+ CFLAGS="$CFLAGS $EXTRA_CFLAGS"
+ LDFLAGS="$LDFLAGS $EXTRA_LDFLAGS"
diff --git a/debian/patches/series b/debian/patches/series
index 642e32f..8f1b1e9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,3 @@
+remove_trailing_whitespace_cmake.patch
 SDL2_dont_propagate_lpthread.diff
 replace-relicenced-SDL_qsort.patch
-- 
2.8.1

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Gianfranco Costamagna]

Hi,
> set(SDL2_LIBRARIES "-L${SDL2_LIBDIR}  -lSDL2 ")

I know where that space comes from:

quoting configure.in

SDL_LIBS="-lSDL2 $BASE_LDFLAGS"


changing to 
SDL_LIBS="-lSDL2$BASE_LDFLAGS"

fixes the issue, because in our case BASE_LDFLAGS is undefined.

I'm not sure why the cmake build is not even creating it, and I'm not sure how
to best fix the issue.

I think a patch would be welcome, so the maintainers can apply it if needed :)

thanks for the bug report,

Gianfranco



signature.asc
Description: OpenPGP digital signature

Bug#822210: sdl2-config.cmake: extra leading / trailing whitespace

[Jason Pleau]

Package: libsdl2-dev
Version: 2.0.4+dfsg1-2+b1
Severity: normal

Dear Maintainer,

When trying to use SDL2 in a small project with CMake, I encountered the
following error:

CMake Error at CMakeLists.txt:18 (add_executable):
  Target "run" links to item "-L/usr/lib/x86_64-linux-gnu -lSDL2 " which has
  leading or trailing whitespace.  This is now an error according to policy
  CMP0004.


This is the relevant part of my own CMakeLists.txt (line 18):

--

find_package(SDL2 REQUIRED)

--


The offending line is in /usr/lib/x86_64-linux-gnu/cmake/SDL2/sdl2-config.cmake
(could be different depending on the arch used)

Line #10:

--

set(SDL2_LIBRARIES "-L${SDL2_LIBDIR}  -lSDL2 ")

--

Removing the trailing whitespace after -lSDL2 fixes the issue for me. (There's
also an extra leading whitespace before -lSDL2)

I am not certain how to fix this properly in the sdl2-config.cmake.in file,
otherwise I would have provided a patch.

For the info, the version of CMake I am running: 3.5.1-1

Thanks !



-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.5.2-jpleau (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages libsdl2-dev depends on:
ii  libasound2-dev 1.1.0-1
ii  libdbus-1-dev  1.10.8-1
ii  libegl1-mesa-dev   11.1.3-1
ii  libgl1-mesa-dev11.1.3-1
ii  libgles2-mesa-dev  11.1.3-1
ii  libglu1-mesa-dev   9.0.0-2.1
ii  libpulse-dev   8.0-2+b2
ii  libsdl2-2.0-0  2.0.4+dfsg1-2+b1
ii  libsndio-dev   1.1.0-2
ii  libudev-dev229-4
ii  libwayland-dev 1.10.0-2
ii  libx11-dev 2:1.6.3-1
ii  libxcursor-dev 1:1.1.14-1+b1
ii  libxext-dev2:1.3.3-1
ii  libxi-dev  2:1.7.6-1
ii  libxinerama-dev2:1.1.3-1+b1
ii  libxkbcommon-dev   0.5.0-1
ii  libxrandr-dev  2:1.5.0-1
ii  libxss-dev 1:1.2.2-1
ii  libxt-dev  1:1.1.5-1
ii  libxv-dev  2:1.0.10-1+b1
ii  libxxf86vm-dev 1:1.1.4-1

libsdl2-dev recommends no packages.

libsdl2-dev suggests no packages.

-- no debconf information