[Bug 683463] Review Request: trafficserver - Apache Traffic Server

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

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

Ed Marshall <esm+redhat@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |esm+redhat@xxxxxxxxx

--- Comment #1 from Ed Marshall <esm+redhat@xxxxxxxxx> 2011-03-09 11:54:12 EST ---
Hi, this is a completely unofficial review, but I'd actually like to see
trafficserver in Fedora. :)

The first thing I noticed: you're packaging the unstable version of
trafficserver (2.1.6) rather than the stable release from September (2.0.1).
Upstream doesn't seem to have confidence that 2.1.6 is ready to be declared
stable yet; should Fedora be overriding them?

rpmlint on the SRPM reports a few errors:

trafficserver.src: W: name-repeated-in-summary C TrafficServer
trafficserver.src: E: description-line-too-long C Apache Traffic Server is
fast, scalable and extensible HTTP/1.1 compliant caching proxy server.
trafficserver.src: E: description-line-too-long C Formerly a commercial
product, Yahoo! donated it to the Apache Foundation, and is now an Apache TLP.
trafficserver.src: W: no-version-in-last-changelog
trafficserver.src: W: invalid-license Apache-2.0
trafficserver.src:11: W: hardcoded-packager-tag Zhao
trafficserver.src:30: W: rpm-buildroot-usage %build echo  $RPM_BUILD_ROOT
trafficserver.src:52: W: macro-in-comment %attr
trafficserver.src:17: W: mixed-use-of-spaces-and-tabs (spaces: line 17, tab:
line 3)

So, some simple changes I'd suggest to make it happier:

- Make the summary more descriptive; Name: tells us what it is, Summary: tells
us what it does (and Description: expands on that). See:
http://fedoraproject.org/wiki/Packaging/Guidelines#Summary_and_description

- Wrap the description field at 80 characters or less. Again, think about what
the package does, rather than it's history.

- Add the applicable package versions to the changelog "*" lines. See:
http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

- Change license field to "ASL 2.0". See:
http://fedoraproject.org/wiki/Licensing:Main#Documentation_Licenses

- Remove the Packager: tag. (We get credit in the changelog. :) See:
http://fedoraproject.org/wiki/Packaging/Guidelines#Rpmlint_Errors

- Take out the "echo $RPM_BUILD_ROOT" from the spec; this looks like debugging
leftovers?

- Line 17 of the spec has tabs instead of spaces.


rpmlint on the binary you built brings up a few more issues (I've trimmed out
the repeats):

trafficserver.x86_64: W: no-documentation
trafficserver.x86_64: E: non-standard-dir-perm /usr/lib64/trafficserver/plugins
0644L
trafficserver.x86_64: W: devel-file-in-non-devel-package /usr/include/ts/ts.h
trafficserver.x86_64: E: non-standard-dir-perm /usr/lib64/trafficserver 0644L
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/include/ts/experimental.h
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/trafficserver/libtsmgmt.so
trafficserver.x86_64: E: non-standard-dir-perm /usr/include/ts 0644L
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/trafficserver/libtsutil.so
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/trafficserver/libtsutil.a
trafficserver.x86_64: W: hidden-file-or-dir
/etc/trafficserver/body_factory/default/.body_factory_info
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/trafficserver/libtsmgmt.a
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/include/ts/mgmtapi.h
trafficserver.x86_64: W: devel-file-in-non-devel-package
/usr/include/ts/remap.h
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
trafficserver.x86_64: W: no-manual-page-for-binary traffic_shell
trafficserver.x86_64: W: no-manual-page-for-binary tsxs
trafficserver.x86_64: W: no-manual-page-for-binary traffic_server
trafficserver.x86_64: W: no-manual-page-for-binary traffic_manager
trafficserver.x86_64: W: no-manual-page-for-binary traffic_line
trafficserver.x86_64: W: no-manual-page-for-binary traffic_sac
trafficserver.x86_64: W: no-manual-page-for-binary traffic_cop
trafficserver.x86_64: W: no-manual-page-for-binary traffic_logstats
trafficserver.x86_64: W: no-manual-page-for-binary traffic_logcat
trafficserver.x86_64: E: init-script-without-chkconfig-postin
/etc/init.d/trafficserver
trafficserver.x86_64: E: init-script-without-chkconfig-preun
/etc/init.d/trafficserver
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
trafficserver.x86_64: E: no-chkconfig-line /etc/init.d/trafficserver
trafficserver.x86_64: E: subsys-not-used /etc/init.d/trafficserver

There's quite a bit here, but the basics are:

- no man pages or documentation; you'll really want to work out the conflicts
with the man pages, and I'd suggest adding a %doc section for the README and
other documentation files in the distribution.

- file and directory permissions need a bit of cleaning up; specifically,
"%defattr(-,root,root,-)" is probably more appropriate. See:
http://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership

- devel files need to be broken out into a separate -devel package; see:
http://fedoraproject.org/wiki/Packaging/Guidelines#Devel_Packages

- you'll want to add a logrotate configuration; see "httpd" as a good example.

- init scripts should be enabled and disabled via postin/preun scripts via
chkconfig. See: http://fedoraproject.org/wiki/Packaging/Guidelines#Scriptlets


Getting into build issues, I tried building the package with mock, and it
failed:

libtool: install: /usr/bin/install -c .libs/traffic_sac
/builddir/build/BUILDROOT/trafficserver-2.1.6-1.i386/usr/bin/traffic_sac
/usr/bin/install -c -d -o nobody -g nobody
/builddir/build/BUILDROOT/trafficserver-2.1.6-1.i386/usr/lib/trafficserver/plugins
/usr/bin/install: cannot change owner and permissions of
`/builddir/build/BUILDROOT/trafficserver-2.1.6-1.i386/usr/lib/trafficserver/plugins':
Operation not permitted
make[3]: *** [install-exec-local] Error 1
make[3]: Leaving directory
`/builddir/build/BUILD/trafficserver-2.1.6-unstable/proxy'
make[2]: *** [install-am] Error 2
make[2]: Leaving directory
`/builddir/build/BUILD/trafficserver-2.1.6-unstable/proxy'
make[1]: *** [install-recursive] Error 1
make[1]: Leaving directory
`/builddir/build/BUILD/trafficserver-2.1.6-unstable/proxy'
make: *** [install-recursive] Error 1


That should give you a start on things that will need to be done; mainly, I'd
try to get to the point where rpmlint isn't complaining about anything (or you
understand why it's complaining), and it builds smoothly in mock. You might
want to read over http://fedoraproject.org/wiki/Packaging:ReviewGuidelines for
a list of some of things a more official review will end up looking at.

Hope this helps!

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
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]