Bug 242310

Hi Marc,

It looks like $RPM_OPT_FLAGS are ignored during building:
+ make -j2 'OPTIMIZE=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector 
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -
DSUPPORT_LH7 -DMKSTEMP'
cc -O2 -g -Wall    isutf8.c   -o isutf8
cc -O2 -g -Wall    ifdata.c   -o ifdata
cc -O2 -g -Wall    pee.c   -o pee
cc -O2 -g -Wall    sponge.c   -o sponge
cc -O2 -g -Wall    mispipe.c   -o mispipe

make %{?_smp_mflags} CFLAGS="%{optflags} -DSUPPORT_LH7 -DMKSTEMP"
works better.

furthermore, can you include COPYING and README in %doc?

Hi Ruben,

Thanks for the comments, I have made the modifications as suggested.

New SRPM: http://marcbradshaw.co.uk/packages/SRPMS/moreutils-0.20-2.f7.src.rpm

New SRPM:
http://marcbradshaw.co.uk/packages/SRPMS/moreutils-0.20-2.src.rpm

IMO, /usr/bin/ts and /usr/bin/combine are names likely to clash with other packages.

(In reply to comment #4)
> IMO, /usr/bin/ts and /usr/bin/combine are names likely to clash with other
packages.

On checking those names do not appear in any current fedora packages.  Do you
know of any clashes?

No, I don't know of such clash in Fedora, but a google search indicates
* ts to be used by some tcl packages
* combine to be used by some ImageMagick packages

Apart of this, generally speaking, choosing such "likely to clash names" for a
newly developed package isn't necessarily a clever design.

I appreciate your comments.
moreutils has been in debian for some time and is now also in the ubuntu repo 
and as such the names already have some "history" behind them.
On the one hand there is the issue of the "likely to clash" names but IMO it 
would be more important in this case to keep the names consistant with both 
upstream and with the equivalent package in the other distros.

ImageMagick has not included the combine binary for a significant number of 
versions.
There is a tcl package with a LaTeX component which uses /usr/bin/ts for SUSE 
(ref: http://www.novell.com/products/linuxpackages/suselinux/ts.html).

I would appreciate some further opinions on this issue.

(In reply to comment #7)
> I appreciate your comments.
> moreutils has been in debian for some time and is now also in the ubuntu repo 
> and as such the names already have some "history" behind them.
This only means their QA has failed to recognize the potential dangers and has
slipped through a package they better should not ship.

> On the one hand there is the issue of the "likely to clash" names but IMO it 
> would be more important in this case to keep the names consistant with both 
> upstream and with the equivalent package in the other distros.
I think it would be appropriate to notify upstream to have their choices
revisited and not to include this package until upstream has fixed them.

Updated URL

http://marcbradshaw.co.uk/packages/moreutils-0.20-2.src.rpm
http://marcbradshaw.co.uk/packages/moreutils.spec

Hi Marc,

As discussed by private mail, I'll review your 3 submissions and when they are
all approved I'll sponsor you.

I've done a full review of the latest version and I've found a few issues
besides the potential name clash for ts:

Must FIX:
---------
* change license from "GPL" to "GPLv2" (sponge is GPL version 2 only)
* upstream has 0.24 out, update please
* The weird CPAN comment doesn't make any sense, either remove it or make it 
  make sense
* Please use the BuildRoot from the Packaging Guidelines
* Don't install README and COPYING under %{_datadir}/%{name} instead add them to
  %files like this: "%doc README COPYING" rpm wil then automatically create a 
  dir under /usr/share/doc for them and put them there.
* Don't use %doc for man files.


As for the ts namespace clash, I see 2 options:
1) Ship with upstreams ts name, so that we are consistent with upstream, and 
   rename to ts-stdin if an actual file conflict arises
2) Rename to ts-stdin now

I tend to prefer 1, but if we do 2 now, we avoid pain for end users if we have
to rename later.

Hi Hans,
Thanks again for that.

New SRPM:
http://marcbradshaw.co.uk/packages/moreutils-0.24-1.src.rpm

Regarding the namespace clash, I have gone with option 1 at present as this will
not require existing scripts to be re-written.  If it does need to be renamed
later so be it.  Unless you have any strong feelings to the contrary I suggest
we go with option 1.

(In reply to comment #11)
> New SRPM:
> http://marcbradshaw.co.uk/packages/moreutils-0.24-1.src.rpm
> 

Almost perfect, please put the %doc in %files under %defattr.

> Regarding the namespace clash, I have gone with option 1 at present as this will
> not require existing scripts to be re-written.  If it does need to be renamed
> later so be it.  Unless you have any strong feelings to the contrary I suggest
> we go with option 1.
> 

I'm fine with going for option 1.

p.s.

Why does this bug depend on 242311? I don't see anything provided by
perl-Time-Duration in the Requires list (including automatic requires) of moreutils?

whoops, moved to the right place now.

the ts command requires them for added functionality, it seems that as they were
being included via an eval statement rpm was missing them from the automatic
requirements list.  I have added both modules to this version as requirements.

http://marcbradshaw.co.uk/packages/moreutils-0.24-2.src.rpm

Approved, please wait with posting a CVS admin request until we're done with the
other 2 and I've sponsored you.

New Package CVS Request
=======================
Package Name: moreutils
Short Description: Additional unix utilities
Owners: deebs
Branches: F-7
InitialCC: 
Cvsextras Commits: yes

cvs done.

Package Change Request
======================
Package Name: moreutils
New Branches: EL-4 EL-5

cvs done.

Package Change Request
======================
Package Name: moreutils
Owners: deebs golfu

No new branches requested.  If an ownership change is needed, that is done
via pkgdb at https://admin.fedoraproject.org/pkgdb.  Thanks!