Quick notes.

Error: Transaction check error:
  file /usr/lib64/httpd/modules/mod_http2.so from install of mod_http2-1.10.0-1.fc25.remi.x86_64 conflicts with file from package httpd-2.4.25-1.fc25.x86_64

What is the need of this package if already provided by httpd ?


- compatibility macro for htpd 2.2 are obviously unneeded (IIRC only _httpd_mmn is need at buildtime)

- latest version is 1.10.1

- LICENSE file must be packaged

- why is there versioned .so, obviously not wanted

- why no LoadModule directive for mod_proxy_httpd2 ?

- %changelog need to be fixed (with your name/address)

Because upstream mod_http2 moves a bit faster than httpd releases, we're planning to package it separately and drop it from httpd.

Thank you Remi, for your review.

(In reply to Remi Collet from comment #1)
> Quick notes.
> 
> Error: Transaction check error:
>   file /usr/lib64/httpd/modules/mod_http2.so from install of
> mod_http2-1.10.0-1.fc25.remi.x86_64 conflicts with file from package
> httpd-2.4.25-1.fc25.x86_64
> 
> What is the need of this package if already provided by httpd ?

Joe decided, that it would be better to have mod_http2 as separate module, since upstream is producing releases more often, than httpd releases. I knew about this problem, so I will will add Requires tag, which will require httpd >= than version, which will not contain mod_http2 (I have to remove it from httpd).

> 
> - compatibility macro for htpd 2.2 are obviously unneeded (IIRC only
> _httpd_mmn is need at buildtime)
- you are right, I forgot to remove them from SPEC file. Fixed.

> 
> - latest version is 1.10.1
- they released this version yesterday, after I made this package... Fixed.

> 
> - LICENSE file must be packaged
- they are missing LICENCE file in release tar.gz. Reported to upstream. https://github.com/icing/mod_h2/issues/136

> 
> - why is there versioned .so, obviously not wanted
- fixed

> 
> - why no LoadModule directive for mod_proxy_httpd2 ?
- fixed

> 
> - %changelog need to be fixed (with your name/address)
- fixed


You can download updated SPEC and SRPM here:
Spec URL: https://luhliarik.fedorapeople.org/mod_http2.spec
SRPM URL: https://luhliarik.fedorapeople.org/mod_http2-1.10.1-1.fc23.src.rpm

(In reply to Joe Orton from comment #2)
> Because upstream mod_http2 moves a bit faster than httpd releases, we're
> planning to package it separately and drop it from httpd.

Thanks for clarification.


> echo "LoadModule http2_module modules/mod_http2.so" > %{buildroot}%{_sysconfdir}/httpd/conf.d/10-h2.conf
> echo "LoadModule proxy_http2_module modules/mod_proxy_http2.so" > %{buildroot}%{_sysconfdir}/httpd/conf.d/10-proxy_h2.conf

Shouldn't this be in conf.modules.d ?
So using %_httpd_modconfdir

Please also use %_httpd_moddir instead of harcoded path

Hardcoded paths have been substituted with macros (%_httpd_moddir and %_httpd_moddir)

You can download updated SPEC and SRPM here:
Spec URL: https://luhliarik.fedorapeople.org/mod_http2.spec
SRPM URL: https://luhliarik.fedorapeople.org/mod_http2-1.10.1-1.fc23.src.rpm

Created attachment 1270745 [details]
review.txt

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1440780
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++

Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines

[x]: Package does not generate any conflict.
	Expected, as this module is going to be removed from httpd
	(package split)
	=> if possible add Conflicts: httpd < ...

[!]: %build honors applicable compiler flags or justifies otherwise.
	Please use 
		%configure
		make %{?_smp_mflags} V=1

[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required

[!]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required

+Conflicts:      httpd < 2.4.25-8

[x]: Package does not generate any conflict.

-%configure
+%configure \
+  --libdir=%{_libdir} \
+  --prefix=%{_prefix}

[x]: %build honors applicable compiler flags or justifies otherwise.

-rm -rf %{buildroot}

[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the

-%clean
-rm -rf %{buildroot}
-

[x]: Package has no %clean section with rm -rf %{buildroot} (or


No more blocker, so approved.

BTW, --libdir and --prefix unneeded (already part of %configure)

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/mod_http2

mod_http2-1.10.1-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8f2df0e3f2

mod_http2-1.10.1-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-8f2df0e3f2

mod_http2-1.10.5-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8f2df0e3f2

mod_http2-1.10.5-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-8f2df0e3f2

mod_http2-1.10.5-1.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.