Bug 174883
Summary: | Review Request: distcc -- A free distributed C/C++ compiler system | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Enrico Scholz <rh-bugzilla> | ||||||||
Component: | Package Review | Assignee: | Callum Lerwick <seg> | ||||||||
Status: | CLOSED DUPLICATE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||
Severity: | low | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | CC: | che666, ch.nolte, denis, ed, laurent.rineau__fedora, lpoetter, mtasaka, paul, seg | ||||||||
Target Milestone: | --- | ||||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
URL: | http://distcc.samba.org/ | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2008-03-07 22:25:33 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Attachments: |
|
Description
Enrico Scholz
2005-12-03 12:30:36 UTC
just rebuilt the package fine on x86_64 rawhide. some comments: i am not sure if the initscript splitting makes sense really. it creates alot subpackages for not much gain. actually the initng script could maybe go directly upstream. just some thoughts. *** Bug 179775 has been marked as a duplicate of this bug. *** Hi! As I have submitted a duplicate, I am now in a position to tear your package apart ;-) I have found the following problems and possible sollutions: A " mock -r fedora-development-i386-core" failed because subpackage gnome needs "BuildRequires: desktop-file-utils" for desktop-file-install. Rpmlint shows the following problems: W: distcc incoherent-version-in-changelog 2.18.3-0.6 2.18.3-1 The version your changelog refers to should be 2.18.3-1 W: distcc conffile-without-noreplace-flag /etc/profile.d/distcc.sh As this is no config-file this warning seems to be ignorable E: distcc executable-marked-as-config-file /etc/profile.d/distcc.sh You should add distcc.sh separately not flagging it as a config. This will make the warning above go away, too. E: distcc script-without-shellbang /etc/profile.d/distcc.sh "!#" is missing in your shellscript W: distcc dangerous-command-in-%preun rm W: distcc dangerous-command-in-%trigger ln W: distcc dangerous-command-in-%trigger rm I don't know how dangerous this really is. Please check that. W: distcc-gnome non-standard-group Development/Applications This should just be one of /usr/share/doc/rpm-4.?.?/GROUPS W: distcc-gnome no-documentation As there is no documentation only for distccmon-gnome, this can be ignored W: distcc-server-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/distccd This is no config-file, don't flag it as such. W: distcc-server-lsb no-documentation The manpage for the server is missing: %{_mandir}/man1/distccd.1.gz E: distcc-server-lsb executable-marked-as-config-file /etc/rc.d/init.d/distccd This is related to the warning above. E: distcc-server-lsb non-standard-gid /var/run/distccd distcc E: distcc-server-lsb non-standard-dir-perm /var/run/distccd 0775 I guess this must be ignored because the server won't work as user "distcc" otherwise E: distcc-server-lsb postin-without-chkconfig /etc/rc.d/init.d/distccd E: distcc-server-lsb preun-without-chkconfig /etc/rc.d/init.d/distccd "chkconfig -add distccd" and "chkconfig -del distccd" is needed for correctly un-/installing the server package W: distcc-server-lsb incoherent-init-script-name distccd I don't know what is wrong here... Is this package in limbo? So it seems... As there has been no further reply from the original poster, I would offer adopting this package. not so fast... there was some silence between my original submission and the first review. Other things toke higher priority and I had no time yet to address the issues from comment #3 completely. * Mon Mar 06 2006 Enrico Scholz <enrico.scholz.de> - 2.18.3-1.3 - added 'desktop-file-utils' BR - moved the gnome icon the a common place - added -server-xinetd subpackage - added -server-ssh subpackage for ssh-setup - added logrotate support http://ensc.de/fedora/distcc.spec http://ensc.de/fedora/distcc-2.18.3-1.3.src.rpm =============== > W: distcc incoherent-version-in-changelog 2.18.3-0.6 2.18.3-1 > The version your changelog refers to should be 2.18.3-1 ok, will be fixed (at least in the released version ;)) > W: distcc conffile-without-noreplace-flag /etc/profile.d/distcc.sh > As this is no config-file this warning seems to be ignorable > > E: distcc executable-marked-as-config-file /etc/profile.d/distcc.sh > You should add distcc.sh separately not flagging it as a config. > This will make the warning above go away, too. I removed the 'x' bit from the distcc.sh. But I think that it should be marked as a simple %config: there should be no need to edit this file but changes should not be lost silently. > E: distcc script-without-shellbang /etc/profile.d/distcc.sh > "!#" is missing in your shellscript The shellbang warning is bogus; the profile.d scripts will be source'd but not executed. > W: distcc dangerous-command-in-%preun rm > W: distcc dangerous-command-in-%trigger ln > W: distcc dangerous-command-in-%trigger rm > I don't know how dangerous this really is. Please check that. should be ok... > W: distcc-gnome non-standard-group Development/Applications > This should just be one of /usr/share/doc/rpm-4.?.?/GROUPS is now Development/Tools > W: distcc-server-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/distccd > This is no config-file, don't flag it as such. initscripts are usually marked as simple %config files (there should be no need to edit them but changes should not be lost silently.) > W: distcc-server-lsb no-documentation > The manpage for the server is missing: %{_mandir}/man1/distccd.1.gz the -server subpackage has this man-page and this subpackage is required by -server-lsb. > E: distcc-server-lsb postin-without-chkconfig /etc/rc.d/init.d/distccd > E: distcc-server-lsb preun-without-chkconfig /etc/rc.d/init.d/distccd > "chkconfig -add distccd" and "chkconfig -del distccd" is needed for > correctly un-/installing the server package there ARE rules to register/unregister the service The rpm's build fine under fc5. I have installed and tested the following
rpm's:
distcc-server-ssh-2.18.3-1.3
distcc-gnome-2.18.3-1.3
distcc-server-lsb-2.18.3-1.3
distcc-server-2.18.3-1.3
> there ARE rules to register/unregister the service
Sorry, I overlooked that. The file distccd.lsb you install as
$RPM_BUILD_ROOT%_initrddir/distccd contains the following lines:
...
f=/etc/sysconfig/distccd
DISTCC_OPTS=
DISTCC_ALLOW=127.0.0.1/32
DISTCC_PIDFILE=/var/run/distccd/distccd.pid
DISTCC_NICENESS=5
DISTCC_USER=distcc
test ! -r "$f" || . "$f"
...
But the file /etc/sysconfig/distccd is missing in your installation. I would
furthermore suggest not to test for readability (-r) but for existance (-a)of the
sysconf-file.
rpmlint shows the following problems:
W: distcc-server-ssh no-documentation
W: distcc-gnome no-documentation
W: distcc-server-lsb conffile-without-noreplace-flag /etc/rc.d/init.d/distccd
W: distcc-server-lsb no-documentation
E: distcc-server-lsb executable-marked-as-config-file /etc/rc.d/init.d/distccd
Ignoreable
E: distcc-server-lsb non-standard-gid /var/run/distccd distcc
E: distcc-server-lsb non-standard-dir-perm /var/run/distccd 0775
That should be fixed, I guess.
E: distcc-server-lsb postin-without-chkconfig /etc/rc.d/init.d/distccd
E: distcc-server-lsb preun-without-chkconfig /etc/rc.d/init.d/distccd
Ignoreable
W: distcc-server-lsb incoherent-init-script-name distccd
W: distcc-server conffile-without-noreplace-flag /var/log/distccd.log
E: distcc-server incoherent-logrotate-file /etc/logrotate.d/distccd
E: distcc-server non-standard-gid /var/log/distccd.log distcc
E: distcc-server non-root-group-log-file /var/log/distccd.log distcc
This should be fixed, too.
W: distcc-server dangerous-command-in-%post chown
Ignoreable
The initng-packages should be checked by someone else, because I have no time
for that.
Best regards
Christian
Is Greg really going to review this or should it be reassigned to nobody? If greg steps down, I'm happy to take it on (I use it quite regularly anyway) %post gnome gtk-update-icon-cache -qf /usr/share/icons/hicolor 2>/dev/null || : Please check http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d %pre server /usr/sbin/fedora-groupadd This really needs to be checked ala the above link %triggerin -- %handled_pkgs for name in %handled_progs; do for c in $name i386-redhat-linux-$name This is an "out of interest" question - why only i386-redhat? There is no excludearch up there. sorry; forgot to upload/announce the updated spec file * Wed Aug 09 2006 Enrico Scholz <enrico.scholz.de> - 2.18.3-1.6 - bound a %postun script to -gnome subpackage - use %bcond* macros * Sun Jul 09 2006 Enrico Scholz <enrico.scholz.de> - 2.18.3-1.5 - updated to new fedora-usermgmt code - updated to new ccache path - added distccmon-gnome-icon.png to the pkgdatadir http://ensc.de/fedora/distcc/ I will comment the issues in comment #11 in this evening. * Wed Aug 16 2006 Enrico Scholz <enrico.scholz.de> - 2.18.3-1.8 - simplified LSB initscripts and support only one $DISTCC_OPTS variable - ship sample sysconfig file for LSB - use rpm macros to create the platform tuple instead of hardcoding 'i386-redhat-linux' - added '%%bcond_without fedora' http://ensc.de/fedora/distcc/ ===== comment #8: ----------- > But the file /etc/sysconfig/distccd is missing in your installation. it will be read only, when existing. Hence, there is no real need to ship it. For documentation purposes, I added it in the current package > I would furthermore suggest not to test for readability (-r) but for > existance (-a)of the sysconf-file. I do not see problems with '-r' > E: distcc-server-lsb non-standard-gid /var/run/distccd distcc > E: distcc-server-lsb non-standard-dir-perm /var/run/distccd 0775 > > That should be fixed, I guess. permissions/group are fine comment #11: ------------ > %post gnome > gtk-update-icon-cache -qf /usr/share/icons/hicolor 2>/dev/null || : > > Please check > > http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d is ok; the 'touch ...' in the sample scriptlet might be from old FC when 'gtk-update-icon-cache' did not know the '-f' flag. I do not see sense in checking for existence first: the command will be called with trailing '|| :' and potential error messages redirected into /dev/null > %pre server > /usr/sbin/fedora-groupadd package should have the corresponding 'Requires(pre): fedora-usermgmt' > for c in $name i386-redhat-linux-$name > > This is an "out of interest" question - why only i386-redhat? thx; I compose the platform tuple from rpm macros now The latest spec-file http://ensc.de/fedora/distcc/distcc-2.18.3-1.8.fc5x.src.rpm can not be built under a FC5 system because of the fedora-usermgmt-devel requirement. According to http://fedoraproject.org/wiki/PackageUserCreation there could be a workaround for systems which do not provide the usermgmt requirement (section 'Alternatives'). It seems that 'fedora-usermgmt-devel' should be 'fedora-usermgmt' The fedora-usermgmt-devel stuff is for/in the -devel branch. When package gets approved during the FC-5 lifetime, I will use the old style. Reassigning bugs from gdk (old RHN engineering Manager) to tsanders (current RHN engineering manager) What is the status of the current review? It seems stalled. Enrico What is the status of the current review? It seems stalled. It seems that nobody is reviewing this bug. Umm... this review ticket has currently no activity. Enrico, would you update your spec/srpm if you have some points you want to change or fix? After that, I will take a look at your spec/srpm. Mon Jan 29 2007 Enrico Scholz <enrico.scholz.de> - 2.18.3-2 - cleanups - updated icon-cache scriptlets - removed empty (ssh), unavailable (minit) and conflicting (initng) initscripts http://ensc.de/fedora/distcc/ Created attachment 146910 [details]
Mock build log of distcc-2.18.3-2.fc7
Mockbuild pf 2.18.3-2 fails on FC-devel i386.
Please check the mockbuild log.
* Sat Mar 17 2007 Enrico Scholz <enrico.scholz.de> - 2.18.3-4 - workaround bug #232761 in desktop-file-utils by using long '--mode' instead of '-m' - moved away from -lsb initstyle and use proprietary RH sysv initstyle instead of - cleaned up categories of the desktop file http://ensc.de/fedora/distcc/ Created attachment 150315 [details] rpmlint log for distcc-2.18.3-3.7 Well, though I have not read the previous discussion on this bug report, I write here my first opinition. A. About rpmlint: A-1 for source: * W: distcc strange-permission distccd.sysv 0755 - Change to 0644. A-2 For binary: * W: distcc incoherent-version-in-changelog 2.18.3-4 2.18.3-3.7.fc7 - Very trivial... * W: distcc dangerous-command-in-%preun rm W: distcc dangerous-command-in-%trigger ln W: distcc dangerous-command-in-%trigger rm - 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. ---------------------------------------------------- [ ! -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)? - By the way, while "ln" and "rm" are marked as dangerous commands, "unlink" is not marked as such. * E: distcc-server non-standard-gid /var/log/distccd.log distcc 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...... * 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. * initrc file ----------------------------------------------------- W: distcc-server-sysv conffile-without-noreplace-flag /etc/rc.d/init.d/distccd E: distcc-server-sysv executable-marked-as-config-file /etc/rc.d/init.d/distccd ----------------------------------------------------- - 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?) * Summary for -server-xinetd ------------------------------------------------------ W: distcc-server-xinetd summary-not-capitalized xinetd initscripts for the distcc daemon ------------------------------------------------------ - Simply change to "Xinetd initscripts...." + IMO the following rpmlint can be ignored. ----------------------------------------------------- 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. B. Scriptlets * For GTK+ icon cache - Well, please check again the scriptlets for "GTK+ icon cache" http://fedoraproject.org/wiki/Packaging/ScriptletSnippets C. Directory ownership ----------------------------------------------------- # 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 1) Is it really necessary to split off the init scripts and xinetd stuff into sub-packages? I see no reason for it. 2) distcc.sh is incredibly unreadable. And broken, it will fail on x86_64 because ccache is in /usr/lib64/ccache, which is also broken. Perhaps libexec is the place both ccache and distcc should be playing their gcc hijacking tricks. How about something more like this: 3) What does release_func accomplish? It obfusticates the fact that the release tag violates policy. It should be a single integer followed by the release tag, unless you're bumping an old tree in which case you can add a number after the release tag. Err, that is, like this: if ! echo "$PATH" | grep -q "ccache" ; then PATH="/usr/share/distcc/bin:$PATH" else CCACHE_PREFIX=distcc fi * Sat Mar 31 2007 Enrico Scholz <enrico.scholz.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.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 (In reply to comment #26) > 3) What does release_func accomplish? It obfusticates the fact that the release > tag violates policy. It should be a single integer followed by the release tag, > unless you're bumping an old tree in which case you can add a number after the > release tag. Actually, if %release_func is not defined, the default %release_func: %global release_func() %1%{?dist} seems to do the right thing. However, that macro is obfuscating. It should be removed from the spec file, and Release: should be: Release: 6%{?dist} (In reply to comment #26) > 2) distcc.sh is incredibly unreadable. And broken, it will fail on x86_64 > because ccache is in /usr/lib64/ccache, which is also broken. That script is also broken even on i386: "PATH=$__d$PATH" should be "PATH=$__:d$PATH". Anyway, that script cannot be shipped in Fedora like that. It should be easily readable. What is more, do we really want to modify all users' PATH like that? (In reply to comment #30) > That script is also broken even on i386: "PATH=$__d$PATH" should > be "PATH=$__:d$PATH". "PATH=$__d:$PATH", of course. * Sat Aug 25 2007 Enrico Scholz <enrico.scholz.de> - 2.18.3-6 - fixed profile script by appending a missing ':' http://ensc.de/fedora/distcc/ ----- > That script is also broken even on i386 thx; but the ':' needs to be added to $__d > Anyway, that script cannot be shipped in Fedora like > that. It should be easily readable. ??? I do not see how to write it in another way. Everybody with 1-2 years bash experience should be able to understand and modify it. > What is more, do we really want to modify all users' PATH like that? yes Is anybody working on that package? yes (resp: no, there are no open issues) As far as I can see, there are a log of remarks made where you have just replied "No you are wrong", "I have done the right thing", "rpmlint is boggy". That might be the reason why this review request is stalled. As far as I am concerned. I told you: (In reply to comment #30) > That script is also broken even on i386: "PATH=$__d$PATH" should > be "PATH=$__:d$PATH". [sic] Anyway, that script cannot be shipped in Fedora like > that. It should be easily readable. > > What is more, do we really want to modify all users' PATH like that? And you answers were not satisfying. You replied: (In reply to comment #32) > > Anyway, that script cannot be shipped in Fedora like > > that. It should be easily readable. > > ??? I do not see how to write it in another way. Everybody with 1-2 years bash > experience should be able to understand and modify it. I was so understandable that you made an error in it. I know that it is readable. Even I, a zsh, user, managed to modify it. However, it is complicated, and I am not sure to understand its need. > > What is more, do we really want to modify all users' PATH like that? > > yes That reply was not a satisfying answer. I would have liked to know why it is needed. is there a question in your comment? Enrico, it would be nice if you could elaborate a bit if someone does not have an immediate understanding of what you are doing in your scripts or spec-files. I guess you can see that there are "implicit" questions in Laurent's comments. Laurent, the PATH has to be modified because if a user installs distcc it should automagically be in the users path, so that it can be used with minimal effort. As for the package: There are multiple rpmlint warnings. I would like you to fix them. If you don't think that the warnings rpmlint gives are correct then please tell us why you think so. I attach the warnings to this bug. The script /etc/profile.d/distcc.sh is fine with me. However, for a better understanding a one-line comment about what this script does would be good. One other thing is that variable names like __c or __d are not self speaking. Created attachment 257941 [details]
distcc rpmlint log 2.18.3-6
Enrico has a hard-on for obfuscation. Why do we need two different methods to do the same thing in discc.sh? Pick one please. I would point out the grep method is what ccache.sh uses. It seems rather gratuitous and inconsistent to me to have such a complex script to do basically the same thing another package is doing, one that distcc is likely to be used in combination with. Consistency is good. It looks like the license is GPLv2+. The license field must be changed to comply with current guidelines. http://fedoraproject.org/wiki/Licensing I'm kind of busy lately, though I do use distcc. If anyone else wants to take over the review, feel free. It would be great if the Zeroconf/Avahi patch I posted could be merged into this package: http://0pointer.de/blog/projects/avahi-distcc.html *** Bug 433228 has been marked as a duplicate of this bug. *** Enrico gave me his ok to go with the oher review, so we can now with all due reverence close this 27 month-old ticket. *** This bug has been marked as a duplicate of 433228 *** |