Bug#892861: Fwd: Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Andrew Caudwell]

-- Forwarded message --
From: Andrew Caudwell <acaudw...@gmail.com>
Date: Thu, Mar 29, 2018 at 11:55 AM
Subject: Re: Bug#892861: libglm-dev: removal of default type initialization
breaking packages
To: Guus Sliepen <g...@debian.org>



On Thu, Mar 29, 2018 at 9:12 AM, Guus Sliepen <g...@debian.org> wrote:

> tags 892861 - wontfix
> thanks
>
> On Wed, Mar 28, 2018 at 01:19:44PM +1300, Andrew Caudwell wrote:
>
> > Upstream has implemented my suggestion to re-add default initialization
> as
> > opt-in via a new define:
> >
> > https://github.com/g-truc/glm/issues/735
> > https://github.com/g-truc/glm/commit/8390a77b3a278b15259e5ca
> 6e67f7e41badc457b
> >
> > Could you apply the commit as a patch so maintainers can then define
> > GLM_FORCE_CTOR_INIT and avoid having to modifying code?
>
> Sure. However, I'm running into problems trying to apply the patch: it
> doesn't apply cleanly on 0.9.9~a2, and if I just package the latest
> revision from git, then I am getting internal compiler errors from GCC.
> I can still compile 0.9.9~a2 without problems, so I will try to see if I
> can just adapt the commit which adds GLM_FORCE_CTOR_INIT to 0.9.9~a2.
>
> Here's a patch I created. The main issue is they added a few macros before
the constructors that we need to remove:

git format-patch -1 8390a77b3a278b15259e5ca6e67f7e41badc457b
perl -npe "s/GLM_CONSTEXPR_(CTOR_)?CXX14 //g"
0001-Added-GLM_FORCE_CTOR_INIT-735-740.patch
> 0001-Added-GLM_FORCE_CTOR_INIT-735-740-fixed.patch

I also had to remove some expected white space from type_mat3x2.inl and
type_mat4x2.inl.

Tested it against the 0.9.9-a2 tag with GLM_FORCE_CTOR_INIT defined and it
fixes my issue.



> > Let me know as then I can then avoid having to embed the current release
> in
> > my software.
>
> Yeah, that would be less than optimal.
>
> --
> Met vriendelijke groet / with kind regards,
>   Guus Sliepen <g...@debian.org>
>
From 8390a77b3a278b15259e5ca6e67f7e41badc457b Mon Sep 17 00:00:00 2001
From: Christophe Riccio <christophe.ric...@unity3d.com>
Date: Tue, 27 Mar 2018 18:23:37 +0200
Subject: [PATCH] Added GLM_FORCE_CTOR_INIT #735 #740

---
 glm/detail/setup.hpp| 11 +++
 glm/detail/type_mat2x2.hpp  |  2 +-
 glm/detail/type_mat2x2.inl  |  9 +++--
 glm/detail/type_mat2x3.hpp  |  2 +-
 glm/detail/type_mat2x3.inl  |  9 +++--
 glm/detail/type_mat2x4.hpp  |  2 +-
 glm/detail/type_mat2x4.inl  |  9 +++--
 glm/detail/type_mat3x2.hpp  |  2 +-
 glm/detail/type_mat3x2.inl  | 10 --
 glm/detail/type_mat3x3.hpp  |  2 +-
 glm/detail/type_mat3x3.inl  | 10 --
 glm/detail/type_mat3x4.hpp  |  2 +-
 glm/detail/type_mat3x4.inl  | 10 --
 glm/detail/type_mat4x2.hpp  |  2 +-
 glm/detail/type_mat4x2.inl  | 11 +--
 glm/detail/type_mat4x3.hpp  |  2 +-
 glm/detail/type_mat4x3.inl  | 11 +--
 glm/detail/type_mat4x4.hpp  |  2 +-
 glm/detail/type_mat4x4.inl  | 11 +--
 glm/detail/type_vec1.inl|  7 +--
 glm/detail/type_vec2.hpp|  2 +-
 glm/detail/type_vec2.inl|  7 +--
 glm/detail/type_vec3.hpp|  2 +-
 glm/detail/type_vec3.inl|  7 +--
 glm/detail/type_vec4.hpp|  2 +-
 glm/detail/type_vec4.inl|  7 +--
 glm/ext/vec1.hpp|  2 +-
 glm/gtc/quaternion.hpp  |  2 +-
 glm/gtc/quaternion.inl  |  5 -
 glm/gtx/dual_quaternion.hpp |  2 +-
 glm/gtx/dual_quaternion.inl |  6 +-
 31 files changed, 127 insertions(+), 43 deletions(-)

diff --git a/glm/detail/setup.hpp b/glm/detail/setup.hpp
index 050978c6..d6e7ba1a 100644
--- a/glm/detail/setup.hpp
+++ b/glm/detail/setup.hpp
@@ -720,8 +720,19 @@
 
 #if GLM_HAS_DEFAULTED_FUNCTIONS
 #	define GLM_DEFAULT = default
+
+#	ifdef GLM_FORCE_NO_CTOR_INIT
+#		undef GLM_FORCE_CTOR_INIT
+#	endif
+
+#	ifdef GLM_FORCE_CTOR_INIT
+#		define GLM_DEFAULT_CTOR
+#	else
+#		define GLM_DEFAULT_CTOR = default
+#	endif
 #else
 #	define GLM_DEFAULT
+#	define GLM_DEFAULT_CTOR
 #endif
 
 #if GLM_HAS_CONSTEXPR || GLM_HAS_CONSTEXPR_PARTIAL
diff --git a/glm/detail/type_mat2x2.hpp b/glm/detail/type_mat2x2.hpp
index ed9b782f..47e28cba 100644
--- a/glm/detail/type_mat2x2.hpp
+++ b/glm/detail/type_mat2x2.hpp
@@ -34,7 +34,7 @@ namespace glm
 
 		// -- Constructors --
 
-		GLM_FUNC_DECL mat() GLM_DEFAULT;
+		GLM_FUNC_DECL mat() GLM_DEFAULT_CTOR;
 		GLM_FUNC_DECL mat(mat<2, 2, T, Q> const& m) GLM_DEFAULT;
 		template
 		GLM_FUNC_DECL mat(mat<2, 2, T, P> const& m);
diff --git a/glm/detail/type_mat2x2.inl b/glm/detail/type_mat2x2.inl
index 3c7fae56..72971d8c 100644
--- a/glm/detail/type_mat2x2.inl
+++ b/glm/detail/type_mat2x2.inl
@@ -7,10 +7,15 @@ namespace glm
 {
 	// -- Constructors --
 
-#	if !GLM_HAS_DEFAULTED_FUNCTIONS
+#	if !GLM_HAS_DEFAULTED_FUNCTIONS || defined(GLM_FORCE_CTOR_INIT)
 		template
 		GLM_FUNC_QUALIFIER mat<2, 2, T, Q>

Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Guus Sliepen]

tags 892861 - wontfix
thanks

On Wed, Mar 28, 2018 at 01:19:44PM +1300, Andrew Caudwell wrote:

> Upstream has implemented my suggestion to re-add default initialization as
> opt-in via a new define:
> 
> https://github.com/g-truc/glm/issues/735
> https://github.com/g-truc/glm/commit/8390a77b3a278b15259e5ca6e67f7e41badc457b
> 
> Could you apply the commit as a patch so maintainers can then define
> GLM_FORCE_CTOR_INIT and avoid having to modifying code?

Sure. However, I'm running into problems trying to apply the patch: it
doesn't apply cleanly on 0.9.9~a2, and if I just package the latest
revision from git, then I am getting internal compiler errors from GCC.
I can still compile 0.9.9~a2 without problems, so I will try to see if I
can just adapt the commit which adds GLM_FORCE_CTOR_INIT to 0.9.9~a2.

> Let me know as then I can then avoid having to embed the current release in
> my software.

Yeah, that would be less than optimal.

-- 
Met vriendelijke groet / with kind regards,
  Guus Sliepen 


signature.asc
Description: PGP signature

Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Andrew Caudwell]

Upstream has implemented my suggestion to re-add default initialization as
opt-in via a new define:

https://github.com/g-truc/glm/issues/735
https://github.com/g-truc/glm/commit/8390a77b3a278b15259e5ca6e67f7e41badc457b

Could you apply the commit as a patch so maintainers can then define
GLM_FORCE_CTOR_INIT and avoid having to modifying code?

Let me know as then I can then avoid having to embed the current release in
my software.

Thanks


On Fri, Mar 23, 2018 at 3:07 PM, Andrew Caudwell 
wrote:

> Hi,
>
> On Sat, Mar 17, 2018 at 9:31 AM, Guus Sliepen  wrote:
>
>> tags 892861 + wontfix
>> thanks
>>
>> On Wed, Mar 14, 2018 at 10:48:33AM +1300, Andrew Caudwell wrote:
>>
>> > The packaged version of GLM, 0.9.9~a2 is an alpha (the current release
>> is still
>> > 0.9.8.5) and removes the default initialization of vector, matrix and
>> > quaternion types. Because of this code written against any earlier
>> versions of
>> > GLM may now have uninitialized value bugs introduced by this change
>> (e.g. where
>> > GLM types are member variables of a class) or now behave differently
>> (mat4()
>> > previously gave you an identity matrix, now this gives you a zero'd
>> matrix).
>> > Several issues have been raised upstream (including by myself) to re-add
>> > initialization or at least make it optional.
>> > Additionally the requirement in this version to define
>> GLM_ENABLE_EXPERIMENTAL
>> [...]
>> > to use simple functions like length2() has broken multiple packages. I
>> have put
>> > off fixing this since making it compile just exposes the user to the
>> > uninitialized value bugs. Unfortunately this has now meant my gource and
>> > logstalgia debian packages have been removed from debian since they
>> don't
>> > complile with this GLM version.
>>
>> I believe these are intentional changes by the author of GLM. I expect
>> that GLM 1.0.0 will be released before the next release of Debian, and
>> any packages that depend on GLM should instead be fixed to handle the
>> new behavior.
>>
>
> The upstream author hasn't commented on these issues yet so there's no
> reason to assume they will leave things in the current state without at
> least providing at least a work around to the initialization issue.
>
>>
>> Unless I am mistaken, projects depending on GLM can just #define
>> GLM_ENABLE_EXPERIMENTAL and provide explicit default initializers, which
>> will be backwards compatible with older versions of GLM.
>>
>
> Fixing the default initializers issue requires a thorough code review and
> probably debugging with valgrind to find these cases (-Wuninitialized won't
> find these) which I think is an unreasonable burden on maintainers.
>
> Other distros such as Redhat, Gentoo and Arch Linux have continued to
> package the current release 0.9.8.5 with a fix for the GCC 7.3 issue (which
> only requires changing an == to >=):
>
> https://git.archlinux.org/svntogit/community.git/tree/
> trunk/PKGBUILD?h=packages/glm
>
> Ubuntu 18.04 has imported the current Debian unstable libglm-dev package
> which has made resolving this issue more urgent in my view.
>
> My current work around is to embed the patched version of 0.9.8.5 and
> provide a --with-glm option for distros that have a compatible version of
> the library.
>
> Ideally I would like to be able to use the Debian libglm-dev package so I
> hope you reconsider.
>
> Cheers
>
> Andrew
>
>
>
>>
>> --
>> Met vriendelijke groet / with kind regards,
>>   Guus Sliepen 
>>
>
>

Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Andrew Caudwell]

Hi,

On Sat, Mar 17, 2018 at 9:31 AM, Guus Sliepen  wrote:

> tags 892861 + wontfix
> thanks
>
> On Wed, Mar 14, 2018 at 10:48:33AM +1300, Andrew Caudwell wrote:
>
> > The packaged version of GLM, 0.9.9~a2 is an alpha (the current release
> is still
> > 0.9.8.5) and removes the default initialization of vector, matrix and
> > quaternion types. Because of this code written against any earlier
> versions of
> > GLM may now have uninitialized value bugs introduced by this change
> (e.g. where
> > GLM types are member variables of a class) or now behave differently
> (mat4()
> > previously gave you an identity matrix, now this gives you a zero'd
> matrix).
> > Several issues have been raised upstream (including by myself) to re-add
> > initialization or at least make it optional.
> > Additionally the requirement in this version to define
> GLM_ENABLE_EXPERIMENTAL
> [...]
> > to use simple functions like length2() has broken multiple packages. I
> have put
> > off fixing this since making it compile just exposes the user to the
> > uninitialized value bugs. Unfortunately this has now meant my gource and
> > logstalgia debian packages have been removed from debian since they don't
> > complile with this GLM version.
>
> I believe these are intentional changes by the author of GLM. I expect
> that GLM 1.0.0 will be released before the next release of Debian, and
> any packages that depend on GLM should instead be fixed to handle the
> new behavior.
>

The upstream author hasn't commented on these issues yet so there's no
reason to assume they will leave things in the current state without at
least providing at least a work around to the initialization issue.

>
> Unless I am mistaken, projects depending on GLM can just #define
> GLM_ENABLE_EXPERIMENTAL and provide explicit default initializers, which
> will be backwards compatible with older versions of GLM.
>

Fixing the default initializers issue requires a thorough code review and
probably debugging with valgrind to find these cases (-Wuninitialized won't
find these) which I think is an unreasonable burden on maintainers.

Other distros such as Redhat, Gentoo and Arch Linux have continued to
package the current release 0.9.8.5 with a fix for the GCC 7.3 issue (which
only requires changing an == to >=):

https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/glm

Ubuntu 18.04 has imported the current Debian unstable libglm-dev package
which has made resolving this issue more urgent in my view.

My current work around is to embed the patched version of 0.9.8.5 and
provide a --with-glm option for distros that have a compatible version of
the library.

Ideally I would like to be able to use the Debian libglm-dev package so I
hope you reconsider.

Cheers

Andrew



>
> --
> Met vriendelijke groet / with kind regards,
>   Guus Sliepen 
>

Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Guus Sliepen]

tags 892861 + wontfix
thanks

On Wed, Mar 14, 2018 at 10:48:33AM +1300, Andrew Caudwell wrote:

> The packaged version of GLM, 0.9.9~a2 is an alpha (the current release is 
> still
> 0.9.8.5) and removes the default initialization of vector, matrix and
> quaternion types. Because of this code written against any earlier versions of
> GLM may now have uninitialized value bugs introduced by this change (e.g. 
> where
> GLM types are member variables of a class) or now behave differently (mat4()
> previously gave you an identity matrix, now this gives you a zero'd matrix).
> Several issues have been raised upstream (including by myself) to re-add
> initialization or at least make it optional.
> Additionally the requirement in this version to define GLM_ENABLE_EXPERIMENTAL
[...]
> to use simple functions like length2() has broken multiple packages. I have 
> put
> off fixing this since making it compile just exposes the user to the
> uninitialized value bugs. Unfortunately this has now meant my gource and
> logstalgia debian packages have been removed from debian since they don't
> complile with this GLM version.

I believe these are intentional changes by the author of GLM. I expect
that GLM 1.0.0 will be released before the next release of Debian, and
any packages that depend on GLM should instead be fixed to handle the
new behavior.

Unless I am mistaken, projects depending on GLM can just #define
GLM_ENABLE_EXPERIMENTAL and provide explicit default initializers, which
will be backwards compatible with older versions of GLM.

-- 
Met vriendelijke groet / with kind regards,
  Guus Sliepen 


signature.asc
Description: PGP signature

Bug#892861: libglm-dev: removal of default type initialization breaking packages

[Andrew Caudwell]

Package: libglm-dev
Version: 0.9.9~a2-1
Severity: important

Dear Maintainer,

The packaged version of GLM, 0.9.9~a2 is an alpha (the current release is still
0.9.8.5) and removes the default initialization of vector, matrix and
quaternion types. Because of this code written against any earlier versions of
GLM may now have uninitialized value bugs introduced by this change (e.g. where
GLM types are member variables of a class) or now behave differently (mat4()
previously gave you an identity matrix, now this gives you a zero'd matrix).
Several issues have been raised upstream (including by myself) to re-add
initialization or at least make it optional.

This is the commit that introduced the change:

https://github.com/g-truc/glm/commit/4cf8a10af2bba678c2ad136006e6ec41118b6746

Additionally the requirement in this version to define GLM_ENABLE_EXPERIMENTAL
to use simple functions like length2() has broken multiple packages. I have put
off fixing this since making it compile just exposes the user to the
uninitialized value bugs. Unfortunately this has now meant my gource and
logstalgia debian packages have been removed from debian since they don't
complile with this GLM version.

Related upstream issues:

https://github.com/g-truc/glm/issues/740
https://github.com/g-truc/glm/issues/735
https://github.com/g-truc/glm/issues/732

IMO the ideal solution would be for 0.9.8.5 to be packaged instead, or at the
very least revert the initialization change.

I believe 0.9.9~a2 was originally packaged as it included an upstream fix to
compile with GCC 7.3. Some other distros have instead patched 0.9.8.5:

https://gitweb.gentoo.org/repo/gentoo.git/diff/media-
libs/glm?id=1e45f7cd1dfa04ef4b89eb453df04b62ee432425



-- System Information:
Debian Release: buster/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 4.14.0-3-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_NZ.UTF-8, LC_CTYPE=en_NZ.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_NZ:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

-- no debconf information