Bug 2160996 - Review Request: draco - A library for compressing and decompressing 3D geometric meshes and point clouds
Summary: Review Request: draco - A library for compressing and decompressing 3D geomet...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Arthur Bols
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-01-15 02:21 UTC by Luya Tshimbalanga
Modified: 2023-02-05 01:53 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-02-05 00:38:11 UTC
Type: ---
Embargoed:
arthur: fedora-review+


Attachments (Terms of Use)
Downstream-only patch to use system gtest library (2.11 KB, text/plain)
2023-01-17 16:39 UTC, Ben Beasley
no flags Details
Patch to fix GCC 13 and enable tests (5.01 KB, text/plain)
2023-01-17 16:41 UTC, Ben Beasley
no flags Details
The .spec file difference from Copr build 5239312 to 5251604 (2.97 KB, patch)
2023-01-19 08:39 UTC, Jakub Kadlčík
no flags Details | Diff
The .spec file difference from Copr build 5251604 to 5260866 (342 bytes, patch)
2023-01-20 10:10 UTC, Jakub Kadlčík
no flags Details | Diff
Patch for fixing help2man (2.39 KB, patch)
2023-01-22 11:20 UTC, Arthur Bols
no flags Details | Diff
Correct patch for fixing help2man (1.22 KB, patch)
2023-01-22 11:22 UTC, Arthur Bols
no flags Details | Diff
The .spec file difference from Copr build 5260866 to 5285652 (1.85 KB, patch)
2023-01-23 17:01 UTC, Jakub Kadlčík
no flags Details | Diff

Description Luya Tshimbalanga 2023-01-15 02:21:00 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/luya/blender-egl/fedora-rawhide-x86_64/05232772-draco/draco.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/luya/blender-egl/fedora-rawhide-x86_64/05232772-draco/draco-devel-1.5.5-1.fc38.x86_64.rpm
Description: A library for compressing and decompressing 3D geometric meshes and point clouds
Fedora Account System Username:luya

Comment 3 Arthur Bols 2023-01-15 15:13:12 UTC
Hi luya


Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated

Issues:
=======

- missing license file

- missing documentation
     There seems to be a lot, so a subpackage could be beneficial.

- Package does not own files or directories owned by other packages.
     /usr/share/cmake is owned by this package. Change:
          %{_datadir}/cmake/
     to
          %{_datadir}/cmake/%{name}/

- missing %check
     Please check if it is feasible to compile and run the tests



Remarks:
========

- unnecessary %global _hardened_build 1


- %global _disable_ld_no_undefined %nil
     Does this do anything?

- remove commented `debug_package` and `_legacy_common_support` macros if they're not necessary.

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License 2.0", "*No copyright*
     Apache License 2.0", "MIT License", "*No copyright* The Unlicense".
     495 files have unknown license. Detailed output of licensecheck in
     /home/arthur/fedora-review/2160996-draco/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
[!]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/cmake(bash-
     completion, glm-devel, cmake-filesystem, kim-api-devel,
     python3-uranium, libmodman-devel, libwbxml-devel, websocketpp-devel)
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in draco-
     devel
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[!]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: draco-1.5.5-1.fc38.x86_64.rpm
          draco-devel-1.5.5-1.fc38.x86_64.rpm
          draco-debuginfo-1.5.5-1.fc38.x86_64.rpm
          draco-debugsource-1.5.5-1.fc38.x86_64.rpm
          draco-1.5.5-1.fc38.src.rpm
============================================================================================================================ rpmlint session starts ===========================================================================================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpeqes01aw')]
checks: 31, packages: 5

draco.x86_64: W: no-manual-page-for-binary draco_decoder
draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
draco.x86_64: W: no-manual-page-for-binary draco_encoder
draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
draco.x86_64: W: no-documentation
draco-devel.x86_64: W: no-documentation
============================================================================================= 5 packages and 0 specfiles checked; 0 errors, 6 warnings, 0 badness; has taken 1.7 s ============================================================================================




Rpmlint (debuginfo)
-------------------
Checking: draco-debuginfo-1.5.5-1.fc38.x86_64.rpm
============================================================================================================================ rpmlint session starts ===========================================================================================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpwc4i_3zc')]
checks: 31, packages: 1

============================================================================================= 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.4 s ============================================================================================





Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 4

draco.x86_64: W: no-manual-page-for-binary draco_decoder
draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
draco.x86_64: W: no-manual-page-for-binary draco_encoder
draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
draco.x86_64: W: no-documentation
draco-devel.x86_64: W: no-documentation
 4 packages and 0 specfiles checked; 0 errors, 6 warnings, 0 badness; has taken 1.2 s 



Source checksums
----------------
https://github.com/google/draco/archive/1.5.5/draco-1.5.5.tar.gz :
  CHECKSUM(SHA256) this package     : 6b7994150bbc513abcdbe22ad778d6b2df10fc8cdc7035e916985b2a209ab826
  CHECKSUM(SHA256) upstream package : 6b7994150bbc513abcdbe22ad778d6b2df10fc8cdc7035e916985b2a209ab826


Requires
--------
draco (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libdraco.so.7()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libm.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

draco-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem
    draco
    libdraco.so.7()(64bit)

draco-debuginfo (rpmlib, GLIBC filtered):

draco-debugsource (rpmlib, GLIBC filtered):



Provides
--------
draco:
    draco
    draco(x86-64)
    libdraco.so.7()(64bit)

draco-devel:
    cmake(draco)
    draco-devel
    draco-devel(x86-64)
    pkgconfig(draco)

draco-debuginfo:
    debuginfo(build-id)
    draco-debuginfo
    draco-debuginfo(x86-64)
    libdraco.so.7.0.0-1.5.5-1.fc38.x86_64.debug()(64bit)

draco-debugsource:
    draco-debugsource
    draco-debugsource(x86-64)



Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23
Command line :/usr/bin/fedora-review -b 2160996
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, C/C++, Generic
Disabled plugins: Ocaml, Haskell, PHP, Perl, R, Java, SugarActivity, Python, fonts
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 4 Ben Beasley 2023-01-15 15:31:56 UTC
I had started on a review for this, but hadn’t decided if I wanted to take it. I’ll post my findings in addition to Arthur’s official review.

Comment 5 Ben Beasley 2023-01-15 15:33:17 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated

===== Issues =====

- The docs/assets/ directory contains precompiled CSS and JavaScript, which
  could not be packaged as-is:

  https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/#_css
  https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/#_compilationminification

  This is OK since you are not packaging the HTML documentation, but it would
  be best if you remove the directory in %prep. 

  (I think this is more than adequate reason NOT to package the documentation.)

- Similarly, you should remove the following in %prep:

    - javascript/ contains precompiled JavaScript
    - maya/ contains precompiled Windows and MacOS binaries (inside .tar.gz
      archives)

- Add to %files:

    %license LICENSE AUTHORS

- Please remove the following, since it is not used and is therefore confusing:

    # draco git
    %global commit0 4cba1acdd718b700bb33945c0258283689d4eac7
    %global shortcommit0 %(c=%{commit0}; echo ${c:0:7})
    %global gver git%{shortcommit0}

- It would be good to remove these commented-out lines too:

    #%%global               debug_package %%{nil}
    #define _legacy_common_support 1

- This is the default since Fedora 23, so the explicit macro can be removed:

    %global         _hardened_build 1

  https://fedoraproject.org/wiki/Changes/Harden_All_Packages

- What is this, and why is it necessary? If it can’t be removed, can you write
  a spec file comment justifying why it is needed?

    %global         _disable_ld_no_undefined %nil

- The -devel package should have a fully versioned dependency on the base
  package. Instead of 

    Requires: draco >= %{version}-%{release}

  use

    Requires: draco%{?_isa} = %{version}-%{release}

- The use of the %{name} macro is inconsistent; this is not against the
  guidelines, but consider standardizing on either using %{name} or writing out
  “draco”. Personally, I’ve come to favor less use of trivial macros like this,
  but this is mostly a subjective question.

- Change

    %{_datadir}/cmake/

  to

    %{_datadir}/cmake/%{name}/

- Please check if this is doing anything useful; I suspect it is not, since
  Fedora is already setting LTO flags in CFLAGS/CXXFLAGS/LDFLAGS.

    -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON

- Please remove this, since it is already in the expansion of the %cmake macro (and uses %{_prefix} instead of hard-coded /usr):

    -DCMAKE_INSTALL_PREFIX=/usr \

- You have this twice; please remove one of them.

    -DCMAKE_INSTALL_PREFIX=/usr \

- Consider including README.md as %doc.

- Man pages are always desired (but not required):

    draco.x86_64: W: no-manual-page-for-binary draco_decoder
    draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
    draco.x86_64: W: no-manual-page-for-binary draco_encoder
    draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5

  https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "Apache License 2.0", "*No copyright*
     Apache License 2.0", "MIT License", "*No copyright* The Unlicense".
     495 files have unknown license. Detailed output of licensecheck in
     /home/reviewer/2160996-draco/licensecheck.txt

     Licenses other than Apache-2.0 come from docs/assets/, which will not be
     packaged.

[!]: License file installed when any subpackage combination is installed.
[!]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/cmake(cmake-
     filesystem, websocketpp-devel, python3-uranium, libmodman-devel, kim-
     api-devel, libwbxml-devel, glm-devel, bash-completion)
[!]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.

       Directories in third_party/ are empty.

[x]: Changelog in prescribed format.
[!]: Sources contain only permissible code or content.

       Precompiled JavaScript, CSS, and binaries are OK for sources but should
       be removed in %prep to ensure and demonstrate that they are not included
       in the binary RPMs.

[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: Package consistently uses macros (instead of hard-coded directory
     names).

       -DCMAKE_INSTALL_PREFIX=/usr \

[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[!]: Package is not known to require an ExcludeArch tag.

       ExclusiveArch is present

[x]: Package complies to the Packaging Guidelines

       (except as noted)

[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in draco-
     devel
[?]: Package functions as described.

     Can the test suite be executed?

[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[!]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: draco-1.5.5-1.fc38.x86_64.rpm
          draco-devel-1.5.5-1.fc38.x86_64.rpm
          draco-debuginfo-1.5.5-1.fc38.x86_64.rpm
          draco-debugsource-1.5.5-1.fc38.x86_64.rpm
          draco-1.5.5-1.fc38.src.rpm
=========================================================================================================== rpmlint session starts ===========================================================================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpv4dbwb1w')]
checks: 31, packages: 5

draco.x86_64: W: no-manual-page-for-binary draco_decoder
draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
draco.x86_64: W: no-manual-page-for-binary draco_encoder
draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
draco.x86_64: W: no-documentation
draco-devel.x86_64: W: no-documentation
============================================================================ 5 packages and 0 specfiles checked; 0 errors, 6 warnings, 0 badness; has taken 0.9 s ============================================================================




Rpmlint (debuginfo)
-------------------
Checking: draco-debuginfo-1.5.5-1.fc38.x86_64.rpm
=========================================================================================================== rpmlint session starts ===========================================================================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpuf2qpu3i')]
checks: 31, packages: 1

============================================================================ 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.3 s ============================================================================





Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 4

draco.x86_64: W: no-manual-page-for-binary draco_decoder
draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
draco.x86_64: W: no-manual-page-for-binary draco_encoder
draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
draco-devel.x86_64: W: no-documentation
draco.x86_64: W: no-documentation
 4 packages and 0 specfiles checked; 0 errors, 6 warnings, 0 badness; has taken 0.8 s 



Source checksums
----------------
https://github.com/google/draco/archive/1.5.5/draco-1.5.5.tar.gz :
  CHECKSUM(SHA256) this package     : 6b7994150bbc513abcdbe22ad778d6b2df10fc8cdc7035e916985b2a209ab826
  CHECKSUM(SHA256) upstream package : 6b7994150bbc513abcdbe22ad778d6b2df10fc8cdc7035e916985b2a209ab826


Requires
--------
draco (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libdraco.so.7()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libm.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

draco-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    cmake-filesystem
    draco
    libdraco.so.7()(64bit)

draco-debuginfo (rpmlib, GLIBC filtered):

draco-debugsource (rpmlib, GLIBC filtered):



Provides
--------
draco:
    draco
    draco(x86-64)
    libdraco.so.7()(64bit)

draco-devel:
    cmake(draco)
    draco-devel
    draco-devel(x86-64)
    pkgconfig(draco)

draco-debuginfo:
    debuginfo(build-id)
    draco-debuginfo
    draco-debuginfo(x86-64)
    libdraco.so.7.0.0-1.5.5-1.fc38.x86_64.debug()(64bit)

draco-debugsource:
    draco-debugsource
    draco-debugsource(x86-64)



Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23
Command line :/usr/bin/fedora-review -b 2160996
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: SugarActivity, Python, fonts, Haskell, Ocaml, PHP, Java, R, Perl
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 6 Ben Beasley 2023-01-15 15:35:45 UTC
Once this package is imported, one or more bugs will need to be filed blocking the appropriate tracker bugs per https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures, similar to bug 1960848.

Comment 7 Ben Beasley 2023-01-15 15:52:48 UTC
Are you sure this package needs to be ExclusiveArch: x86_64?

Upstream documentation (https://github.com/google/draco/blob/master/BUILDING.md) suggests that at least i686 and aarch64 should be supported, and after removing the ExclusiveArch line I was able to do a successful scratch build for all current Fedora primary architectures.

It would be good to get the tests running so that we can be sure everything is really working properly on all of the architectures.

Comment 8 Luya Tshimbalanga 2023-01-16 03:58:07 UTC
Thank you for taking the review

> - missing license file
> 
Fixed by including LICENSE file 
> - missing documentation
>      There seems to be a lot, so a subpackage could be beneficial.

As pointed out by Ben, it turned out unnecessary because of precompiled javascripts.

> 
> - Package does not own files or directories owned by other packages.
>      /usr/share/cmake is owned by this package. Change:
>           %{_datadir}/cmake/
>      to
>           %{_datadir}/cmake/%{name}/

Fixed.

> 
> - missing %check
>      Please check if it is feasible to compile and run the tests
> 

%ctest applied on %check line.

> - unnecessary %global _hardened_build 1
> 
> 
> - %global _disable_ld_no_undefined %nil
>      Does this do anything?
> 
> - remove commented `debug_package` and `_legacy_common_support` macros if
> they're not necessary.
> 

All removed.

> ===== Issues =====
> 
> - The docs/assets/ directory contains precompiled CSS and JavaScript, which
>   could not be packaged as-is:
> 
>   https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/#_css
>  
> https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/
> #_compilationminification
> 
>   This is OK since you are not packaging the HTML documentation, but it would
>   be best if you remove the directory in %prep. 
> 
>   (I think this is more than adequate reason NOT to package the
> documentation.)

Done.

> 
> - Similarly, you should remove the following in %prep:
> 
>     - javascript/ contains precompiled JavaScript
>     - maya/ contains precompiled Windows and MacOS binaries (inside .tar.gz
>       archives)
> 

Done by removing both javascript and maya directory.


> - Add to %files:
> 
>     %license LICENSE AUTHORS
> 

Fixed


> - Please remove the following, since it is not used and is therefore
> confusing:
> 
>     # draco git
>     %global commit0 4cba1acdd718b700bb33945c0258283689d4eac7
>     %global shortcommit0 %(c=%{commit0}; echo ${c:0:7})
>     %global gver git%{shortcommit0}
> 
> - It would be good to remove these commented-out lines too:
> 
>     #%%global               debug_package %%{nil}
>     #define _legacy_common_support 1
> 
> - This is the default since Fedora 23, so the explicit macro can be removed:
> 
>     %global         _hardened_build 1
> 
>     %global         _disable_ld_no_undefined %nil

Above lines removed.

> 
> - The -devel package should have a fully versioned dependency on the base
>   package. Instead of 
> 
>     Requires: draco >= %{version}-%{release}
> 
>   use
> 
>     Requires: draco%{?_isa} = %{version}-%{release}
> 

Fixed.

> - The use of the %{name} macro is inconsistent; this is not against the
>   guidelines, but consider standardizing on either using %{name} or writing
> out
>   “draco”. Personally, I’ve come to favor less use of trivial macros like
> this,
>   but this is mostly a subjective question.
>

An oversight from by part. I chose keeping the %{name} macro for consistency.
 
> - Change
> 
>     %{_datadir}/cmake/
> 
>   to
> 
>     %{_datadir}/cmake/%{name}/
> 
Fixed

> - Please check if this is doing anything useful; I suspect it is not, since
>   Fedora is already setting LTO flags in CFLAGS/CXXFLAGS/LDFLAGS.
> 
>     -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON

It turned out unneeded so that line is removed.


> 
> - Please remove this, since it is already in the expansion of the %cmake
> macro (and uses %{_prefix} instead of hard-coded /usr):
> 
>     -DCMAKE_INSTALL_PREFIX=/usr \
> 
> - You have this twice; please remove one of them.
> 
>     -DCMAKE_INSTALL_PREFIX=/usr \
> 
Fixed by removed them.


> - Consider including README.md as %doc.

Done


> 
> - Man pages are always desired (but not required):
> 
>     draco.x86_64: W: no-manual-page-for-binary draco_decoder
>     draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
>     draco.x86_64: W: no-manual-page-for-binary draco_encoder
>     draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
> 
>   https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages


The linked guideline failed to clarify about the proper use of "help2man" command in the spec file. I commented out these line for the time until a proper solution occurs.


(In reply to Ben Beasley from comment #7)
> Are you sure this package needs to be ExclusiveArch: x86_64?
> 
> Upstream documentation
> (https://github.com/google/draco/blob/master/BUILDING.md) suggests that at
> least i686 and aarch64 should be supported, and after removing the
> ExclusiveArch line I was able to do a successful scratch build for all
> current Fedora primary architectures.

The build succeeded on all supported architecture as highlighted on  

https://copr.fedorainfracloud.org/coprs/luya/blender-egl/build/5235725/
so ExclusiveArch line is removed.


Based on the feedback,  here is the update:
SPEC: https://download.copr.fedorainfracloud.org/results/luya/blender-egl/fedora-rawhide-x86_64/05235725-draco/draco.spec
SRPM: https://download.copr.fedorainfracloud.org/results/luya/blender-egl/fedora-rawhide-x86_64/05235725-draco/draco-devel-1.5.5-1.fc38.x86_64.rpm

Comment 10 Arthur Bols 2023-01-16 23:00:31 UTC
Thanks for the changes!

- I noticed you've added this macro. Does this do anything? The only reference I could find was a closed PR.

    %global     __cmake_builder ninja
    

- As Ben pointed earlier, it would be best to remove the docs/assets
  directory, as it contains precompiled CSS and JavaScript.



(In reply to Luya Tshimbalanga from comment #8)
> The linked guideline failed to clarify about the proper use of "help2man"
> command in the spec file. I commented out these line for the time until a
> proper solution occurs.

You can use it like this:

    mkdir -p %{buildroot}%{_mandir}/man1
    help2man -N -o %{buildroot}%{_mandir}/man1/%{name}_decoder.1 %{buildroot}/%{_bindir}/%{name}_decoder
    help2man -N -o %{buildroot}%{_mandir}/man1/%{name}_encoder.1 %{buildroot}/%{_bindir}/%{name}_encoder
    
    *snip*
    
    %{_mandir}/man1/%{name}_decoder.1*
    %{_mandir}/man1/%{name}_encoder.1*

> > 
> > - missing %check
> >      Please check if it is feasible to compile and run the tests
> > 
> 
> %ctest applied on %check line.

Sadly this doesn't work:
    
    + /usr/bin/ctest --test-dir redhat-linux-build --output-on-failure --force-new-ctest-process -j8
    Internal ctest changing into directory: /builddir/build/BUILD/draco-1.5.5/redhat-linux-build
    Test project /builddir/build/BUILD/draco-1.5.5/redhat-linux-build
    No tests were found!!!
    
You will have to set -DDRACO_TESTS=ON during %build to compile the tests, but 
this won't work by itself. It tries to find the googletest submodule, which 
isn't included in the tarball. Maybe this is a trivial fix, so it would be 
great if you could look into it further!

Comment 11 Luya Tshimbalanga 2023-01-17 01:43:27 UTC
(In reply to Arthur Bols from comment #10)
> Thanks for the changes!
> 
> - I noticed you've added this macro. Does this do anything? The only
> reference I could find was a closed PR.
> 
>     %global     __cmake_builder ninja

It looks like that macro does nothing so I removed it.
>     
> 
> - As Ben pointed earlier, it would be best to remove the docs/assets
>   directory, as it contains precompiled CSS and JavaScript.
> 

Done.

> 
> (In reply to Luya Tshimbalanga from comment #8)
> > The linked guideline failed to clarify about the proper use of "help2man"
> > command in the spec file. I commented out these line for the time until a
> > proper solution occurs.
> 
> You can use it like this:
> 
>     mkdir -p %{buildroot}%{_mandir}/man1
>     help2man -N -o %{buildroot}%{_mandir}/man1/%{name}_decoder.1
> %{buildroot}/%{_bindir}/%{name}_decoder
>     help2man -N -o %{buildroot}%{_mandir}/man1/%{name}_encoder.1
> %{buildroot}/%{_bindir}/%{name}_encoder
>     
>     *snip*
>     
>     %{_mandir}/man1/%{name}_decoder.1*
>     %{_mandir}/man1/%{name}_encoder.1*

I hit an issue with rpmlint with the following result:
rpmlint /var/lib/mock/fedora-37-x86_64/result/*.rpm
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 5

draco.x86_64: W: no-manual-page-for-binary draco_decoder-1.5.5
draco.x86_64: W: no-manual-page-for-binary draco_encoder-1.5.5
draco-devel.x86_64: W: no-documentation
 5 packages and 0 specfiles checked; 0 errors, 3 warnings, 0 badness; has taken 


How to properely implement the help2man for the case above? I vainly tried

help2man -N -o %{buildroot}%{_mandir}/man5/%{name}_decoder.1-%{version} \
	--no-discard-stderr  %{buildroot}/%{_bindir}/%{name}_decoder-%{version}
help2man -N -o %{buildroot}%{_mandir}/man5/%{name}_encoder.1-%{version} \
	--no-discard-stderr  %{buildroot}/%{_bindir}/%{name}_encoder-%{version}






> 
> > > 
> > > - missing %check
> > >      Please check if it is feasible to compile and run the tests
> > > 
> > 
> > %ctest applied on %check line.
> 
> Sadly this doesn't work:
>     
>     + /usr/bin/ctest --test-dir redhat-linux-build --output-on-failure
> --force-new-ctest-process -j8
>     Internal ctest changing into directory:
> /builddir/build/BUILD/draco-1.5.5/redhat-linux-build
>     Test project /builddir/build/BUILD/draco-1.5.5/redhat-linux-build
>     No tests were found!!!
>     
Hm, in that condition, running the test is unfeasible in the current state it is temporarily disabled for the time being.

Comment 13 Jakub Kadlčík 2023-01-17 08:19:24 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5239312
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160996-draco/fedora-rawhide-x86_64/05239312-draco/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 14 Arthur Bols 2023-01-17 10:38:13 UTC
(In reply to Luya Tshimbalanga from comment #11)
> How to properely implement the help2man for the case above? I vainly tried
> 
> help2man -N -o %{buildroot}%{_mandir}/man5/%{name}_decoder.1-%{version} \
> 	--no-discard-stderr  %{buildroot}/%{_bindir}/%{name}_decoder-%{version}
> help2man -N -o %{buildroot}%{_mandir}/man5/%{name}_encoder.1-%{version} \
> 	--no-discard-stderr  %{buildroot}/%{_bindir}/%{name}_encoder-%{version}

I can't get rid of the warning message either... If you really want to solve it, you could try asking on the devel list.
In my opinion, it doesn't really matter since draco_decoder is just a symlink to draco_decoder-1.5.5, and the man page works fine.

I think this will create the most useful manpages:

    mkdir -p %{buildroot}%{_mandir}/man1
    help2man -N --version-string=%{version} -o %{buildroot}%{_mandir}/man1/%{name}_decoder-%{version}.1 %{buildroot}/%{_bindir}/%{name}_decoder
    help2man -N --version-string=%{version} -o %{buildroot}%{_mandir}/man1/%{name}_encoder-%{version}.1 %{buildroot}/%{_bindir}/%{name}_encoder

    cd %{buildroot}%{_mandir}/man1
    ln -s %{name}_decoder-%{version}.1 %{name}_decoder.1
    ln -s %{name}_encoder-%{version}.1 %{name}_encoder.1


    %{_mandir}/man1/%{name}_decoder.1*
    %{_mandir}/man1/%{name}_encoder.1*
    %{_mandir}/man1/%{name}_decoder-%{version}.1*
    %{_mandir}/man1/%{name}_encoder-%{version}.1*
    

So what I did:
- Run help2man against draco_decoder to not include the version number in the command name (I couldn't find a better way to do this).
- Add --version-string=%{version}
    Otherwise the --help output is included twice
- Create a symlink from draco_decoder to draco_decoder-%{version}
    This makes sure `man draco_decoder` and `man draco_decoder-1.5.5` work.

> Hm, in that condition, running the test is unfeasible in the current state
> it is temporarily disabled for the time being.
That's fine.


- I just noticed an inconsistent use of macros:
  For %files devel you used:

    %{_libdir}/lib%{name}.so

  But for %files:

    %{_libdir}/libdraco.so.7
    %{_libdir}/libdraco.so.7.0.0


I couldn't find any more issues, so hopefully this is the last iteration! :)

Comment 15 Arthur Bols 2023-01-17 10:53:43 UTC
(In reply to Arthur Bols from comment #14)
> 
> I think this will create the most useful manpages:
> 
>     mkdir -p %{buildroot}%{_mandir}/man1
>     help2man -N --version-string=%{version} -o
> %{buildroot}%{_mandir}/man1/%{name}_decoder-%{version}.1
> %{buildroot}/%{_bindir}/%{name}_decoder
>     help2man -N --version-string=%{version} -o
> %{buildroot}%{_mandir}/man1/%{name}_encoder-%{version}.1
> %{buildroot}/%{_bindir}/%{name}_encoder
> 
>     cd %{buildroot}%{_mandir}/man1
>     ln -s %{name}_decoder-%{version}.1 %{name}_decoder.1
>     ln -s %{name}_encoder-%{version}.1 %{name}_encoder.1
> 
> 
>     %{_mandir}/man1/%{name}_decoder.1*
>     %{_mandir}/man1/%{name}_encoder.1*
>     %{_mandir}/man1/%{name}_decoder-%{version}.1*
>     %{_mandir}/man1/%{name}_encoder-%{version}.1*
>     
> 
> So what I did:
> - Run help2man against draco_decoder to not include the version number in
> the command name (I couldn't find a better way to do this).
> - Add --version-string=%{version}
>     Otherwise the --help output is included twice
> - Create a symlink from draco_decoder to draco_decoder-%{version}
>     This makes sure `man draco_decoder` and `man draco_decoder-1.5.5` work.

That doesn't work, I missed some things. This should be correct:

    mkdir -p %{buildroot}%{_mandir}/man1
    LD_LIBRARY_PATH=%{buildroot}%{_libdir} help2man -N --version-string=%{version} -o %{buildroot}%{_mandir}/man1/%{name}_decoder-%{version}.1 %{buildroot}%{_bindir}/%{name}_decoder
    LD_LIBRARY_PATH=%{buildroot}%{_libdir} help2man -N --version-string=%{version} -o %{buildroot}%{_mandir}/man1/%{name}_encoder-%{version}.1 %{buildroot}%{_bindir}/%{name}_encoder

    %{_mandir}/man1/%{name}_decoder-%{version}.1*
    %{_mandir}/man1/%{name}_encoder-%{version}.1*

- The symlink for the manpage isn't needed
- We need to add `LD_LIBRARY_PATH` to find libdraco.so.7

Comment 16 Ben Beasley 2023-01-17 16:30:34 UTC
It looks like the introduction of GCC 13 to Rawhide broke this. Looks like a missing #include or “using”.

https://koji.fedoraproject.org/koji/taskinfo?taskID=96251494

[32/141] /usr/bin/g++ -DDRACO_CMAKE=1 -DDRACO_FLAGS_SRCDIR=\"/builddir/build/BUILD/draco-1.5.5\" -DDRACO_FLAGS_TMPDIR=\"/tmp\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/builddir/build/BUILD/draco-1.5.5 -I/builddir/build/BUILD/draco-1.5.5/src -I/builddir/build/BUILD/draco-1.5.5/redhat-linux-build -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -DNDEBUG -fPIC -MD -MT CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o -MF CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o.d -o CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o -c /builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.cc
FAILED: CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o 
/usr/bin/g++ -DDRACO_CMAKE=1 -DDRACO_FLAGS_SRCDIR=\"/builddir/build/BUILD/draco-1.5.5\" -DDRACO_FLAGS_TMPDIR=\"/tmp\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/builddir/build/BUILD/draco-1.5.5 -I/builddir/build/BUILD/draco-1.5.5/src -I/builddir/build/BUILD/draco-1.5.5/redhat-linux-build -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -DNDEBUG -fPIC -MD -MT CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o -MF CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o.d -o CMakeFiles/draco_io.dir/src/draco/io/file_utils.cc.o -c /builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.cc
In file included from /builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.cc:15:
/builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.h:61:35: error: 'uint8_t' was not declared in this scope
   61 |                       std::vector<uint8_t> *buffer);
      |                                   ^~~~~~~
/builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.h:20:1: note: 'uint8_t' is defined in header '<cstdint>'; did you forget to '#include <cstdint>'?
   19 | #include <vector>
  +++ |+#include <cstdint>
   20 | 
/builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.h:61:42: error: template argument 1 is invalid
   61 |                       std::vector<uint8_t> *buffer);
      |                                          ^
/builddir/build/BUILD/draco-1.5.5/src/draco/io/file_utils.h:61:42: error: template argument 2 is invalid

Comment 17 Ben Beasley 2023-01-17 16:36:39 UTC
https://github.com/google/draco/pull/964

Comment 18 Ben Beasley 2023-01-17 16:39:50 UTC
Created attachment 1938649 [details]
Downstream-only patch to use system gtest library

Comment 19 Ben Beasley 2023-01-17 16:41:25 UTC
Created attachment 1938650 [details]
Patch to fix GCC 13 and enable tests

Includes the patches from my previous two comments, along with some small changes to the spec file.

Comment 21 Jakub Kadlčík 2023-01-19 08:39:05 UTC
Created attachment 1939112 [details]
The .spec file difference from Copr build 5239312 to 5251604

Comment 22 Jakub Kadlčík 2023-01-19 08:39:08 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5251604
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160996-draco/fedora-rawhide-x86_64/05251604-draco/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 24 Jakub Kadlčík 2023-01-20 10:10:12 UTC
Created attachment 1939320 [details]
The .spec file difference from Copr build 5251604 to 5260866

Comment 25 Jakub Kadlčík 2023-01-20 10:10:15 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5260866
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160996-draco/fedora-rawhide-x86_64/05260866-draco/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 26 Arthur Bols 2023-01-22 11:16:47 UTC
Sorry for the delay, I've been busy studying.

I think you missed my last comment. For generating the man pages, you shouldn't need --no-discard-error. It does only fail because of the missing library, so you'll have to add `LD_LIBRARY_PATH` (see the patch in the next comment).
The symlinks are also not necessary, so they may be removed as well. (`man draco_encoder` and `man draco_encoder-1.5.5` both work fine)

Since the tests are now working, you can remove the gtest macro, but you can keep it if you want.

Finally, there is an rpmlint error: W: mixed-use-of-spaces-and-tabs (spaces: line 19, tab: line 2)

I believe everything else looks good!

Comment 27 Arthur Bols 2023-01-22 11:20:19 UTC
Created attachment 1939783 [details]
Patch for fixing help2man

Comment 28 Arthur Bols 2023-01-22 11:22:20 UTC
Created attachment 1939785 [details]
Correct patch for fixing help2man

Comment 30 Jakub Kadlčík 2023-01-23 17:01:00 UTC
Created attachment 1940011 [details]
The .spec file difference from Copr build 5260866 to 5285652

Comment 31 Jakub Kadlčík 2023-01-23 17:01:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5285652
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160996-draco/fedora-rawhide-x86_64/05285652-draco/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 32 Arthur Bols 2023-01-25 08:27:53 UTC
Looks good!

Package APPROVED.

Comment 33 Luya Tshimbalanga 2023-01-27 02:19:06 UTC
Thank you Arthur!

Comment 34 Fedora Admin user for bugzilla script actions 2023-01-27 02:21:35 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/draco

Comment 35 Fedora Update System 2023-01-27 07:11:50 UTC
FEDORA-2023-b164ffd66d has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2023-b164ffd66d

Comment 36 Fedora Update System 2023-01-27 07:11:50 UTC
FEDORA-EPEL-2023-eecebf4f97 has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-eecebf4f97

Comment 37 Fedora Update System 2023-01-27 07:11:51 UTC
FEDORA-2023-87cef26f69 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-87cef26f69

Comment 38 Fedora Update System 2023-01-28 01:41:21 UTC
FEDORA-2023-87cef26f69 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-87cef26f69 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-87cef26f69

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 39 Fedora Update System 2023-01-28 01:42:28 UTC
FEDORA-EPEL-2023-eecebf4f97 has been pushed to the Fedora EPEL 9 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-eecebf4f97

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 40 Fedora Update System 2023-01-28 02:19:15 UTC
FEDORA-2023-b164ffd66d has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-b164ffd66d \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-b164ffd66d

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 41 Fedora Update System 2023-02-05 00:38:11 UTC
FEDORA-EPEL-2023-eecebf4f97 has been pushed to the Fedora EPEL 9 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 42 Fedora Update System 2023-02-05 01:46:21 UTC
FEDORA-2023-87cef26f69 has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 43 Fedora Update System 2023-02-05 01:53:02 UTC
FEDORA-2023-b164ffd66d has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.