[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #14 from Fedora Update System  ---
openzwave-1.4.164-1.fc25 has been pushed to the Fedora 25 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2017-07-20 11:53:44



--- Comment #13 from Fedora Update System  ---
openzwave-1.4.164-1.fc26 has been pushed to the Fedora 26 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #12 from Fedora Update System  ---
openzwave-1.4.164-1.fc26 has been pushed to the Fedora 26 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-959b609e7d

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #11 from Fedora Update System  ---
openzwave-1.4.164-1.fc25 has been pushed to the Fedora 25 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-e568d4629e

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Fedora Update System  changed:

   What|Removed |Added

 Status|ASSIGNED|MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #10 from Fedora Update System  ---
openzwave-1.4.164-1.fc26 has been submitted as an update to Fedora 26.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-959b609e7d

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #9 from Gwyn Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/openzwave

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Michael Cronenworth  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Ben Rosser  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #8 from Ben Rosser  ---
Okay; package is now APPROVED then.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #7 from Michael Cronenworth  ---
They bundle 0.7.0 and Fedora carries a git checkout (0.8.0+). I'm waiting on my
Z-wave stick to arrive so I can't test it at the moment. I'll mark it as
bundled for now.

Spec: http://michael.cronenworth.com/RPMS/openzwave.spec
SRPM: http://michael.cronenworth.com/RPMS/openzwave-1.4.164-4.fc25.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Ben Rosser  changed:

   What|Removed |Added

  Flags|fedora-review+  |fedora-review?



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Ben Rosser  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #6 from Ben Rosser  ---
> Strange that my rpmlint does not disclose the symbol issue otherwise I would 
> have not wasted your time with another review scan.

For whatever reason, rpmlint only picked that up when it scanned the installed
package, not the RPM (both of which fedora-review does). Not entirely sure why.

Anyway, I apologize, but I did one final review pass and there's one more thing
that I missed initially. In addition to tinyxml, openzwave appears to bundle
hidapi: https://github.com/OpenZWave/open-zwave/tree/master/cpp/hidapi.

It may not be possible to unbundle, as it looks like upstream may have modified
the tree themselves. If that's the case please add a bundled Provides to the
spec as per
https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #5 from Michael Cronenworth  ---
(In reply to Ben Rosser from comment #4)
> That all now looks good, but rpmlint now throws the following warnings:
[snip]
> Are you sure that the library doesn't want to link against tinyxml?

Strange that my rpmlint does not disclose the symbol issue otherwise I would
have not wasted your time with another review scan. Yes, the library does use
tinyxml. Fixed.

New spec: http://michael.cronenworth.com/RPMS/openzwave.spec
New SRPM: http://michael.cronenworth.com/RPMS/openzwave-1.4.164-3.fc25.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #4 from Ben Rosser  ---
That all now looks good, but rpmlint now throws the following warnings:

libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 vtable for TiXmlDocument
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::~TiXmlNode()
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::FirstChildElement(char const*) const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlDocument::TiXmlDocument()
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 vtable for TiXmlText
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::FirstChildElement() const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::LinkEndChild(TiXmlNode*)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::Attribute(char const*) const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::TiXmlNode(TiXmlNode::NodeType)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::SetAttribute(char const*, char
const*)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlDocument::SaveFile(char const*) const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlNode::NextSiblingElement() const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::GetText() const
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlDeclaration::TiXmlDeclaration(char const*,
char const*, char const*)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::SetAttribute(char const*, int)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlDocument::LoadFile(char const*,
TiXmlEncoding)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::TiXmlElement(char const*)
libopenzwave.x86_64: W: undefined-non-weak-symbol
/usr/lib64/libopenzwave.so.1.4 TiXmlElement::QueryIntAttribute(char const*,
int*) const

Are you sure that the library doesn't want to link against tinyxml?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #3 from Michael Cronenworth  ---
(In reply to Ben Rosser from comment #1)
> Issues:
> ===
> - Large documentation must go in a -doc subpackage. Large could be size
>   (~1MB) or number of files.
>   Note: Documentation size is 110735360 bytes in 2808 files.
>   See:
>   http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

The documentation is API documentation. The guidelines specifically state "For
example API documentation belongs in the -devel subpackage, not the main one.",
which is what occurs here. Now, large documentation is recommended to a
sub-package (keyword: recommended), but it is still API documentation. I'll
create a -devel-docs subpackage.

> 
> - According to licensecheck, tinyxml sources appear to be bundled:
> 
> zlib/libpng
> ---
> openzwave-1.4.164/cpp/tinyxml/tinyxml.cpp
> openzwave-1.4.164/cpp/tinyxml/tinyxml.h
> openzwave-1.4.164/cpp/tinyxml/tinyxmlerror.cpp
> openzwave-1.4.164/cpp/tinyxml/tinyxmlparser.cpp
> 
>   Please either unbundle and remove these files if they're not necessary, or
>   add the appropriate Bundled provides and modify the License field
> accordingly
>   to "LGPLv3+ and zlib"
>  
> (https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:
> Bundled_Libraries)

The example program uses tinyxml and not the library itself. I've removed the
compiling pieces and linked the example program against the system library.

> 
> - Binaries don't appear to be getting built with all hardening enabled:

Fixed.

> - In a similar vein, it looks as if you haven't enabled parallel build in
> make. 
>   Is there a reason for this?

Deficiency from the upstream spec file this review originated from.

> - rpmlint warning:
>   libopenzwave.x86_64: W: shared-lib-calls-exit
> /usr/lib64/libopenzwave.so.1.4 exit@GLIBC_2.2.5
>   This should probably be investigated and reported upstream.

The code is doing the opposite of what it describes... I've patched it and will
report upstream.

New spec: http://michael.cronenworth.com/RPMS/openzwave.spec
New SRPM: http://michael.cronenworth.com/RPMS/openzwave-1.4.164-2.fc25.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #2 from Ben Rosser  ---
> ldconfig called in %post and %postun if required.

My bad, sorry. The issue here is that the libraries are in a subpackage and
therefore the %post scriptlets must have a -n libopenzwave specified, otherwise
they'll be put in the wrong package. It's nothing to do with the order.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Ben Rosser  changed:

   What|Removed |Added

  Flags||fedora-review?



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766



--- Comment #1 from Ben Rosser  ---
Issues:
===
- Dist tag is present. Please add _dist macro to the Release field.

- ldconfig called in %post and %postun if required.
  Note: /sbin/ldconfig not called in libopenzwave
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries

  Note: this appears to be a false positive from fedora-review because the
  macros are located underneath %files. I think it can safely be ignored, but
  just inquired on #fedora-devel about it to be safe.

- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 110735360 bytes in 2808 files.
  See:
  http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

- According to licensecheck, tinyxml sources appear to be bundled:

zlib/libpng
---
openzwave-1.4.164/cpp/tinyxml/tinyxml.cpp
openzwave-1.4.164/cpp/tinyxml/tinyxml.h
openzwave-1.4.164/cpp/tinyxml/tinyxmlerror.cpp
openzwave-1.4.164/cpp/tinyxml/tinyxmlparser.cpp

  Please either unbundle and remove these files if they're not necessary, or
  add the appropriate Bundled provides and modify the License field accordingly
  to "LGPLv3+ and zlib"
 
(https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries)

- Binaries don't appear to be getting built with all hardening enabled:

$ checksec --file ./MinOZW 
RELRO   STACK CANARY  NXPIE RPATH 
RUNPATH  FORTIFY Fortified Fortifiable  FILE
Partial RELRO   No canary found   NX enabledNo PIE  No RPATH   No
RUNPATH   No  0   2   ./MinOZW

  This suggests that compiler flags aren't being passed to the makefile. I 
  suspect you need to explicitly pass %{optflags} and possibly
  %{__global_ldflags} to make in order to build with the right flags.

- In a similar vein, it looks as if you haven't enabled parallel build in make. 
  Is there a reason for this?

- rpmlint warning:
  libopenzwave.x86_64: W: shared-lib-calls-exit /usr/lib64/libopenzwave.so.1.4
exit@GLIBC_2.2.5
  This should probably be investigated and reported upstream.

- rpmlint warning:
  libopenzwave.x86_64: W: unused-direct-shlib-dependency
/usr/lib64/libopenzwave.so.1.4 /lib64/libm.so.6
  This may be a consequence of the compiler flags being wrong, but should
  likely also be investigated: 
  
https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Ben Rosser  changed:

   What|Removed |Added

 CC||rosser@gmail.com
   Assignee|nob...@fedoraproject.org|rosser@gmail.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1468766] Review Request: openzwave - OpenZWave is a library to support for Z-Wave home-automation devices

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1468766

Michael Cronenworth  changed:

   What|Removed |Added

 Blocks||1468768




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1468768
[Bug 1468768] Review Request: domoticz - Open source Home Automation System
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org