Bug 683463 - (trafficserver) Review Request: trafficserver - Apache Traffic Server
Review Request: trafficserver - Apache Traffic Server
Status: CLOSED DUPLICATE of bug 787020
Product: Fedora EPEL
Classification: Fedora
Component: Package Review (Show other bugs)
el6
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
http://trafficserver.apache.org/
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-09 08:34 EST by Zhao Yongming
Modified: 2012-02-02 17:34 EST (History)
10 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-02-02 17:34:21 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tremble: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Zhao Yongming 2011-03-09 08:34:37 EST
Spec URL: http://zymlinux.net/trafficserver/rpm/trafficserver.spec
SRPM URL: http://zymlinux.net/trafficserver/rpm/trafficserver-2.1.6-1.src.rpm
Description: Apache Traffic Server is fast, scalable and extensible HTTP/1.1 compliant caching proxy server.
Formerly a commercial product, Yahoo! donated it to the Apache Foundation, and is now an Apache TLP.
site: http://trafficserver.apache.org/
version 2.1.6 is the most stable+useable release.
Comment 1 Ed Marshall 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!
Comment 2 Zhao Yongming 2011-03-11 07:36:22 EST
yeah, thanks for points out so many issues, I have put up my modified rpm in the same location (same name):

cutdown rpmlint reporting to 3warning:
  [root@ts1 SPECS]# rpmlint /usr/src/redhat/RPMS/x86_64/trafficserver-2.1.6-1.x86_64.rpm
  trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
  trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
  trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
  1 packages and 0 specfiles checked; 0 errors, 3 warnings.
  [root@ts1 SPECS]# rpmlint ../SRPMS/trafficserver-2.1.6-1.src.rpm 
  trafficserver.src:34: W: rpm-buildroot-usage %build echo  $RPM_BUILD_ROOT
  trafficserver.src:51: E: use-of-RPM_SOURCE_DIR
  trafficserver.src: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 3)
  1 packages and 0 specfiles checked; 1 errors, 2 warnings.
  [root@ts1 SPECS]# rpmlint ../RPMS/x86_64/trafficserver-devel-2.1.6-1.x86_64.rpm 
  trafficserver-devel.x86_64: W: no-documentation
  1 packages and 0 specfiles checked; 0 errors, 1 warnings.
  [root@ts1 SPECS]# 
my comment:
  hidden-file is really the trafficserver config
  logrotating, trafficserver do strict log rotating and very carefull of the disk usage, that is by design.
  service-default-enabled, not started in any runlevel by default
  se-of-RPM_SOURCE_DIR, how to avoid this error? I have no idea, as httpd is doing the same too.


- why 2.1.6 unstable but not 2.0.1 stable?
  from my point, we should use v2.1.6 other than v2.0.1, we are more confidence in v2.1.6 indeed.
  it will be the pre release of v3.0.

- Take out the "echo $RPM_BUILD_ROOT" from the spec; this looks like debugging
leftovers?
  that is because of our unstable release suffix in package name, may be removed in V3.0 stable.

It will need some more work on tracking all requirement in official ReviewGuidelines, just put a update and we are working on it.
Comment 3 Jason Tibbitts 2011-03-12 21:32:16 EST
I happened to look at this package at random.

Please remember to increase release and generate a new RPM whenever you make changes.  I do not know which version of the package I happened to download.  However, it did not build for me in mock (x86_64, rawhide):

CoreUtils.cc: In static member function 'static void CoreUtils::find_stuff(StuffTest_f)':
CoreUtils.cc:546:258: error: 'coress.core_stack_state::pc' may be used uninitialized in this function [-Werror=uninitialized]
CoreUtils.cc:546:258: error: 'coress.core_stack_state::framep' may be used uninitialized in this function [-Werror=uninitialized]

Perhaps you could try a koji scratch build to verify that your package builds OK.

Please clear the whiteboard if providing a package which builds.
Comment 4 Zhao Yongming 2011-03-13 01:14:16 EST
that is a new bug? I haven't tested on recently FC release, the recent RHEL 5 release works for me.

issue confirmed, I have tested on my rawhide testing box too. have open the bug for upstream:
https://issues.apache.org/jira/browse/TS-705
Comment 5 Zhao Yongming 2011-03-19 12:59:51 EDT
I have update to 2.1.6-2, which fixed the building issue and other Fedora tweak.
http://yum.zymlinux.net/trafficserver/rpm/2.1.6-2/

passed x86_64 native building and i386 mock building.

and here is the rpmlint information
[root@unknown-10-62-163-x SPECS]# rpmlint /root/rpmbuild/RPMS/x86_64/trafficserver-devel-2.1.6-2.fc16.x86_64.rpm
trafficserver-devel.x86_64: I: enchant-dictionary-not-found en_US
trafficserver-devel.x86_64: W: no-documentation
trafficserver-devel.x86_64: W: no-manual-page-for-binary tsxs
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
[root@unknown-10-62-163-x SPECS]# rpmlint /root/rpmbuild/RPMS/x86_64/trafficserver-2.1.6-2.fc16.x86_64.rpm
trafficserver.x86_64: I: enchant-dictionary-not-found en_US
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_cop
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_shell
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/plugins/conf_remap.so
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_sac
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsmgmt.so.2.1.6
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_server
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logcat
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_manager
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_line
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logstats
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsutil.so.2.1.6
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
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 traffic_line
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_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: W: service-default-enabled /etc/init.d/trafficserver
1 packages and 0 specfiles checked; 0 errors, 22 warnings.

things I am still tracking:
1, static libary, the -devel package have .la file, from the Guideline that is not a good way, I am still investing how to avoid it with upstream.

2, extra platform testing, may need to find out others.

for things that will take into action:
1, v2.1.7 will be released out next Monday, I will catch it up.
2, most patch for Fedora tweak is not in upstream trunk, will push it after we have done the most reviews.

thanks
Comment 7 Zhao Yongming 2011-06-15 04:29:25 EDT
Bumped to new stable release 3.0.0, uploaded in http://zymlinux.net/trafficserver/rpm/3.0.0-2/

http://zymlinux.net/trafficserver/rpm/3.0.0-2/trafficserver-3.0.0-2.fc16.src.rpm
http://zymlinux.net/trafficserver/rpm/3.0.0-2/trafficserver-3.0.0-2.fc16.x86_64.rpm
http://zymlinux.net/trafficserver/rpm/3.0.0-2/trafficserver-devel-3.0.0-2.fc16.x86_64.rpm

and here is the rpmlint output:

[root@unknown-10-62-163-x SPECS]# rpmlint /root/rpmbuild/RPMS/x86_64/trafficserver-3.0.0-2.fc16.x86_64.rpm
trafficserver.x86_64: I: enchant-dictionary-not-found en_US
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_cop
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_shell
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsutil.so.3.0.0
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsmgmt.so.3.0.0
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/plugins/conf_remap.so
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_sac
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_server
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logcat
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_manager
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_line
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logstats
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
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 traffic_line
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_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: W: one-line-command-in-%postun /sbin/ldconfig
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
1 packages and 0 specfiles checked; 0 errors, 23 warnings.
[root@unknown-10-62-163-x SPECS]# rpmlint /root/rpmbuild/RPMS/x86_64/trafficserver-devel-3.0.0-2.fc16.x86_64.rpm
trafficserver-devel.x86_64: I: enchant-dictionary-not-found en_US
trafficserver-devel.x86_64: W: no-documentation
trafficserver-devel.x86_64: W: no-manual-page-for-binary tsxs
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
[root@unknown-10-62-163-x SPECS]# ls
distcache.spec  trafficserver-3.0.0.spec  trafficserver.spec
[root@unknown-10-62-163-x SPECS]# rpmlint /root/rpmbuild/SRPMS/trafficserver-3.0.0-2.fc16.src.rpm
trafficserver.src: I: enchant-dictionary-not-found en_US
trafficserver.src:34: W: macro-in-comment %patch9
trafficserver.src:35: W: macro-in-comment %patch10
trafficserver.src:36: W: macro-in-comment %patch1001
trafficserver.src:47: W: macro-in-comment %find_lang
trafficserver.src:47: W: macro-in-comment %{name}
trafficserver.src:68: W: macro-in-comment %attr
1 packages and 0 specfiles checked; 0 errors, 6 warnings.
[root@unknown-10-62-163-x SPECS]#
Comment 8 Jan-Frode Myklebust 2011-06-15 06:31:57 EDT
I'm interested in this for RHEL6/EPEL6, so that's were I'm testing now.

Can't you just delete all the noise from the specfile:

-# we need to deal with the -unstable suffix:
-#Patch9:                trafficserver-fix-wccp-support.patch
-#Patch10:       trafficserver_wccp_virtual_destructor.patch
-#Patch1001:     trafficserver_ssdtier.patch
-#%patch9 -p1 -b .patch9
-#%patch10 -p1 -b .patch10
-#%patch1001 -p1 -b .patch1001
-#%find_lang %{name}
-# man pages conflicts with man-pages-zh_CN-1.5
-#%attr(0644, root, root) /usr/share/man/man1/*

[janfrode@RHEL6 SPECS]$ rpmlint trafficserver.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.


I'm not seeing the unstripped-binary warnings on RHEL6, so there it's looking quite good:

$ rpmlint /home/janfrode/rpmbuild/SRPMS/trafficserver-3.0.0-2.oc3.src.rpm
trafficserver.src: I: enchant-dictionary-not-found en_US
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint /home/janfrode/rpmbuild/RPMS/x86_64/trafficserver-3.0.0-2.oc3.x86_64.rpm
trafficserver.x86_64: I: enchant-dictionary-not-found en_US
trafficserver.x86_64: W: incoherent-version-in-changelog 3.0.0-2 ['3.0.0-2.oc3', '3.0.0-2.oc3']
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
trafficserver.x86_64: W: one-line-command-in-%postun /sbin/ldconfig
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
1 packages and 0 specfiles checked; 0 errors, 5 warnings.
$ rpmlint /home/janfrode/rpmbuild/RPMS/x86_64/trafficserver-debuginfo-3.0.0-2.oc3.x86_64.rpm
trafficserver-debuginfo.x86_64: I: enchant-dictionary-not-found en_US
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint /home/janfrode/rpmbuild/RPMS/x86_64/trafficserver-devel-3.0.0-2.oc3.x86_64.rpm
trafficserver-devel.x86_64: I: enchant-dictionary-not-found en_US
trafficserver-devel.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 9 Jan-Frode Myklebust 2011-06-15 07:01:24 EDT
To fix the man-page conflict, maybe you can just exclude the 2-3 generically named man-pages, and keep the rest ?

mkdir -p $RPM_BUILD_ROOT/usr/share/man/man1
cp doc/man/*.1 $RPM_BUILD_ROOT/usr/share/man/man1/
rm -f $RPM_BUILD_ROOT/usr/share/man/man1/enable.1
rm -f $RPM_BUILD_ROOT/usr/share/man/man1/disable.1
rm -f $RPM_BUILD_ROOT/usr/share/man/man1/exit.1
Comment 10 Jan-Frode Myklebust 2011-06-15 07:26:33 EDT
It was suggested on #epel to rather rename these conflicting man-pages, and leave a not about it in README.fedora:

mkdir -p $RPM_BUILD_ROOT/usr/share/man/man1
cp doc/man/*.1 $RPM_BUILD_ROOT/usr/share/man/man1/
mv $RPM_BUILD_ROOT/usr/share/man/man1/enable.1 $RPM_BUILD_ROOT/usr/share/man/man1/ts-enable.1
mv $RPM_BUILD_ROOT/usr/share/man/man1/disable.1 $RPM_BUILD_ROOT/usr/share/man/man1/ts-disable.1
mv $RPM_BUILD_ROOT/usr/share/man/man1/exit.1 $RPM_BUILD_ROOT/usr/share/man/man1/ts-exit.1
cat <<EOF > README.fedora
The man-pages for enable, disable and exit was renamed to ts-enable, 
ts-disable and ts-exit to avoid conflicts with other man-pages.
EOF
Comment 11 Zhao Yongming 2011-06-16 03:41:45 EDT
http://zymlinux.net/trafficserver/rpm/3.0.0-3/

http://zymlinux.net/trafficserver/rpm/3.0.0-3/trafficserver-3.0.0-3.fc16.src.rpm
http://zymlinux.net/trafficserver/rpm/3.0.0-3/trafficserver-3.0.0-3.fc16.x86_64.rpm
http://zymlinux.net/trafficserver/rpm/3.0.0-3/trafficserver-devel-3.0.0-3.fc16.x86_64.rpm


updates the man-pages and cleanup spec file, thanks Jan-Frode Myklebust <janfrode@tanso.net>


[root@unknown-10-62-163-x rpmbuild]# rpmlint SRPMS/trafficserver-3.0.0-3.fc16.src.rpm RPMS/x86_64/trafficserver-devel-3.0.0-3.fc16.x86_64.rpm RPMS/x86_64/trafficserver-3.0.0-3.fc16.x86_64.rpm
trafficserver.src: I: enchant-dictionary-not-found en_US
trafficserver-devel.x86_64: W: no-documentation
trafficserver-devel.x86_64: W: no-manual-page-for-binary tsxs
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/plugins/conf_remap.so
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_shell
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_sac
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logstats
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsutil.so.3.0.0
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_server
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_line
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_logcat
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_cop
trafficserver.x86_64: W: unstripped-binary-or-object /usr/lib64/trafficserver/libtsmgmt.so.3.0.0
trafficserver.x86_64: W: unstripped-binary-or-object /usr/bin/traffic_manager
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
trafficserver.x86_64: W: manual-page-warning /usr/share/man/man1/config_clock.1.gz 53: warning: macro `..' not defined
trafficserver.x86_64: W: manual-page-warning /usr/share/man/man1/config_logging.1.gz 89: warning: macro `SS"custom' not defined (possibly missing space after `SS')
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
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: W: one-line-command-in-%postun /sbin/ldconfig
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
3 packages and 0 specfiles checked; 0 errors, 26 warnings.
[root@unknown-10-62-163-x rpmbuild]#
Comment 12 Zhao Yongming 2011-06-17 06:14:06 EDT
can someone give me some review and tell me what I need to do on this?

we have aready put TS on FreeBSD ports, I don't want Linux to be too slow


thanks guys.


and here is the mock testing:


[zym@unknown-10-62-163-x root]$ mock --rebuild rpmbuild/SRPMS/trafficserver-3.0.0-3.fc16.src.rpm 
INFO: mock.py version 1.1.10 starting...
State Changed: init plugins
INFO: selinux disabled
State Changed: start
INFO: Start(rpmbuild/SRPMS/trafficserver-3.0.0-3.fc16.src.rpm)  Config(fedora-rawhide-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-rawhide-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.10
INFO: Mock Version: 1.1.10
INFO: enabled root cache
INFO: root cache aged out! cache will be rebuilt
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: creating cache
State Changed: unlock buildroot
State Changed: setup
State Changed: build
INFO: Done(rpmbuild/SRPMS/trafficserver-3.0.0-3.fc16.src.rpm) Config(default) 11 minutes 45 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-rawhide-x86_64/result
State Changed: end
[zym@unknown-10-62-163-x root]$ ls /var/lib/mock/fedora-rawhide-x86_64/result
build.log  trafficserver-3.0.0-3.fc16.src.rpm               trafficserver-devel-3.0.0-3.fc16.x86_64.rpm
root.log   trafficserver-3.0.0-3.fc16.x86_64.rpm
state.log  trafficserver-debuginfo-3.0.0-3.fc16.x86_64.rpm
[zym@unknown-10-62-163-x root]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
trafficserver.src: I: enchant-dictionary-not-found en_US
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
trafficserver.x86_64: W: manual-page-warning /usr/share/man/man1/config_clock.1.gz 53: warning: macro `..' not defined
trafficserver.x86_64: W: manual-page-warning /usr/share/man/man1/config_logging.1.gz 89: warning: macro `SS"custom' not defined (possibly missing space after `SS')
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
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: W: one-line-command-in-%postun /sbin/ldconfig
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
trafficserver-devel.x86_64: W: no-documentation
trafficserver-devel.x86_64: W: no-manual-page-for-binary tsxs
4 packages and 0 specfiles checked; 0 errors, 15 warnings.
[zym@unknown-10-62-163-x root]$
Comment 13 Jan-Frode Myklebust 2011-06-17 06:37:38 EDT

Only thing I find strange currently is that you install the daemon binaries to /usr/bin instead of /usr/sbin/. Is that intentional ? Hmm.. I see the Gentoo layout used has defined "sbindir: ${exec_prefix}/sbin" -- so it's strange it's not putting anything there.

Also, it would probably be nice to restart ts on upgrades in %postun:

if [ $1 -eq 1 ] ; then
   /sbin/service trafficserver condrestart &>/dev/null || :
fi

I think it's starting to look quite good, and I think it would be great to get it into the package db as soon as possible to ease builds and also get more testers. Unfortunately I'm just an EPEL packager, don't know if my reviews counts for anything.
Comment 14 Daniel Walsh 2011-06-17 08:58:29 EDT
What is /etc/trafficserver/* 

Are these config files?  I am trying to write some SELinux policy and I see traffic_manager modifying content in this directory

/etc/trafficserver/log_hosts.config_2

Any reason these files are not in /var/lib/trafficserver?

/etc should be treated as readonly.
Comment 15 Jan-Frode Myklebust 2011-06-17 09:20:24 EDT
Yes, /etc/trafficserver/* are configfiles, but I believe they can be modified by the traffic_manager (or traffic_server?), so it might be good to make it an selinux optional if one wants to allow this.

Every time the configfiles are changed, the old version is rewritten to name.config_number+1.

The traffic_manager is also a cluster manager, so config changes on one server can automatically be replicated to the other servers.
Comment 16 Daniel Walsh 2011-06-17 13:46:34 EDT
So all files in /etc/trafficserver can be modified by the traffic_manager?
Comment 17 Jan-Frode Myklebust 2011-06-17 16:14:57 EDT
dwalsh: I replied on the users@trafficserver mailinglist. Hopefull the developers there are following the discussion and can chime in with more knowledge.
Comment 18 Zhao Yongming 2011-06-18 05:55:43 EDT
dwalsh: yes, all files in /etc/trafficserver should be modified by traffic_manager.
I am a trafficserver devel, that is why I request this pkg :D
Trafficserver is a property enterprise grade system before opensource, and the config management is one of the most powerful designs:
1, ts will backup each version of the config file, with a _XXX suffix, the default is set to keep 3 versions for each file
2, ts config file can be changed by many ways:
   2.1, edit directly(or by any means for common config management)
   2.2, changed by traffic_manager, triggered by commands such as traffic_line and traffic_shell
   2.3, changed by the Cluster management, with broadcast like protocol.
   2.4, changed by other config management function, such as config upload/rollback etc.
   2.5, the remote config management interface, port 8083

there are problems when we deal with trafficserver and the modern operation systems:
1, for security reason, traffics_manager process will drop root privileges to nobody. and the configs is located in /etc/trafficserver, but it must be set to nobody writable.
   this is a problem that we still have no idea how to fix in the following release, maybe the config system will be changed to something like config database etc.
2, many config files in /etc/trafficserver can be override by configs(ie: most files can be refined in records.config) and environment variables, the config may be lose the meaning.
   in the opensource release, we have dropped the most config environment variables, and even command options etc, we make the release file directory a httpd like layout, that is Gentoo layout for. and most config options in records.config is a hiden options. that will prevent most confusion for common users. that is the most things we have done to make it much easy for users to manage the big ship( you can config 500 options in the records.config file for now, after we have removed many functions in the past 2 years ).

for now, I don't think we have a quick solution to get all things settle. so I'd suggest we take it and give the devel team to get it improved. these cool features is very useful for ISP grade CDN operations, we'd like not get it removed blindly.

hopes the information not miss-leading
many thanks for you help!
Comment 19 Jan-Frode Myklebust 2011-06-18 10:38:22 EDT
Zhao: 

I would suggest you change ATS to use a dedicated user/group, instead of "nobody". Having configfiles owned by "nobody" could mean that other nobodies can change them, which sounds bad....

So, 

./configure --enable-layout=Gentoo --libdir=%{_libdir}/trafficserver --with-tcl=%{_libdir} --with-user traffic --with-group traffic


and in %pre:

getent group traffic >/dev/null || groupadd -r traffic
getent passwd traffic >/dev/null || useradd -r -g traffic -d / -s /sbin/nologin -c "Apache Traffic Server" traffic
Comment 20 Zhao Yongming 2011-06-22 04:25:51 EDT
Jan-Frode:

thanks for you help on owner this RPM.

from the IRC, devel team prefer the user ats:ats. and we need a dedicated uid/gid, as we will have a cluster env, when we don't want to make things mess if id changes.

thanks
Comment 21 Jan-Frode Myklebust 2011-06-22 07:48:31 EDT
I requested static numbers in #715266.

Zhao: Could you please create a FAS account so that we can try to find a fedora packaging sponsor for you?
Comment 22 Jan-Frode Myklebust 2011-06-23 04:27:59 EDT
Thanks for letting me take over this Zhao. I've created a webpage (inspired by your fancy pages) at http://blag.tanso.net/code/ats/ to continue this. Will now look for someone to review the package for me.

The changes in v3.0.0-4 is to add the dedicated ats/ats user/group, and do a conditional restart on upgrades.
Comment 24 Mark Chappell 2011-06-27 03:12:54 EDT
Issues:

* Rpmlint

 rpmlint SPECS/trafficserver.spec 
SPECS/trafficserver.spec:93: W: mixed-use-of-spaces-and-tabs (spaces: line 93, tab: line 3)
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

* Odd macro - not needed.

%define version 3.0.0

* Please consider renaming the patches, so they at least all contain trafficserver-
Comment 27 Rahul Sundaram 2011-06-27 11:33:07 EDT
No need to define buildroot anymore unless you are branching for EPEL 5 as well

BuildRoot:	%{_builddir}/%{name}-%{version}-root

Patches should have a comment indicating upstream status

http://fedoraproject.org/wiki/Packaging:PatchUpstreamStatus

I would prefer you use systemd native service file for Rawhide.
Comment 28 Rahul Sundaram 2011-06-27 11:35:10 EDT
Can remove the following as well for Fedora spec unless you want to keep the same spec for EPEL as well

%clean
rm -rf $RPM_BUILD_ROOT

http://fedoraproject.org/wiki/PackagingGuidelines#.25clean
Comment 29 Jan-Frode Myklebust 2011-06-27 15:54:32 EDT
Rahul: thanks for the comments! I do intend to use this specfile for EPEL also, and possibly all the way back to EPEL5 -- so I'll just leave a note in the specfile for now to indicate these should be removed or ifdef'ed out for newer EPEL/Fedoras.

Regarding the upstream status of the patches, I will check ASAP.

And providing systemd native service file sounds like a good idea, but I would prefer to finish this for EPEL initially before starting working on rawhide/fedora.
Comment 30 Zhao Yongming 2011-06-27 23:18:23 EDT
comment on the patches:
Patch2:		trafficserver-init_scripts.patch
this patch is for systemd and rhel5.x style fixing, it is not intend to submit to upstream for now. we may submit it after we have it stable after all.

Patch7:		trafficserver_make_install.patch
this patch is for mock building, fixing make install to run in unprivileged mock account, I think that is not a common case, it should not be passed to upstream.

Patch51:	trafficserver-cluster_interface_linux.patch
the problem this patch want to fix, is fixed in TS-845, the trunk. this patch is quick fix for linux, we can live with it.

FYI
Comment 31 Jan-Frode Myklebust 2011-06-29 16:01:41 EDT
uid:gid 176:176 has been reserved for ats in setup-2.8.35-1.fc16/rawhide, 
ref: bz#715266.
Comment 32 Mark Chappell 2011-06-30 14:32:33 EDT
 - = N/A
 / = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [/] Package is named according to the Package Naming Guidelines.
 [/] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [/] Package meets the Packaging Guidelines including the Language specific items
 [/] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested: https://koji.fedoraproject.org/koji/taskinfo?taskID=3169610
 [!] Rpmlint output:  (snipping the things to ignore)

$ rpmlint *.rpm
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
trafficserver-devel.x86_64: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging
trafficserver-devel.x86_64: W: spelling-error %description -l en_US apache -> Apache, apace

 [/] Package is not relocatable.
 [!] Buildroot is correct  ( Not needed if >= EL6 and >= F13 )
 [/] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [/] License field in the package spec file matches the actual license.
     License type: ASL 2.0
 [!] 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 %doc.
 [!] With any Subpackage installed the license must also be installed (this may
belong to another subpackage)
 [/] Spec file is legible and written in American English.
 [/] Sources used to build the package matches the upstream source, as provided
in the spec URL.

$ md5sum trafficserver-3.0.0.tar.bz2 SOURCES/trafficserver-3.0.0.tar.bz2 
343661b10a0d8425180438ae43af7b4d  trafficserver-3.0.0.tar.bz2
343661b10a0d8425180438ae43af7b4d  SOURCES/trafficserver-3.0.0.tar.bz2

 [/] Package is not known to require ExcludeArch
 [/] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [/] The spec file handles locales properly.
 [/] ldconfig called in %post and %postun if required.
 [/] Package must own all directories that it creates.
 [/] Package requires other packages for directories it uses.
 [/] Package does not contain duplicates in %files.
 [/] Permissions on files are set properly.
 [/] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT. ( Not
needed if >= EL6 and >= F13 )
 [/] Package consistently uses macros.
 [/] Package contains code, or permissible content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [/] Package uses nothing in %doc for runtime.
 [/] Header files in -devel subpackage, if present.
 [!] Static libraries in -static subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [/] Development .so files in -devel subpackage, if present.
 [/] Fully versioned dependency in subpackages, if present.
 [!] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [/] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [/] Latest version is packaged.
 [/] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [/] Reviewer should test that the package builds in mock.
     Tested through koji
 [/] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: fedora-rawhide
 [-] Package functions as described.
 [/] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [-] %check is present and the tests pass


=== COMMENTS ===

* Buildroot: (You're building for EPEL 5)
  Should be  %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

* You should be including the LICENSE file.

* Static Libraries
  You appear to be shipping static libraries in your devel subpackage.  static libraries are frowned upon unless you have a very good reason, and if you need them they should be in a separate -static subpackage

* Libtool Archives present
  Please don't include the .la files, just delete them.

* Hidden files...
  Does this need to be there ?
trafficserver.x86_64: W: hidden-file-or-dir /etc/trafficserver/body_factory/default/.body_factory_info

* Spelling mistakes
trafficserver-devel.x86_64: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging
trafficserver-devel.x86_64: W: spelling-error %description -l en_US apache -> Apache, apace

* No logrotate config - please include one
trafficserver.x86_64: W: log-files-without-logrotate /var/log/trafficserver

* It's not always considered advisable to start the service by default once the package has been installed
trafficserver.x86_64: W: service-default-enabled /etc/init.d/trafficserver
Comment 33 Jan-Frode Myklebust 2011-06-30 16:29:41 EDT
Addressing your comments in reverse order:

* I don't understand why rpmlint claims it's starting the service by default. Please check the initscript at http://blag.tanso.net/code/ats/v3.0.0-6/trafficserver.init for what's installed. Both the chkconfig and INIT INFO looks  disabled by default to me.

* The traffic server has it's internal mechanism for log rotation, so logrotate isn't appropriate.

* Spelling mistakes fixed.

* Hidden files -- yes, these should be there. Ref: http://blag.tanso.net/code/ats/v3.0.0-6/body_factory.README

* Removed .la.

* Removed static libs, and left comment about these needing to go to -static
  if we want to include them at some later point.

* Set your suggested buildroot.

Updated specfile at http://blag.tanso.net/code/ats/v3.0.0-6/trafficserver.spec
Updated srpm: http://blag.tanso.net/code/ats/v3.0.0-6/trafficserver-3.0.0-6.el6.src.rpm
Comment 34 Zhao Yongming 2011-07-15 04:06:04 EDT
the bugfixing stable release v3.0.1 will be released in days, can we get the review process done at the time of next release?
Comment 36 Bill McGonigle 2011-10-28 01:04:20 EDT
I'm new to trafficserver - figured I'd start with this (on el6).  Thanks for packaging!

A few notes:

1) I built this SRPM on a machine called 'admin', copied the RPM over to a machine called 'traffic', and when I installed the RPM, records.config had:

  CONFIG proxy.config.proxy_name STRING admin.bfccomputing.com

It would be cool if a scriptlet could handle this automatically on install, but failing that the buildhost shouldn't be included in the RPM's default config file.

2) Also would be nice is a 'reload' option in the init script.  I think 'traffic_line -x' does this.

3) Perhaps Redhat-ify config options, such as:

 CONFIG proxy.config.ssl.server.cert.path STRING /etc/pki/tls/certs
 CONFIG proxy.config.ssl.server.private_key.path STRING /etc/pki/tls/private

That's all for now - I have a functional reverse proxy that seems to be working quite well!
Comment 37 Jan-Frode Myklebust 2011-10-28 04:25:10 EDT
Thanks for the feedback Bill! I'll look into implementing your suggestions.


Tremble: any hope on progress on this review?
Comment 38 Jan-Frode Myklebust 2011-12-08 03:41:37 EST
Updated to upstream release v3.0.2. Also added a small patch to correct condrestart() in initscript (fixed in v3.1).

http://blag.tanso.net/code/ats/v3.0.2-0/trafficserver.spec
http://blag.tanso.net/code/ats/v3.0.2-0/trafficserver-3.0.2-0.el6.src.rpm
Comment 39 Jan-Frode Myklebust 2012-02-02 17:34:21 EST

*** This bug has been marked as a duplicate of bug 787020 ***

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