[Bug 174883] Review Request: distcc -- A free distributed C/C++ compiler system

bugzilla at redhat.com bugzilla at redhat.com
Sat Mar 31 14:42:17 UTC 2007


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

Summary: Review Request: distcc -- A free distributed C/C++ compiler system


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


bugzilla at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium

enrico.scholz at informatik.tu-chemnitz.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|medium                      |low




------- Additional Comments From enrico.scholz at informatik.tu-chemnitz.de  2007-03-31 10:42 EST -------
* Sat Mar 31 2007 Enrico Scholz <enrico.scholz at informatik.tu-chemnitz.de> - 2.18.3-5
- require main package by -gnome
- use %_bindir macro instead of /usr/bin to create links to the
  compilers

* Sun Mar 18 2007 Enrico Scholz <enrico.scholz at informatik.tu-chemnitz.de> - 2.18.3-5
- moved symlinks from /usr/share/distcc/bin to /usr/libexec/distcc/bin
- handle symlinks with %ghost instead of removing them manually
- use CCACHE_PREFIX in the profile script
- followed nonsense^Wrule-of-the-day and removed %config from
  init-scripts, added big 'DO NOT MODIFY' banners to it and made it
  0555
- ship the icon only in the them directory and apply patch to use
  gtk_window_set_icon_name() instead of
  gtk_window_set_icon_from_file()


http://ensc.de/fedora/distcc/


=============================


@comment 25
===========

>   * W: distcc strange-permission distccd.sysv 0755

sounds like a bug in rpmlint... 0755 is a valid, non-strange permission



>      - The following script
> ----------------------------------------------------
> %preun
> test "$1" -ne 0 || rm -f %pkgdatadir/bin/*
> ----------------------------------------------------
>        should be treated by %ghost files. And having symlinks marked
>        as %ghost files is needed anyway, otherwise these symlinks
>        are regarded as being not owned by any package.
>
>        And "rm -f %pkgdatadir/bin/*" (all glob) is too dangerous. Remove
>        only the files which should really be treated by this rpm.

ok; changed



> ----------------------------------------------------
>     [ ! -x /usr/bin/$c ] || ln -sf %_bindir/distcc %pkgdatadir/bin/$c
> ----------------------------------------------------
>        - Why do you use "/usr/bin/"$c (this is not macro) and "%_bindir"/distcc
> 	 (here macro %_bindir is used)?

ok; changed



>        - By the way, while "ln" and "rm" are marked as dangerous
> 	   commands, "unlink" is not marked as such.

... I will not change such things just to silent rpmlint. There are
enough other things (e.g. 'L=r;M=m; ${L}${M} -rf /') how such messages
can be prevented


>     * E: distcc-server non-standard-gid /var/log/distccd.log distcc

bug in rpmlint to mark such things as errors


>       E: distcc-server non-root-group-log-file /var/log/distccd.log distcc
>       - Fot the latter rpmlint says:
> -----------------------------------------------------
> If you need log files owned by a non-root group, just create a subdir in
> /var/log and put your log files in it.
> -----------------------------------------------------
> 	Perhaps you have to create /var/log/distccd directory and
> 	move the log files under the directory, however I can see
> 	some other packages putting log files under /var/log with
> 	non-standard gid......

IMO, it is overkill to create for every single logfile an own directory.
It will break logrotation when for example 'olddir .old' is specified in
a global configuration file and administrator did not created '.old' in
the directory.

Beside this, changing the default /var/log/distccd.log logfile would
complicate things because it is hardcoded in some places.


>     * W: distcc-server dangerous-command-in-%post chown
>       - The corresponding scripts are:
> -----------------------------------------------------
> %post server
> test -e '%logfile' || {
> 	touch '%logfile'
> 	chown root:%username '%logfile'
> 	chmod 0620 '%logfile'
> }
> -----------------------------------------------------
> 	If the %logfile should always exist, then this should
> 	not be handled by %ghost, but should be handled by
> 	* this file should be touched at %install stage
> 	* should be handled by %verify(not md5 size mtime)
> 	* and chown call should be removed.

I think, the '%config %ghost' mark are the only correct way to handle
logfiles. See http://fedoraproject.org/wiki/PackagingDrafts/Logfiles


>       - Well, I remember this was discussed on fedora-????-list
> 	  recently, and what was the conclusion?? Should rcinit file
> 	  be marked as %config?  (Is this really a %config file?)

Ok; I applied this nonsense-of-the-day rule...


> W: distcc-server-xinetd summary-not-capitalized xinetd initscripts for the distcc daemon
> ------------------------------------------------------
>       - Simply change to "Xinetd initscripts...."

I use 'The xinetd initscripts'; capitalizing 'xinetd' looks very
odd...


> W: distcc conffile-without-noreplace-flag /etc/profile.d/distcc.sh
> W: distcc-server conffile-without-noreplace-flag /var/log/distccd.log
> E: distcc-server incoherent-logrotate-file /etc/logrotate.d/distccd
> W: distcc-server-sysv no-documentation
> E: distcc-server-sysv non-standard-dir-perm /var/run/distccd 0775
> W: distcc-server-sysv incoherent-init-script-name distccd
> W: distcc-server-xinetd no-documentation
> -----------------------------------------------------
>      ... However, once please comment on these warnings.

Looks like bugs in rpmlint


>    * For GTK+ icon cache
>      - Well, please check again the scriptlets for "GTK+ icon cache"
>        http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

ok; changed (I thought this has been changed recently)


>   # ignore ownership of the %_datadir/icons/... directories; Core is
>   # too broken to add good Requires(pre/postun).
> -----------------------------------------------------
>    - If you mind, you can simply add to -gnome package:
>      Requires: hicolor-icon-theme

thx; I did not saw this tree in the wood of packages owning
%_datadir/icons/hicolor



@comment 26
===========

> 1) Is it really necessary to split off the init scripts and xinetd
>    stuff into sub-packages?

yes; server works without xinetd or initscripts and both packages are
adding additional dependencies

> 2) distcc.sh is incredibly unreadable. And broken, it will fail on
>    x86_64 because ccache is in /usr/lib64/ccache

thx; fixed


>    Perhaps libexec is the place both ccache and distcc should be
>    playing their gcc hijacking tricks.

thx; I moved distcc stuff into libexec/


>         CCACHE_PREFIX=distcc

thx; I was not aware of this option and use it now in the profile
file. Nevertheless, this file is yet more complex now


> 3) What does release_func accomplish? It obfusticates the fact that
>    the release tag violates policy.

sorry; I can not follow you here


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.




More information about the package-review mailing list