[Bug 1297215] Review Request: dnsdist - A highly DNS-, DoS- and abuse-aware loadbalancer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #2 from Sander Hoentjen <sander@xxxxxxxxxxx> ---
(In reply to Roman Tsisyk from comment #1)
> Thanks your for you spec!
Thanks for reviewing!
> 
> > #%license COPYING
> 
> License is mandatory for all new packages [1].
> Your ticket has been fixed in the upstream, please enable
> https://github.com/PowerDNS/pdns/pull/3202
> 
> [1]:
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
> 
Will fix this with the release of second alpha, expected soon
> --------------
> 
> > BuildRequires: systemd-units
> 
> => BuildRequires: systemd
> 
> systemd-units has been merged into systemd package [2].
> 
> [2]: https://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations
Yes but for EPEL 6 you need systemd-units right? So that works on al releases.
If I need to make it conditional I can of course.
> 
> --------------
> 
> > # install systemd unit file
> > %{__install} -D -p -m 644 contrib/%{name}.service %{buildroot}%{_unitdir}/%{name}.service
> 
> + Requires(post): systemd
> + Requires(preun): systemd
> + Requires(postun): systemd
> +
> + %post
> + %systemd_post %{name}.service
> +
> + %preun
> + %systemd_preun %{name}.service
> +
> + %postun
> + %systemd_postun_with_restart %{name}.service
> 
> [3] https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#New_Packages
> 
Will fix
> --------------
> 
> > %{_mandir}/man1/%{name}.1.gz
> 
> > When installing man pages, note that they should be installed uncompressed as the build system will compress them as needed. The compression method may change, so it is important to reference the pages in the %files section with a pattern that takes this into account [4]
> 
> [4]: https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages
> 
> >  /usr/bin/install -c -m 644 dnsdist.1 '/builddir/build/BUILDROOT/dnsdist-1.0.0-0.1.alpha1.fc24.x86_64/usr/share/man/man1'
> 
Ok, will fix
> --------------
> 
> dnsdist-1.0.0-alpha1/html/js/jquery-1.8.3.min.js
> dnsdist-1.0.0-alpha1/html/js/jsrender.js
> dnsdist-1.0.0-alpha1/html/js/moment.min.js
> dnsdist-1.0.0-alpha1/html/js/purl.js
> dnsdist-1.0.0-alpha1/html/js/rickshaw.min.js
> dnsdist-1.0.0-alpha1/html/js/underscore-min.js
> ...
> 
> Obfuscated (=compiled) JavaScript looks like a binary for me:
> 
> > When you encounter prebuilt binaries in a package you MUST:
> >
> > Remove all pre-built program binaries and program libraries in %prep prior to the building of the package.
> > Ask upstream to remove the binaries in their next release. [5]
> 
> [5]
> https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-
> built_binaries_or_libraries
> 
> I'm not so experienced with Fedora packaging yet, but some other distros
> (you know) blame for all these *.min.js VERY strongly, so upstream might
> have problems with other packages as well. Please correct me if I'm wrong.

I will look into this
> 
> --------------
> 
> %dir %{_sysconfdir}/%{name}/
> 
> It would be nice to include an example of configuration file.
> 
Ok, will do that
> --------------
> 
> Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
> dnsdist-debuginfo
> 
> --------------
> 
> > -I/builddir/build/BUILD/dnsdist-1.0.0-alpha1/ext/mbedtls/include
> 
> mbedtls.i686 : Light-weight cryptographic and SSL/TLS library
> mbedtls.x86_64 : Light-weight cryptographic and SSL/TLS library
> mbedtls-utils.x86_64 : Utilities for mbedtls
> mbedtls-static.i686 : Static files for mbedtls
> mbedtls-static.x86_64 : Static files for mbedtls
> mbedtls-devel.i686 : Development files for mbedtls
> mbedtls-devel.x86_64 : Development files for mbedtls
> mbedtls-doc.noarch : Documentation files for mbedtls
> 
ok, will look into unbundling
> --------------
> 
> ^I--disable-silent-rules \$
>   --enable-dnscrypt \$
>   --enable-libsodium \$
> ^I--with-lua \$
> 
> (`:set list` in vim)
Good catch
> --------------
> 
> > Summary: A highly DNS-, DoS- and abuse-aware loadbalancer
> 
> Remove the "A" article from Summary, it looks better in listings without.
> It is not mandatory and is not mentioned in the guidelines, but I was
> noticed about that couple times by Zbigniew Jędrzejewski-Szmek.
> 
ok
> --------------
> 
> Please try to use koji to ensure that package compiles on all supported
> architectures:
> koji build --scratch rawhide pdns-4.0.0-0.1.alpha1.fc24.src.rpm
> 
will do
> --------------
> 
> What the difference between https://github.com/PowerDNS/pdns and this
> software?
> Shouldn't dnsdist to be added as a subpackage of existing pdns package?
> 
dnsdist lives in the pdns tree, but it is a fully seperate thing.
https://www.powerdns.com/nluug/2015%20nluug%20powerdns%20dnsdist.pdf
"Open source, vendor neutral - it is not “PowerDNS Dist”"
> --------------
> 
> It seems that pdns packages also uses yahttp.
> Probably yahttp needs their own package.
Will look into this
> 
> --------------
> 
> License and unclear relation with existing pdns package are major problems
> here.
Hope I have resolved these, will try to have a new spec ready soon.

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]