Bug 1917271 (qt6-qtbase) - Review Request: qt6-qtbase - Qt6 - QtBase components
Summary: Review Request: qt6-qtbase - Qt6 - QtBase components
Keywords:
Status: CLOSED RAWHIDE
Alias: qt6-qtbase
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: qt6-reviews
TreeView+ depends on / blocked
 
Reported: 2021-01-18 08:49 UTC by Jan Grulich
Modified: 2021-01-22 16:12 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-01-22 16:12:33 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Jan Grulich 2021-01-18 08:49:44 UTC
Spec URL: https://jgrulich.fedorapeople.org/qt6/qt6-qtbase/qt6-qtbase.spec
SRPM URL: https://jgrulich.fedorapeople.org/qt6/qt6-qtbase/qt6-qtbase-6.0.0-1.fc33.src.rpm
Description:
Qt is a software toolkit for developing applications.

This package contains base tools, like string, xml, and network
handling.

Fedora Account System Username: jgrulich

Comment 1 Neal Gompa 2021-01-18 14:43:47 UTC
Taking this review.

Comment 2 Neal Gompa 2021-01-18 14:57:05 UTC
> %global rpm_macros_dir %(d=%{_rpmconfigdir}/macros.d; [ -d $d ] || d=%{_sysconfdir}/rpm; echo $d)

This is irrelevant, we can just use %_rpmmacrodir, as that's defined in RHEL8 and newer, and backported to EPEL7.

> %if 0%{?rhel} < 9
> %ifarch %{ix86}
> %global no_sse2  1
> %endif
> %endif

This conditional is wrong. Fedora 29 updated the i686 baseline to include SSE2: https://fedoraproject.org/wiki/Changes/Update_i686_architectural_baseline_to_include_SSE2

This should be "%if 0%{?rhel} && 0%{?rhel} < 9".

> BuildRequires: clang >= 3.7.0

I'm pretty sure this is wrong, since Qt6 requires a C++17 compiler.

> %if 0%{?fedora} || 0%{?rhel} > 6

There's a bunch of these that can just be ripped out.

> %if 0%{?fedora} > 22

This can be dropped as well.

> %setup -q -n %{qt_module}-everywhere-src-%{version}
>
> # omit '-b .tell-truth-about-private-api' so it doesn't end up in installed files -- rdieter
> %patch1 -p1
>
> # Upstreamable patches
> %patch50 -p1 -b .version-check
> %patch51 -p1 -b .moc-macros
> %patch52 -p1 -b .qmake-lflags
> %patch53 -p1 -b .no-relocatable
> %patch54 -p1 -b .cxxflag.patch
> %patch55 -p1 -b .firebird
> %patch56 -p1 -b .mysql
> # omit -b .python3
> %patch57 -p1
> 
> %patch80 -p1 -b .use-wayland-on-gnome.patch
> 
> %patch90 -p1 -b .gcc11

Please just use "%autosetup -n %{qt_module}-everywhere-src-%{version} -p1"

> Name: Qt6
> Description: Qt6 Configuration
> Version: 6.0.0

This should probably use "Version: %{version}" for the generated pkgconfig file...

Comment 3 Jan Grulich 2021-01-20 07:42:20 UTC
Updated package:

Spec URL: https://jgrulich.fedorapeople.org/qt6/qt6-qtbase/qt6-qtbase.spec
SRPM URL: https://jgrulich.fedorapeople.org/qt6/qt6-qtbase/qt6-qtbase-6.0.0-1.fc33.src.rpm
Description:
Qt is a software toolkit for developing applications.

This package contains base tools, like string, xml, and network
handling.

Fedora Account System Username: jgrulich

Comment 4 Neal Gompa 2021-01-21 11:39:46 UTC
Review notes:

* Package is named correctly
* Package follows proper conventions for Qt packages
* Package builds and installs (when everything else is available...)
* No serious issues from rpmlint

Comment 5 Gwyn Ciesla 2021-01-21 14:19:43 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/qt6-qtbase


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