Bug 523540 - Review Request: opentracker - BitTorrent Tracker
Review Request: opentracker - BitTorrent Tracker
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dominic Hopf
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-15 17:11 EDT by Simon
Modified: 2011-01-26 01:46 EST (History)
11 users (show)

See Also:
Fixed In Version: 0.28-4.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-01-26 01:46:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dmaphy: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)
build log (11.58 KB, application/octet-stream)
2009-11-09 16:00 EST, Simon
no flags Details
opentracker source RPM (58.69 KB, audio/x-pn-realaudio-plugin)
2010-04-09 04:05 EDT, Romain Wartel
no flags Details
Updated opentracker 0-0.9.20100410cvs RPM (59.53 KB, audio/x-pn-realaudio-plugin)
2010-04-09 06:20 EDT, Romain Wartel
no flags Details
opentracker 0-0.11.20100414cvs package (58.72 KB, audio/x-pn-realaudio-plugin)
2010-04-14 08:57 EDT, Romain Wartel
no flags Details
opentracker 0-0.11.20100414cvs tarball (54.91 KB, application/x-gzip)
2010-04-14 08:57 EDT, Romain Wartel
no flags Details
opentracker 0-0.11.20100414cvs ipv4 x86_64 binary RPM (built on EL5) (40.21 KB, audio/x-pn-realaudio-plugin)
2010-04-14 08:58 EDT, Romain Wartel
no flags Details

  None (edit)
Description Simon 2009-09-15 17:11:34 EDT
Spec URL:
http://cassmodiah.fedorapeople.org/opentracker/opentracker.spec

SRPM URL:
http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.1.20090915cvs.fc11.src.rpm

DeESCRIPTION:
opentracker is a open and free bittorrent tracker project. It aims for minimal
resource usage and is intended to run at your wlan router. Currently it is 
deployed as an open and free tracker instance

RPMLINT:
opentracker.i586: E: statically-linked-binary /usr/bin/opentracker
The package installs a statically linked binary or object file.  Usually this
is a packaging bug. If not, contact your rpmlint distributor about this so
that this error gets included in the exception file for rpmlint and will not
be flagged as a packaging bug in the future (or add it to your local
configuration if you installed rpmlint from the source tarball).

opentracker.i586: W: executable-stack /usr/bin/opentracker
The binary declares the stack as executable.  Executable stack is usually an
error as it is only needed if the code contains GCC trampolines or similar
constructs which uses code on the stack.  One common source for needlessly
executable stack cases are object files built from assembler files which don't
define a proper .note.GNU-stack section.

3 packages and 0 specfiles checked; 1 errors, 1 warnings.
Comment 1 Simon 2009-09-15 17:16:33 EDT
please do not start a review. this package need some more love.
i will do this...
Comment 2 Peter Lemenkov 2009-09-16 05:34:53 EDT
This is a Really Great addition to Fedora!
Comment 3 Till Maas 2009-09-16 17:10:58 EDT
(In reply to comment #1)
> please do not start a review. this package need some more love.
> i will do this...  

Please remove NotReady from the Whiteboard once it is ready.
Comment 5 Peter Lemenkov 2009-09-30 00:46:30 EDT
Few notes:

* Provide the exact way of how to build Source0. E.g. don't use only cvs co, but
cvs co -D "2009-09-16" (from the cvs man - "use the most recent revision no later than given date") or something similar. I advice you to describe the whole process in the following way:

# cvs -d:pserver:anoncvs@cvs.erdgeist.org:/home/cvsroot co -D "2009-09-15" -d opentracker-0 opentracker
# tar cjf opentracker-0.tar.bz2 opentracker-0
Source0:        %{name}-%{version}.tar.bz2

As a result, you won't need to add "-n %{name}" to %setup -q.

* I'm in doubts, whether this needed at all - please explain:
Provides: opentracker-static

Other things looks sane for me.
Comment 6 Simon 2009-10-01 02:43:31 EDT
* Provide the exact way of how to build Source0

K, I will add this



*please explain: Provides: opentracker-static

rpmlint:
opentracker.i586: E: statically-linked-binary /usr/bin/opentracker

I had an equal rpmlint-output at libowfat with dietlibc. 
These packages are very static.
I had to add "provides: libowfat-static" in the libowfat, there is a "provides dietlibc-static" in dietlibc, too.
So this would make sense (for me) to continue this procedure.
Comment 7 Matt Domsch 2009-10-15 22:59:02 EDT
opentracker isn't a library, it's an executable, and yes, it statically links against libowfat (libowfat is only available in a static libraray).  So that error can be ignored.  This situation is even recognized in the packaging guidelines for statically-linked executables, so it doesn't require FESCo approval.

This is configured as an open tracker, yes?  Meaning anyone can upload any torrent to it.  For Fedora Infrastructure use, I wouldn't want to put into production this kind of service - I would want to use the closed mode (which I know, the authors abhore).

Furthermore, I'd really like to see a second build, enabling the IPv6-only mode, be included.

It's too bad the upstream doesn't provide runtime config options for these two features (open vs closed mode, IPv4-only vs IPv6-only).  What you find is that you'd need 4 executables built to satisfy that matrix.

I've built libowfat for EL-5 today, linked against glibc and not dietlibc.  I'll go ahead and commit that.  Then I ask you to create an EL-5 branch for opentracker once your package here is approved.  For FI, I need IPv6-only, closed tracker mode.
Comment 8 Matt Domsch 2009-10-16 23:33:58 EDT
I updated the package to build both IPv4 and IPv6 binaries, with requisite duplicate config & init files.  I dropped the useless README_v6.  I also enabled a few features in both builds:
-DWANT_ACCESSLIST_WHITE
-DWANT_COMPRESSION_GZIP
-DWANT_RESTRICT_STATS

These are all that made sense IMHO.

http://domsch.com/linux/fedora/opentracker/

I also looked at using latest CVS HEAD, but that fails to build, complaining about not including <unistd.h> when it clearly does.  Not sure what the problem there is.
Comment 9 Simon 2009-10-17 11:51:30 EDT
-DWANT_ACCESSLIST_WHITE isn't a cool feature.
Imho it doesn't make sense to use ot with black- or whitelist.
(btw, your pkg missing a whitelist and won't work; a whitelist is required if you enable this 'feature')

-DWANT_COMPRESSION_GZIP - jeah, k this is a usefull feature
-DWANT_RESTRICT_STATS - not sure if this is usefull, but it won't be harmfull

http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.4.20090915cvs.fc11.src.rpm
Comment 10 Christoph Wickert 2009-10-17 16:16:52 EDT
(In reply to comment #6)
 
> I had to add "provides: libowfat-static" in the libowfat, there is a "provides
> dietlibc-static" in dietlibc, too.
> So this would make sense (for me) to continue this procedure.  

This only would make sense if there are things to build on top of opentracker, e.g. plugins for it. If not, I suggest to remove it to not blow up metadata and confuse yum more than necessary.
Comment 11 Dominic Hopf 2009-10-17 16:41:22 EDT
First of all, thanks very much to the other guys who also have had a look on this package. I'm very sorry for the delayed feedback from my site.

Here's the formal review now.


$ rpmlint opentracker.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint opentracker-0-0.4.20090915cvs.fc11.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint opentracker-debuginfo-0-0.4.20090915cvs.fc11.x86_64.rpm opentracker-ipv4-0-0.4.20090915cvs.fc11.x86_64.rpm opentracker-ipv6-0-0.4.20090915cvs.fc11.x86_64.rpm
opentracker-ipv4.x86_64: E: statically-linked-binary /usr/bin/opentracker-ipv4
opentracker-ipv4.x86_64: W: executable-stack /usr/bin/opentracker-ipv4
opentracker-ipv4.x86_64: W: service-default-enabled /etc/rc.d/init.d/opentracker-ipv4
opentracker-ipv6.x86_64: E: statically-linked-binary /usr/bin/opentracker-ipv6
opentracker-ipv6.x86_64: W: executable-stack /usr/bin/opentracker-ipv6
opentracker-ipv6.x86_64: W: incoherent-subsys /etc/rc.d/init.d/opentracker-ipv6 $prog
opentracker-ipv6.x86_64: W: service-default-enabled /etc/rc.d/init.d/opentracker-ipv6
3 packages and 0 specfiles checked; 2 errors, 5 warnings.

As Matt already wrote before, the statically-linked-binary error can be ignored.
I think those warnings are okay too so far.

MUSTs
-----

OK: packaged is named according to the package naming guidelines
OK: specfile name matches %{name}.spec
OK: package seems to meet packaging guidelines
OK: license in specfile matches actual license and meets licensing guidelines
OK: license file is included in %doc
	Note: there is no separate license file, but a license hint is contained in
	the README, which actually is included in %doc.
OK: specfile is written in AE
OK: specfile is legible
OK: sourcefile in the package is the same as provided in the mentioned source,
    Yes, it is. It is not possible to get a fitting md5sum, so I have had a look
    with meld on an own checkout and the unpacked tarball from the package. There
    wasn't any diff.
OK: package compiles successfully
OK: all build dependencies are listed in BuildRequires
N/A: package handles locales properly
     there are no locales installed with this package
N/A: call ldconfig in %post and %postun
     there is no shared library linked against the installed binary
OK: package is not designed to be relocatable
OK: package owns directorys it creates
OK: does not list a file more than once in the %files listing
OK: %files section includes %defattr and permissions are set properly
OK: %clean section is there and contains rm -rf %{buildroot}
OK: macros are consistently used
OK: package contains code
N/A: subpackage for large documentation files
     there are no large documentation files
OK: program runs properly without files listed in %doc
N/A: header files are in a -devel package
     there are no header files installed with this package
N/A: static libraries are in a -static package
     there are no static libs installed with this package
N/A: require pkgconfig if package contains a pkgconfig(.pc)
     there is no pkgconfig file
N/A: put .so-files into -devel package if there are library files with suffix
     there is no library with suffix, in fact there isn't any library
N/A: devel package includes fully versioned dependency for the base package
     there is no devel package
N/A: any libtool archives are removed
     there are no libtool archives
N/A: contains desktop file if it is a GUI application
     this is no GUI application
OK: package does not own any files or directories owned by other packages
OK: buildroot is removed at beginning of %install
N/A: filenames are encoded in UTF-8
     not necessary since there are no non-ASCII filenames


SHOULD
------
N/A: non-English translations for description and summary
     a localization is not neccessary for this package
OK: package builds in mock
NOT OK: package builds into binary rpms for all supported architectures
     does not build for ppc64
OK: program runs
N/A: subpackages contain fully versioned dependency for the base package
     there are no subpackages
N/A: pkgconfig file is placed in a devel package
     there is no pkgconfig file
N/A: require package providing a file instead of the file itself
     no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required

Building the package in koji fails for ppc64 architecture.
You can have a look at the concerning build.log here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1751807
I think this is no problem for the packaging procedure at all since this is a
SHOULD, but you maybe want to contact upstream regarding this issue.

The description is not okay yet. According to http://www.bittorrent.com/ the
spelling is "BitTorrent", which is not correct in your description.
Also "wlan" is a term which is good known in Germany and maybe some other
countrys, but in general I think "Wi-Fi" would be a better term to fit world
wide. I would also consider "Wireless LAN" as okay.
Comment 12 Dominic Hopf 2009-10-17 17:10:53 EDT
Oh, and it would be nice if you also could fix the service-default-enabled warning by patching this out from the init script. Sorry for pointing this out a bit later.
Comment 13 Dominic Hopf 2009-10-17 17:16:23 EDT
Well, the opentracker daemon does not run after installing the packages, so please ignore my previous posting and stay at ignoring the warning.
Comment 14 Simon 2009-10-17 17:41:37 EDT
http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.5.20090915cvs.fc11.src.rpm

added "excludearch: ppc64", emailed infos to upstream
replaced wlan with wi-fi
Comment 15 Dominic Hopf 2009-10-17 18:07:12 EDT
Mentioned issues are fixed. The package builds in mock and koji now without any problems.

APPROVED.
Comment 16 Simon 2009-10-17 18:17:55 EDT
Thank you, Dominic,  for your review!

New Package CVS Request
=======================
Package Name: opentracker
Short Description: BitTorrent Tracker
Owners: cassmodiah
Branches: F-10 F-11 F-12
InitialCC:

New Package CVS Request
=======================
Package Name: opentracker
Short Description: BitTorrent Tracker
Owners: mdomsch
Branches: EL-5
InitialCC:
Comment 17 Matt Domsch 2009-10-17 18:41:37 EDT
Re WANT_ACCESSLIST_WHITE:  Without this in place, can't anyone cause any torrent to be tracked by this tracker?  Even torrents that you might not wish to have tracked? Or am I misunderstanding?
Comment 18 Matt Domsch 2009-10-17 18:46:59 EDT
Re closed mode:
http://erdgeist.org/arts/software/opentracker/

Closed mode

While personally I like my tracker to be open, I can see that there's people that want to control what torrents to track – or not to track. If you've compiled opentracker with one of the accesslist-options (see Build instructions above), you can control which torrents are tracked by providing a file that contains a list of human readable info_hashes. An example whitelist file would look like:

0123456789abcdef0123456789abcdef01234567
890123456789abcdef0123456789abcdef012345

To make opentracker reload it's white/blacklist, send a SIGHUP unix signal.
Comment 19 Matt Domsch 2009-10-17 19:23:26 EDT
I like the subpackage (-ipv4 and -ipv6) model.  That's clean.

Are two users/groups required?  I would think there would be a need for a single opentracker group and user, not two (one for IPv4 and one for IPv6).

subpackage Summary tags are misspelled.
Summary:         IPv4-verion of %{name}
Summary:         IPv6-verion of %{name}

Instead, let me recommend:

Summary: A high-performance torrent tracker using IPv4
Summary: A high-performance torrent tracker using IPv6

Likewise, the descriptions last sentence read like:
This is the ipv6-Version.

Instead, let me recommend:

This version uses IPv4 only.
This version uses IPv6 only.
Comment 20 Dominic Hopf 2009-10-17 19:48:00 EDT
(In reply to comment #19)
> subpackage Summary tags are misspelled.
> Summary:         IPv4-verion of %{name}
> Summary:         IPv6-verion of %{name}
> 
> Instead, let me recommend:
> 
> Summary: A high-performance torrent tracker using IPv4
> Summary: A high-performance torrent tracker using IPv6
> 
> Likewise, the descriptions last sentence read like:
> This is the ipv6-Version.
> 
> Instead, let me recommend:
> 
> This version uses IPv4 only.
> This version uses IPv6 only.  

Blame on me i didn't saw this in my review. I would appreciate Matts recommendations.
Comment 21 Matt Domsch 2009-10-17 23:08:55 EDT
Need to fix opentracker-ipv6 initscript.

# /sbin/service opentracker-ipv6 start
Starting opentracker-ipv4: /etc/init.d/opentracker-ipv6: line 36: /usr/bin/opentracker-ipv4: No such file or directory  [FAILED]
Comment 22 Matt Domsch 2009-10-17 23:10:58 EDT
and it's looking for the config file in the wrong place.

# /sbin/service opentracker-ipv6 start
Starting opentracker-ipv6: Warning: Can't open config file: /etc/opentracker/opentracker.conf.socket_bind6_reuse: Invalid argument
                                                           [FAILED]
Comment 23 Simon 2009-10-18 04:53:16 EDT
okay, I will correct typo and initscript before i include it in cvs.

Whitelist:
Freedom is to share whatever you want. The responsibility should be by the users, not by the admins. There should be no "supremacy" who control and restrict the users! I don't want import a tool that controls and restrict me, so i left it out!
Comment 24 Christoph Wickert 2009-10-18 05:01:40 EDT
(In reply to comment #23)
> okay, I will correct typo and initscript before i include it in cvs.
> 
> Whitelist:
> Freedom is to share whatever you want. The responsibility should be by the
> users, not by the admins. There should be no "supremacy" who control and
> restrict the users! 

I don't think this security concept will work out. Freedom also includes the freedom to protect yourself from damage.
Comment 25 Simon 2009-10-18 05:04:50 EDT
# /sbin/service opentracker-ipv6 start
Starting opentracker-ipv6: Warning: Can't open config file:
/etc/opentracker/opentracker.conf.socket_bind6_reuse: Invalid argument

aaaah!
i bound the conf in the deamon patch, i will change it to ipv4 and sed it before ipv6 built in ipv6 of course
Comment 26 Simon 2009-10-18 05:31:20 EDT
(In reply to comment #21)
> Need to fix opentracker-ipv6 initscript.
> 
> # /sbin/service opentracker-ipv6 start
> Starting opentracker-ipv4: /etc/init.d/opentracker-ipv6: line 36:
> /usr/bin/opentracker-ipv4: No such file or directory  [FAILED]  

aaah²

install -Dpm0755 %{SOURCE1} \
        %{buildroot}%{_initrddir}/%{name}-ipv4

install -Dpm0755 %{SOURCE2} \
        %{buildroot}%{_initrddir}/%{name}-ipv6

double used SOURCE1 before... damn!
Comment 27 Simon 2009-10-18 06:15:35 EDT
will work now!

matt, you are right, there stands "can" not "must"
Will include whitelist
Comment 29 Matt Domsch 2009-10-18 10:13:15 EDT
I haven't been able to get opentracker-ipv6 to actually run.

# opentracker-ipv6 
Parse error in config file: listen.tcp_udp [::0]
socket_bind6_reuse: Invalid argument


My /etc/opentracker-ipv6/opentracker-ipv6.conf file has:
listen.tcp_udp [::0]
access.whitelist ./whitelist
tracker.rootdir /srv/opentracker
tracker.redirect_url https://torrent.fedoraproject.org/

and specifying argument -i always fails too:

# opentracker-ipv6 -i '[::1]'
Usage: opentracker-ipv6 [-i ip] [-p port] [-P port] [-r redirect] [-d dir] [-A ip] [-f config] [-s livesyncport] [-w whitelistfile]

# opentracker-ipv6 -i '[::1]' -d /srv/opentracker -w /srv/opentracker/whitelist 
Usage: opentracker-ipv6 [-i ip] [-p port] [-P port] [-r redirect] [-d dir] [-A ip] [-f config] [-s livesyncport] [-w whitelistfile]



Any ideas why?  This is a showstopper IMHO.
Comment 30 Matt Domsch 2009-10-18 23:05:32 EDT
FWIW, opentracker-ipv6 works if built against a libowfat that is built against glibc (and not dietlibc).  I've spent several hours trying to get it to build against the libowfat and dietlibc in Fedora, and am failing miserably.  It builds fine, but it doesn't run fine.

socket_bind6_reuse() is failing (return -1, errno=0).  Using strace, I can see socket_bind6_reuse internally falling back to running socket_bind4() for some non-obvious reason, which then fails.

Building opentracker for EPEL, which doesn't use dietlibc, it succeeds.

 ./opentracker.debug 
Warning: Can't open config file: /etc/opentracker-ipv6/opentracker-ipv6.conf.
Binding socket type TCP to address [::]:6969... success.
Binding socket type UDP to address [::]:6969... success.
Warning: Can't open accesslist file: (null) (but will try to create it later, if necessary and possible).
PWD: /
Comment 31 Matt Domsch 2009-10-22 00:07:14 EDT
I'm putting this back under review and holding off on the CVS request until the above concerns are addressed.
Comment 32 Simon 2009-10-22 01:59:38 EDT
mh, iv4 works. so the problem will be easily solved after removing the non-mainstream ip-protocoll. what do you think?
Comment 33 Simon 2009-10-22 02:34:23 EDT
ipv4 works with the fedora libowfat.
if you say (after you tested it) ipv4 will work with your changes you made in epel. I will change the fedora libowfat, too.
If it won't work, I won't change the fedoralibowfat and drop ipv6
Comment 34 Simon 2009-10-22 03:49:12 EDT
okay, ipv4 works for me with libowfat which doesn't use dietlibc.
can you confirm it?
Comment 35 Simon 2009-10-24 06:13:18 EDT
Matt, please test it with one of these libowfat.
I hope it works for you with ipv6, it works for me with ipv4.

F-10
http://koji.fedoraproject.org/koji/taskinfo?taskID=1765960

F-11
http://koji.fedoraproject.org/koji/taskinfo?taskID=1765961

F-12
http://koji.fedoraproject.org/koji/taskinfo?taskID=1765963

F-13
http://koji.fedoraproject.org/koji/taskinfo?taskID=1765962

I can't test ipv6.
After it works for you too. I will update libowfat for f11 and higher.
Comment 36 Matt Domsch 2009-10-24 09:03:16 EDT
This is working much better now, thank you.  I installed your libowfat-devel package on my F11 box, and after building opentracker to link against this new version, I can start both the IPv4 and IPv6 trackers in parallel.

I have cleaned up the opentracker SPEC and Makefile patch accordingly, and have posted them at:

http://domsch.com/linux/fedora/opentracker/
(it's the packages with .md1 in the tag)

The Makefile now links in -lgcc to avoid a pthread_cancel error on exit:

# opentracker-ipv6
Parse error in config file: listen.tcp [::1]:6969
Warning: Can't open accesslist file: (null) (but will try to create it later, if necessary and possible).
PWD: /
^Clibgcc_s.so.1 must be installed for pthread_cancel to work
Aborted


In addition, I cleaned up the Description tags, and am using a single opentracker user (rather than one for each of ipv4 and ipv6 - I think 2 users is overkill).  I also fixed the rpmlint warnings (initscripts were set to start by default).

There is a functional problem with the opentracker code, which I have not debugged.  If you start opentracker without any config file, it default to listen on all addresses, both IPv4 and IPv6, even the IPv4-only and IPv6-only versions.  This is odd, but I think it may be expected behavior.  The argument to bind() in the case of no requested addresses is to use the all-zeros (INADDR_ANY), which can bind both protocols.

If you start the v4 tracker with -i ::1 (an IPv6 address), it will fail with an error.  If you start with -i 127.0.0.1 (an IPv4 address), it succeeds.

Likewise with the v6 tracker.  Using -i ::1, it works, -i 127.0.0.1, it fails (as one would expect).

But, I haven't figured out the syntax for putting IPv6 addresses in the /etc/opentracker-ipv6/opentracker-ipv6.conf file.  From the code, the config file _should_ be:

listen.tcp [::1]:6969

but that fails:

# opentracker-ipv6
Parse error in config file: listen.tcp [::1]:6969


If you can figure out the config file syntax and fix up the provided .conf files, that should take care of it all.

Thanks,
Matt
Comment 37 Matt Domsch 2009-10-24 09:20:49 EDT
does this also let you remove the ppc64 build exclude?
Comment 38 Fedora Update System 2009-10-24 11:42:11 EDT
libowfat-0.28-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libowfat-0.28-4.fc11
Comment 39 Fedora Update System 2009-10-27 03:06:12 EDT
libowfat-0.28-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 40 Simon 2009-11-09 16:00:00 EST
Created attachment 368288 [details]
build log
Comment 41 Simon 2009-11-09 16:11:35 EST
(In reply to comment #36)
> This is working much better now, thank you.  I installed your libowfat-devel
> package on my F11 box, and after building opentracker to link against this new
> version, I can start both the IPv4 and IPv6 trackers in parallel.
mh, okay, i think we made a mistake, as we build libowfat-devel against libc instead of dietlibc...
Take a look at the build.log. I'm no t a c guru and haven't found the error yet.
http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.7.20090915cvs.fc12.src.rpm

> The Makefile now links in -lgcc to avoid a pthread_cancel error on exit:
Don't realize why -lpthread doesn't do this trick, but okay
 
> But, I haven't figured out the syntax for putting IPv6 addresses in the
> /etc/opentracker-ipv6/opentracker-ipv6.conf file.  From the code, the config
> file _should_ be:
> 
> listen.tcp [::1]:6969
> 
> but that fails:
> 
> # opentracker-ipv6
> Parse error in config file: listen.tcp [::1]:6969
> 
> 
> If you can figure out the config file syntax and fix up the provided .conf
> files, that should take care of it all.
https://erdgeist.org/cvsweb/opentracker/opentracker.c.diff?r2=1.225&r1=1.224&f=u
Comment 42 Simon 2009-11-10 14:10:33 EST
ah, damn... i'm blind
will work with F10 and F11 because the -4 libowfat is in this releases...

matt, please test it!
Comment 43 Matt Domsch 2009-11-25 10:52:41 EST
One nit here: s/ipv4/ipv6/.

%preun ipv6
if [ $1 = 0 ] ; then
    /sbin/service %{name}-ipv4 stop >/dev/null 2>&1
    /sbin/chkconfig --del %{name}-ipv4
fi

I would not include README_v6 in the -ipv6 package.  It is obsolete, and removed from upstream.

it's listening on ports properly now, and I'm trying to set up a torrent to play with to be sure it's working as expected.
Comment 44 Matt Domsch 2009-11-25 10:57:45 EST
A fatal runtime crash noted by skvidal:

<skvidal> mdomsch: try hitting:
  http://torrenthost:torrentport/stats?mode=everything
<skvidal> mdomsch: if opentracker doesn't fall over then that's progresss
<mdomsch> whoops
<mdomsch> bang dead

$ wget -6 'http://[2001:4830:1602:1::2]:6969/stats?mode=everything'
--2009-11-25 09:55:17--  http://[2001:4830:1602:1::2]:6969/stats?mode=everything
Connecting to 2001:4830:1602:1::2:6969... connected.
HTTP request sent, awaiting response... No data received.
Retrying.

--2009-11-25 09:55:18--  (try: 2)  http://[2001:4830:1602:1::2]:6969/stats?mode=everything
Connecting to 2001:4830:1602:1::2:6969... failed: Connection refused.
[mdomsch@p opentracker]$ netstat -apn | grep 6969
(Not all processes could be identified, non-owned process info
 will not be shown, you would have to be root to see it all.)


yes, it's crashing when hitting that URL.  That'll have to be fixed before it can be released.
Comment 46 Romain Wartel 2010-03-28 18:00:45 EDT
FWIW, I took the RPM that Simon kindly prepared and rebuilt it against a RHEL5 variant (CERN Scientific Linux 5), with the CVS snapshot from 2010-03-25.

The tracker works as expected (and at least the ipv4 x86_64 binary I built does not suffer from the bug reported in Comment 44).

There is however an issue with the access list file (white list), enabled with the Makefile option "-DWANT_ACCESSLIST_WHITE". When opentracker is started, it states:

Warning: Can't open accesslist file: /foo/bar (but will try to create it later,
if necessary and possible).

The error occurs although /foo/bar exists, contains valid torrent hashes, and is readable by the opentracker user. This is preventing any torrent to be handled by the tracker in case "-DWANT_ACCESSLIST_WHITE" is used.

The error seems upstream and is reproducible in our environment with the current libowfat + opentracker CVS snapshots, as of 2010-03-25.

I will try to look at this if/when time permits, but would welcome any feedback in case anyone sees this.

Romain.

$Source: /home/cvsroot/opentracker/opentracker.c,v $: $Revision: 1.227 $
$Source: /home/cvsroot/opentracker/ot_accesslist.c,v $: $Revision: 1.28 $
$Source: /home/cvsroot/opentracker/ot_clean.c,v $: $Revision: 1.19 $
$Source: /home/cvsroot/opentracker/ot_fullscrape.c,v $: $Revision: 1.33 $
$Source: /home/cvsroot/opentracker/ot_http.c,v $: $Revision: 1.47 $
$Source: /home/cvsroot/opentracker/ot_iovec.c,v $: $Revision: 1.6 $
$Source: /home/cvsroot/opentracker/ot_mutex.c,v $: $Revision: 1.23 $
$Source: /home/cvsroot/opentracker/ot_stats.c,v $: $Revision: 1.59 $
$Source: /home/cvsroot/opentracker/ot_udp.c,v $: $Revision: 1.23 $
$Source: /home/cvsroot/opentracker/ot_vector.c,v $: $Revision: 1.19 $
$Source: /home/cvsroot/opentracker/scan_urlencoded_query.c,v $: $Revision: 1.34 $
$Source: /home/cvsroot/opentracker/trackerlogic.c,v $: $Revision: 1.135 $
$Source: /home/cvsroot/opentracker/ot_livesync.c,v $: $Revision: 1.17 $
Comment 47 Simon 2010-04-02 06:22:07 EDT
(In reply to comment #46)
> FWIW, I took the RPM that Simon kindly prepared and rebuilt it against a RHEL5
> variant (CERN Scientific Linux 5), with the CVS snapshot from 2010-03-25.
> 
> The tracker works as expected (and at least the ipv4 x86_64 binary I built does
> not suffer from the bug reported in Comment 44).
> 
> There is however an issue with the access list file (white list), enabled with
> the Makefile option "-DWANT_ACCESSLIST_WHITE". When opentracker is started, it
> states:
> 
> Warning: Can't open accesslist file: /foo/bar (but will try to create it later,
> if necessary and possible).
Does opentracker start? It has to start, this is just a warning 
You have to modify the configfile to add a whitelist or use the sysconfigfile. Both are in /etc/ packaged.

> The error occurs although /foo/bar exists, contains valid torrent hashes, and
> is readable by the opentracker user. This is preventing any torrent to be
> handled by the tracker in case "-DWANT_ACCESSLIST_WHITE" is used.
Whitelist is a feature which is a wish of Matt Domsch to dominate and control and restrict the freedom of the users. This feature is just enabled, but not used by default.
/foo/bar is just a fake! Just edit one of the conffiles to use the whitelist-feature!
Comment 48 Matt Domsch 2010-04-02 08:31:37 EDT
Simon, I object to your characterization.  If I wanted to "dominate and control and restrict the freedom of the users", I would not do so through a whitelist in a little-used application, the author of such application added him/herself recognizing the need for such a feature.  Try again...
Comment 49 Simon 2010-04-02 11:40:09 EDT
btw, you are still blocking this bug.
please give feedback.
Comment 50 Romain Wartel 2010-04-07 05:25:23 EDT
(In reply to comment #47)
> Does opentracker start? It has to start, this is just a warning 

Yes, it starts properly. Except the whitelist file is not accepted by opentracker, so no torrent can be added.

> You have to modify the configfile to add a whitelist or use the sysconfigfile.
> Both are in /etc/ packaged.

Sure - and this is done. "/foo/bar" is a generic name for our real whitelist path, which looks more like /etc/opentracker-ipv4/whitelist.txt.

> Whitelist is a feature which is a wish of Matt Domsch to dominate and control
> and restrict the freedom of the users. 

Actually we find the whitelist option pretty useful. We plan to use opentracker to share VM images over a large infrastructure, and we would like our tracker to be dedicated for this purpose only.

> /foo/bar is just a fake! Just edit one of the conffiles to use the
> whitelist-feature!    

Yes, /foo/bar is obviously used here as a generic placeholder to demonstrate the issue.

We have edited the configuration files, added a whitelist (we can reference it with /etc/opentracker-ipv4/whitelist.txt if you prefer).

Nevertheless, although /etc/opentracker-ipv4/whitelist.txt exists, is populated and world readable, opentracker still displays:

  Warning: Can't open accesslist file: /etc/opentracker-ipv4/whitelist.txt (but will try to create it later, if necessary and possible).

Since the option is activated in the configuration file, but the whitelist cannot be used, no torrent can be added to the tracker.
Comment 51 Simon 2010-04-08 03:33:37 EDT
(In reply to comment #50)
> > Whitelist is a feature which is a wish of Matt Domsch to dominate and control
> > and restrict the freedom of the users. 
> 
> Actually we find the whitelist option pretty useful. We plan to use opentracker
> to share VM images over a large infrastructure, and we would like our tracker
> to be dedicated for this purpose only.
 
> > /foo/bar is just a fake! Just edit one of the conffiles to use the
> > whitelist-feature!    
> 
> Yes, /foo/bar is obviously used here as a generic placeholder to demonstrate
> the issue.
> 
> We have edited the configuration files, added a whitelist (we can reference it
> with /etc/opentracker-ipv4/whitelist.txt if you prefer).
> 
> Nevertheless, although /etc/opentracker-ipv4/whitelist.txt exists, is populated
> and world readable, opentracker still displays:
> 
>   Warning: Can't open accesslist file: /etc/opentracker-ipv4/whitelist.txt (but
> will try to create it later, if necessary and possible).
> 
> Since the option is activated in the configuration file, but the whitelist
> cannot be used, no torrent can be added to the tracker.    


@cern.ch and @dell.com both are totalitarian ruled systems. I would prefer an anarchy and grant all users to add whatever they want. The main problem is that this package can't meet all demands, because there are users who prefered free access to the resources like me and there are some oppressors like you and matt who prefer a whitelist and I'm sure someday there will be a user who asked for the blacklist-feature.
The Whitelist-feature seems to satisfy the wish of the vast majority (I can't believe that 2 people can be a majority, but okay), so I will completely integrate the whitelist-feature in the next few days. Then it will work ootb after the deamon is (re)started with the filled whitelist. Btw it works for me if I edit all sections of the conffile which are related to this feature.
Comment 52 Romain Wartel 2010-04-09 04:02:53 EDT
Simon,

I think it is essential that the service provider have control over the service they provide, and can decide to control access to it, should it be opentracker, sshd, imap, etc.

The whitelist feature is useful for us to ensure the performance and integrity of the service we provide to our users. In our environment, our users have no need or will to add themselves new torrents.
Access control mechanisms in general are not aimed at diminishing the freedom of users, but to ensure confidentiality, integrity and availability, etc.

Both the backlist and whitelist options are fully implemented, and even if they are compiled in the package, they remain anyway _disabled by default_. 

I have made small modifications to the patches you have made, to chroot opentracker in /var/opentracker, directory in which the whitelist can now be successfully accessed.

The problem was caused by the default chroot (in ".") applied by opentracker when "-d" or "tracker.rootdir" is not called or instanciated, and which caused problem when the process was daemonised.

Although the issue is fully resolved by new spec+config files, I have reported it upstream.

I have also suggested the author to push as much options as possible from the source code or compile time to the configuration file, like the user used when privileges are dropped (hardcoded to "nobody", changed to "opentracker" in the current package).

I have attached the SRPM we use here, in case anyone finds it useful.
Comment 53 Romain Wartel 2010-04-09 04:05:29 EDT
Created attachment 405482 [details]
opentracker source RPM
Comment 54 Romain Wartel 2010-04-09 06:20:18 EDT
Created attachment 405506 [details]
Updated opentracker 0-0.9.20100410cvs RPM

Implemented minor fixes, and addressed the segmentation fault issue reported in Comment 44.

The segfault was caused by the GCC option "-Os" being commented in the Makefile. This caused GCC to use the default "-O2" optimisation instead, which for some reason results in a segmentation fault. The root cause of the issue is not understood so the workaround may be weak, but re-enabling the size optimization option just seemed to solve the issue for us.

For debugging enthousiasts, the GCC(1) manpage states:
"-Os Optimize for size.  -Os enables all -O2 optimizations that do not typically increase code size. It also performs further optimizations designed to reduce code size."
Comment 55 Simon 2010-04-11 17:16:58 EDT
Thank you very much Romain.
I realized to give the chroot in the pkg is essential, but I think it's more elegant to give this in an extra package. so it's possible for both tracker-versions to use one chroot instead of staying in a conflict to each other. They also can use one whitelist.
I also added a little readme, its just a try to explain my line of thought.

http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.10.20100410cvs.fc14.src.rpm

What do you think, Romain?
Comment 56 Romain Wartel 2010-04-14 08:52:35 EDT
Thanks for the update Simon. 

I have had some discussions with Dirk Engling (opentracker author) and he kindly fixed the root cause of the segmentation fault, changed the default chroot behavior, and enabled the opentracker uid to be selected in the configuration time. Just what we needed, thanks Dirk!

I am a bit concerned with having multiple packages and if at all possible, given the chroot issue is fixed upstream, I would like to avoid having a separate RPM based on the chroot configuration.

Note also that it is not necessary to move the configuration file into the chroot jail, only the white/black list needs to be. In our environment, the  configuration file is in /etc/opentracker-ipv4/opentracker-ipv4.conf, and our whitelist is /var/opentracker/torrents.conf.

I have disabled the whitelist option in the configuration file as a default. In other words, opentracker is now completely open by default, unless "access.whitelist" is later populated in the configuration file, then the option becomes active upon service restart. I hope this is a good compromise for everyone.

To summarize, from the CVS snapshot from 2010-04-14, there are patches to:
- change slightly the default configuration file
- enable a "daemon mode"
- modify some of the Makefile parameters
- have init.d scripts to enable service start/stop/restart

Thanks to the fixes made by Dirk upstream, opentracker is now integrated nicely within our environment. Given the files are extremely small, I will attach below the tarball, SRPM and the x86_64 build we use here, for those interested.
Comment 57 Romain Wartel 2010-04-14 08:57:05 EDT
Created attachment 406498 [details]
opentracker 0-0.11.20100414cvs package
Comment 58 Romain Wartel 2010-04-14 08:57:29 EDT
Created attachment 406499 [details]
opentracker 0-0.11.20100414cvs tarball
Comment 59 Romain Wartel 2010-04-14 08:58:42 EDT
Created attachment 406500 [details]
opentracker 0-0.11.20100414cvs ipv4 x86_64 binary RPM (built on EL5)
Comment 61 Dominic Hopf 2010-05-22 15:04:20 EDT
Guys, thanks very much for your efforts. Here's another try of a formal review. There are at least two more small issues which need to be fixed (scroll down for those) before I will approve the package.

$ rpmlint opentracker.spec
opentracker.spec: W: invalid-url Source0: opentracker-0.tar.bz2
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

This warning can be ignored, since opentracker is rolling release.


$ rpmlint opentracker-0-0.12.20100410cvs.fc12.src.rpm
opentracker.src: W: invalid-url Source0: opentracker-0.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

See above.


$ rpmlint opentracker-debuginfo-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-ipv4-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-ipv6-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch.rpm

opentracker-ipv4.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod
opentracker-ipv4.x86_64: W: no-manual-page-for-binary opentracker-ipv4
opentracker-ipv6.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod
opentracker-ipv6.x86_64: W: no-manual-page-for-binary opentracker-ipv6
opentracker-rootdir.noarch: W: spelling-error Summary(en_US) dir -> deer, rid, Dir
opentracker-rootdir.noarch: W: spelling-error %description -l en_US Filesystem -> File system, File-system, Systematic
opentracker-rootdir.noarch: W: spelling-error %description -l en_US chroot -> cheroot, ch root, ch-root
opentracker-rootdir.noarch: W: no-documentation
opentracker-rootdir.noarch: W: non-standard-gid /var/opentracker opentracker
4 packages and 0 specfiles checked; 0 errors, 9 warnings.


Manpages should be provided by upstream. Spelling issues can be ignored,
I think the wording is okay. The other rootdir package warnings can also be
ignored.


Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines
 [x] Specfile name matches %{name}.spec
 [x] Package seems to meet Packaging Guidelines
 [x] Package successfully compiles and builds into binary RPMs on at least one
     supported architecture.
     Tested on: Fedora 12/x86_64
 [x] Rpmlint output:
     source RPM: see above
     binary RPM: see above
 [x] Package is not relocatable.
 [x] License in specfile matches actual License and meets Licensing Guidelines
     License: Beerware
 [-] License file is included in %doc.
 [x] Specfile is legible and written in AE
 [x] Sourcefile in the Package is the same as provided in the mentioned Source
     diff and meld do not show any differences between an own checkout and
     provided tarball
 [x] Package compiles successfully
 [x] All build dependencies are listed in BuildRequires
 [-] Specfile handles locales properly
 [-] ldconfig called in %post and %postun if required
 [x] Package owns directorys it creates
 [-] Package requires other packages for directories it uses.
 [x] Package does not list a file more than once in the %files listing
 [x] %files section includes %defattr and permissions are set properly
 [x] %clean section is there and contains rm -rf %{buildroot}
 [x] Macros are consistently used
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage
 [x] Program runs properly without files listed in %doc
 [-] Header files are in a -devel package
 [-] Static libraries are in a -static package
 [-] Package requires pkgconfig if .pc files are present
 [-] .so-files are put into a -devel subpackage
 [-] Subpackages include fully versioned dependency for the base package
 [-] Any libtool archives (*.la) are removed
 [-] contains desktop file (%{name}.desktop) if it is a GUI application
 [x] Package does not own files or directories owned by other packages.
 [x] %{buildroot} is removed at beginning of %install
 [-] Filenames are encoded in UTF-8

=== SUGGESTED ITEMS ===
 [-] Package contains latest upstream version
 [x] Package does not include license text files separate from upstream.
 [-] non-English translations for description and summary
 [x] Package builds in mock
     Tested on: F12/x86_64
 [x] Package should compile and build into binary RPMs on all supported architectures.
     tested build with koji
 [!] Program runs
	 The opentracker-ipv6 service does not start. See below.

 [-] Scriptlets must be sane, if used.
 [-] pkgconfig (*.pc) files are placed in a -devel package
 [-] require package providing a file instead of the file itself
     no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required

=== Issues to point out ===
==== 1 ====
The user change for the directory /var/%{name} doesn't seem to work as intended.
The user opentracker is also needed for the opentracker-rootdir subpackage, but
you create it just with the %pre ipv4 and %pre ipv6 commands. This results in a
warning when installing the package with yum:

	Running Transaction
		Installing     : opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch                                                                          1/4
	warning: group opentracker does not exist - using root

/var/opentracker is not owned by the user opentracker then, but by the user
root.

Since the user creation is the same for both subpackages, I suggest to create
the user once, when installing the rootdir subpackage (since it is
installed as the first package).

==== 2 ====
I get an error when trying to start the opentracker-ipv6 service:
# LANG=C service opentracker-ipv6 start
opentracker-ipv6 starten: /etc/init.d/opentracker-ipv6: Zeile 36: -f: Kommando nicht gefunden.
                                                           [FEHLGESCHLAGEN]

I can not do any deeper testing for IPv6, unfortunately.

==== 3 ====
I think there is no need for two different configuration directories (when the
ipv4 and ipv6 package is installed) in /etc/. It should be enough to have one
directory (/etc/opentracker/) with two different files (opentracker-ipv4.conf,
opentracker-ipv6.conf) in it. However, this one shall not block the review.
Comment 62 Simon 2010-09-20 13:35:35 EDT
thx you Dominic, I will take a look again in this pkg next week.
Comment 63 Simon 2010-09-29 15:07:22 EDT
1 this makes sense
2 an error while i copy/paste phrases
3 you are right. i will keep it simple next time.

no new cvs checkout - just code cleaning and it won't build. Don't know why. I will keep an eye on it.

http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.13.20100414cvs.fc15.src.rpm
Comment 64 Joel Uckelman 2010-10-16 14:50:46 EDT
(In reply to comment #60)
> http://cassmodiah.fedorapeople.org/opentracker/opentracker-0-0.12.20100410cvs.fc14.src.rpm

I'm testing out the opentracker-ipv4 RPM built from this, and found that the opentracker process goes completely nuts if you make a HTTP request to /stats.
Soon after I do that, opentracker-ipv4 stops responding, and top shows it running at nearly 100% CPU.
Comment 65 Joel Uckelman 2010-10-16 16:28:19 EDT
I noticed that -DWANT_RESTRICT_STATS isn't enabled in the SRPM. It would be nice if it were.
Comment 67 Stefan Neufeind 2010-12-05 19:33:37 EST
Since no more objects from a packaging-side arrived so far, the package builds fine for me on FC14 and opentracker works as intended, I'd like to suggest to accept this package.
Comment 68 Dominic Hopf 2010-12-11 22:36:54 EST
I'll spare you another full formal review. Any issue I pointed out in my previous reviews seem to be fixed and the package builds fine for me, also in mock and koji.

There are two new warnings by rpmlint I at least like to mention:

opentracker-common.noarch: W: spelling-error %description -l en_US Filesystem -> File system, File-system, Systematic

This warning is warrantable, I'd also rather write "file system" then "filesystem".


opentracker.src: W: invalid-license Beerware

This occurred when I rpmlinted the source RPM. I'd consider this an rpmlint bug, since renaming the License to "Beerware License" throws a similar warning, but it's listed with that name at https://fedoraproject.org/wiki/Licensing. Anyone has a hint about that?



opentracker-ipv4 starts up here, but trying to start opentracker-ipv6 results in an error again:

# service opentracker-ipv6 start
opentracker-ipv6 starten: socket_bind6_reuse: Invalid argument
                                                           [FEHLGESCHLAGEN]

I don't know whats going on there and I won't do any deeper investigation since I'm not using IPv6. If anyone is interested to get this working, feel free to do so.

The IPv6 has to be fixed before I can approve the package. We can not deliver programs which are not starting or working correctly. I personally would be fine with disabling the IPv6 subpackage by commenting out the concerning lines in the spec file until this issue is finally fixed. If anyone is interested in having  an opentracker-ipv6 subpackage immediately, please support Simon and maybe upstream to get this working.
Comment 69 Dominic Hopf 2010-12-12 13:06:54 EST
May anyone who is using IPv6 confirm opentracker-ipv6 is running fine for him. The problems I noticed on my system most likely be related to the fact that I don't even have NICs started with an IPv6 address.
Comment 70 Stefan Neufeind 2010-12-12 16:55:37 EST
Dominic, comment 69:
Confirmed to work fine. Running opentracker-ipv4 and opentracker-ipv6 side-by-side. If you want to test something, msg me offlist :-)
Comment 71 Stefan Neufeind 2010-12-12 17:08:39 EST
Small enhancement-suggestion for the init-scripts: opentracker supports sending it a SIGHUP as well to re-read the configuration (for example if you use it in whitelist/blacklist-mode to re-read the lists without disruption).

Part can simply be taken from another Fedora-init-script (for example snmpd):
reload(){
        echo -n $"Reloading $prog: "
        killproc -p $pidfile $binary -HUP
        RETVAL=$?
        echo
        return $RETVAL
}

And maybe stop should also be using the pidfile instead of just using the program-name (also for example from snmpd):

stop() {
        echo -n $"Stopping $prog: "
        if [ $UID -ne 0 ]; then
                RETVAL=1
                failure
        else
                killproc -p $pidfile $binary
                RETVAL=$?
                [ $RETVAL -eq 0 ] && rm -f $lockfile
        fi;
        echo
        return $RETVAL
}
Comment 72 Dominic Hopf 2010-12-12 19:01:42 EST
Thanks for your feedback Stefan. Simon also told me its working on several virtual machines, so I assume this is a local issue on my side. Simon, I think you can go ahead and request CVS.. euh, GIT on this now.. :)
Comment 73 Simon 2010-12-21 03:40:22 EST
New Package SCM Request
=======================
Package Name: opentracker
Short Description: Bit Torrent Tracker
Owners: cassmodiah
Branches: EL-5 EL-6 F-14 F-13
InitialCC:
Comment 74 Jason Tibbitts 2010-12-21 10:59:33 EST
Git done (by process-git-requests).
Comment 75 Simon 2011-01-26 01:46:19 EST
imported in all requested branches!
Thank you all!

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