Bug#892861: Fwd: Bug#892861: libglm-dev: removal of default type initialization breaking packages
-- Forwarded message -- From: Andrew Caudwell 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 On Thu, Mar 29, 2018 at 9:12 AM, Guus Sliepen 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 > From 8390a77b3a278b15259e5ca6e67f7e41badc457b Mon Sep 17 00:00:00 2001 From: Christophe Riccio 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>::mat() - {} + { +# ifdef GLM_FORCE_CTOR_INIT +this->value[0] = col_type(1, 0); +this->value[1] = col_ty
Bug#892861: libglm-dev: removal of default type initialization breaking packages
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
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
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
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
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