Bug 2115560 - Review Request: DirectX-Headers - Direct3D 12 headers
Summary: Review Request: DirectX-Headers - Direct3D 12 headers
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-08-04 22:21 UTC by Pete Walter
Modified: 2024-02-27 17:47 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Pete Walter 2022-08-04 22:21:06 UTC
Spec URL: https://pwalter.fedorapeople.org/DirectX-Headers.spec
SRPM URL: https://pwalter.fedorapeople.org/DirectX-Headers-1.606.3-1.fc37.src.rpm
Description: This package provides the development headers for Direct3D 12.
Fedora Account System Username: pwalter

Comment 1 Petr Menšík 2022-09-19 15:58:16 UTC
This package should run test, it has some present in sources. Therefore it should not be noarch package, but built separately for every platform.

Also I think it should follow devel subpackages rule. Static library is a problem definitely. I guess it might provide libdxguid.so in a main package. Fedora requires all code libraries to be shared. It does not seem to me those guid list should be an exception. Though it does not add more additional code.

Comment 2 Stefan Hoffmeister 2023-02-26 11:47:15 UTC
This specific package apparently wraps https://github.com/microsoft/DirectX-Headers - which effectively is "only" a set of header files, to be located by pkgconfig. Any content is not expected to be used.

For Fedora these headers are of interest to build the "d3d12" Mesa DRI driver subset - and that is quite relevant for running Fedora Linux on the Microsoft Windows Subsystem for Linux, in particular for Desktop interaction through the WSLg subsystem, see https://github.com/microsoft/wslg

It is not expected that users of Fedora Linux ever install this package (except to rebuild the Mesa driver world).

So, given the above:

a) "tests" - the test is neither effective nor meaningful for Fedora Linux; I fully understand the presence of "%meson -Dbuild-test=false"

b) the "-devel" part is a bit trickier - one could argue that this package is only for "internal" builds of Fedora, it is not really a -devel package, but a Fedora Linux product build enabling package, only meant for use by Fedora Linux building Mesa?

I am willing to invest some time to make things happen, but these things should be meaningful in the Big Picture.

In the end, once this package here exists, the Fedora Linux Mesa package build configuration will be extended with "pkgconfig(DirectX-Header) >= ...", some build options tweakage will take place, and that's almost certainly all there will ever be.

Anything that gets Fedora to the above, I can spend some time on.

Comment 3 Petr Menšík 2023-03-11 22:53:06 UTC
Even include-only C++ libraries are *-devel packages. I understand that primary goal might be building Mesa with it, but it should follow general rules where possible. I do not think there is any reason to not move headers to devel subpackage. Header only libraries have their own section [1]. I think that pretty much applies to those libraries as well.
There is also special section for static libraries [2], which are used in this package too. That also does not pass in current package.

In any case, noarch package cannot contain compiled library, be it static or dynamic. It must have own architecture package, compiled for any architecture where it may work. I just guess that might be just x86_64 and aarch64 architectures.

As for the pkg-config file, it just requires library -lDirectX-Guids. It seems to me there is no reason for it to not be shared. Except it would need versioning in that case implemented.

I think your plan for Mesa does not pass [3]. If you insist on using static libraries as is common on windows, it has to be at least packaged as valid static library. It required exception from Packaging comitee before, I have not seen any link to that.

The main point might be building mesa, but this package can be the only package of DirectX-Headers in Fedora. So it should be as general as possible. If other project needs it for something, it should be able to use it too.

As for tests, not sure what do they need. If they can pass only on actual WSL builder and not in common builder, consider having %bcond_with check disabled by default. But may it allow fedpkg local --with check to build it including tests.

%bcond_with check

...
%check
%if %{with check}
  test/DirectX-Headers-Test
%endif

1. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_packaging_header_only_libraries
2. https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries
3. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_statically_linking_executables

Comment 4 Petr Menšík 2023-03-11 22:57:43 UTC
Package Review
==============

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


Issues:
=======
- Header files in -devel subpackage, if present.
  Note: DirectX-Headers : /usr/include/directx/d3d12.h DirectX-Headers :
  /usr/include/directx/d3d12compatibility.h DirectX-Headers :
  /usr/include/directx/d3d12sdklayers.h DirectX-Headers :
  /usr/include/directx/d3d12shader.h DirectX-Headers :
  /usr/include/directx/d3d12video.h DirectX-Headers :
  /usr/include/directx/d3dcommon.h DirectX-Headers :
  /usr/include/directx/d3dx12.h DirectX-Headers :
  /usr/include/directx/dxcore.h DirectX-Headers :
  /usr/include/directx/dxcore_interface.h DirectX-Headers :
  /usr/include/directx/dxgicommon.h DirectX-Headers :
  /usr/include/directx/dxgiformat.h DirectX-Headers :
  /usr/include/dxguids/dxguids.h DirectX-Headers :
  /usr/include/wsl/stubs/basetsd.h DirectX-Headers :
  /usr/include/wsl/stubs/oaidl.h DirectX-Headers :
  /usr/include/wsl/stubs/ocidl.h DirectX-Headers :
  /usr/include/wsl/stubs/rpc.h DirectX-Headers :
  /usr/include/wsl/stubs/rpcndr.h DirectX-Headers :
  /usr/include/wsl/stubs/unknwn.h DirectX-Headers :
  /usr/include/wsl/stubs/unknwnbase.h DirectX-Headers :
  /usr/include/wsl/stubs/winapifamily.h DirectX-Headers :
  /usr/include/wsl/stubs/wrl/client.h DirectX-Headers :
  /usr/include/wsl/stubs/wrl/implements.h DirectX-Headers :
  /usr/include/wsl/winadapter.h DirectX-Headers :
  /usr/include/wsl/wrladapter.h
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_devel_packages
- Static libraries in -static or -devel subpackage, providing -devel if
  present.
  Note: Package has .a files: DirectX-Headers. Illegal package name:
  DirectX-Headers. Does not provide -static: DirectX-Headers.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#packaging-static-libraries


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

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]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "MIT License", "*No copyright* MIT
     License". 5 files have unknown license. Detailed output of
     licensecheck in /home/pemensik/fedora/rawhide/2115560-DirectX-
     Headers/licensecheck.txt
[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.
[!]: 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]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[!]: 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]: 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]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[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]: 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).
[?]: Package functions as described.
[x]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[x]: The placement of pkgconfig(.pc) files are correct.
     Note: DirectX-Headers : /usr/lib64/pkgconfig/DirectX-Headers.pc
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[!]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %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]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: DirectX-Headers-1.606.3-1.fc39.noarch.rpm
          DirectX-Headers-1.606.3-1.fc39.src.rpm
============================================================ rpmlint session starts ============================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.10/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/tmpebobh76t')]
checks: 31, packages: 2

DirectX-Headers.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/DirectX-Headers/README.md
DirectX-Headers.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/DirectX-Headers/SECURITY.md
DirectX-Headers.noarch: E: noarch-with-lib64
DirectX-Headers.spec:34: W: libdir-macro-in-noarch-package (main package) %{_libdir}/libDirectX-Guids.a
DirectX-Headers.spec:35: W: libdir-macro-in-noarch-package (main package) %{_libdir}/pkgconfig/DirectX-Headers.pc
DirectX-Headers.noarch: W: files-duplicate /usr/include/wsl/stubs/rpc.h /usr/include/wsl/stubs/oaidl.h:/usr/include/wsl/stubs/ocidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12compatibility.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12sdklayers.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12shader.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12video.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3dcommon.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3dx12.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxcore.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxcore_interface.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxgicommon.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxgiformat.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/dxguids/dxguids.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/basetsd.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/oaidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/ocidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/rpc.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/rpcndr.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/unknwn.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/unknwnbase.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/winapifamily.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/wrl/client.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/wrl/implements.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/winadapter.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/wrladapter.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/lib64/libDirectX-Guids.a
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/DirectX-Headers.pc
DirectX-Headers.noarch: E: arch-independent-package-contains-binary-or-object /usr/lib64/libDirectX-Guids.a
============================ 2 packages and 0 specfiles checked; 2 errors, 31 warnings, 2 badness; has taken 0.2 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: 1

DirectX-Headers.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/DirectX-Headers/README.md
DirectX-Headers.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/DirectX-Headers/SECURITY.md
DirectX-Headers.noarch: E: noarch-with-lib64
DirectX-Headers.noarch: W: files-duplicate /usr/include/wsl/stubs/rpc.h /usr/include/wsl/stubs/oaidl.h:/usr/include/wsl/stubs/ocidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12compatibility.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12sdklayers.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12shader.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3d12video.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3dcommon.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/d3dx12.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxcore.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxcore_interface.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxgicommon.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/directx/dxgiformat.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/dxguids/dxguids.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/basetsd.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/oaidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/ocidl.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/rpc.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/rpcndr.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/unknwn.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/unknwnbase.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/winapifamily.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/wrl/client.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/stubs/wrl/implements.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/winadapter.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/include/wsl/wrladapter.h
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/lib64/libDirectX-Guids.a
DirectX-Headers.noarch: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/DirectX-Headers.pc
DirectX-Headers.noarch: E: arch-independent-package-contains-binary-or-object /usr/lib64/libDirectX-Guids.a
 1 packages and 0 specfiles checked; 2 errors, 29 warnings, 2 badness; has taken 0.1 s 



Source checksums
----------------
https://github.com/microsoft/DirectX-Headers/archive/v1.606.3/DirectX-Headers-1.606.3.tar.gz :
  CHECKSUM(SHA256) this package     : bf0183981e505336e918609374907c934b99eb61c0826d75a5649f41568abc4b
  CHECKSUM(SHA256) upstream package : bf0183981e505336e918609374907c934b99eb61c0826d75a5649f41568abc4b


Requires
--------
DirectX-Headers (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config



Provides
--------
DirectX-Headers:
    DirectX-Headers
    pkgconfig(DirectX-Headers)



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

Comment 5 Petr Menšík 2023-03-11 23:05:10 UTC
There is also more recent version 1.608.2, which should be updated to. But main problems are unrelated.

[!]: Latest version is packaged.

Comment 6 Nicolas Chauvet (kwizart) 2024-02-27 17:35:52 UTC
I've packaged the project without looking at existing review https://dl.kwizart.net/review/DirectX-Headers.spec


This package obviously isn't noarch as it provides a static archive (compiled with gcc-c++). But as no debug information are extracted from static archives, there is a need to disable debug_package (and even prevent strip from touching the produced static archives).

About -devel/-static sub-packages, it's a moot situation IMHO:
This project relies on libd3d12.so/libd3d12core.so/libdxcore.so libraries exposed by the WSL2 sub-system by Windows as a special directory (/usr/lib/wsl/lib) to the given Linux userspace.
This DirectX-Headers project, hence, doesn't provided theses shared libraries implementation at all and the static archives aren't even a minimal version of theses. So this projects hardly fall into our category of a -devel (and -static) sub-package, as it will miss the implementation of the library itself. Instead, it provides a stub loader via the static archives.
So it's not even a headers only package.

So to sum-up:
- Not a -devel because it misses a proper implementation in a library
- Not a -static because it wouldn't used to distinguishes between shared/static library and the former is missing.
- Not even a headers only because it provides a arched static archive.

My point would be to just package the project as-is and acknowledge that it doesn't fall in our previous categories. I would like to also point that this is clearly a development package and it shouldn't ends in end-users system by any means, so this question has very little impact.


On my side, I'm still at a testing point about WSL2. Here a short todo:
- To verify if lto is relevant here (is disabled in mesa still today).
- others components to leverage wsl2 support.
- Compare ubuntu 22.04 support
- Documentation of the process to migrate from koji generated vanilla docker image to a WSL2 enabled counterpart.

Comment 7 Nicolas Chauvet (kwizart) 2024-02-27 17:47:09 UTC
FYI, my change for a d3d12 conditional before this package is allowed and the d3d12 support properly tested.
https://src.fedoraproject.org/rpms/mesa/pull-request/41


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