Bug 170978 - Review Request: nomadsync
Review Request: nomadsync
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael A. Peters
David Lawrence
http://nomadsync.sourceforge.net/
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-10-16 16:19 EDT by Linus Walleij
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-15 17:13:20 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Linus Walleij 2005-10-16 16:19:03 EDT
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-2.src.rpm
Description: 

This package provides an application to synchronize music trees
with Creative NOMAD Jukebox, Creative Zen and Dell DJ line of
MP3 players.

Patched to add a .desktop file in order to conform with FE 
packaging guidelines.
Comment 1 Bastien Nocera 2005-11-10 14:27:18 EST
Is id3lib already shipped in the extras?
Also, you could simply have your .desktop file as another Source:
Source1:	nomadsync.desktop
Comment 2 Ville Skyttä 2005-11-10 16:07:25 EST
(In reply to comment #1) 
> Is id3lib already shipped in the extras? 
 
Yes. 
Comment 3 Linus Walleij 2005-11-16 16:17:06 EST
It's not id3lib it's libid3tag (there is also taglib and libmetatag somehwere
just to add to your confusion) and yes, that is already in the extras too.

I switched to using a Source1: line instead of a patch, much smarter, indeed.

Fixed package:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-4.src.rpm
Comment 4 Linus Walleij 2005-11-16 16:18:23 EST
EH sorry yes, no its id3lib not libid3tag and yes as pointed out its in
the extras already. I was drunk... or rather... well.
Comment 5 Michael A. Peters 2005-11-29 15:27:41 EST
In %install section - please change

%makeinstall

to

make DESTDIR=%buildroot install

I don't know if it is issue here, but I've had issue with the former before that
don't happen with the latter (strings including the buildroot in files)

I tested the change - it works.

%defattr(-, root, root)

please change to

%defattr(-,root,root,-)

rpmlint errors (after mock build):

[mpeters@utility result]$ rpmlint *.rpm
E: nomadsync script-without-shellbang /usr/share/doc/nomadsync-0.4.2/copying
E: nomadsync script-without-shellbang /usr/share/doc/nomadsync-0.4.2/ChangeLog
E: nomadsync wrong-script-end-of-line-encoding
/usr/share/doc/nomadsync-0.4.2/ChangeLog
E: nomadsync script-without-shellbang /usr/share/doc/nomadsync-0.4.2/install
E: nomadsync script-without-shellbang /usr/share/doc/nomadsync-0.4.2/readme
E: nomadsync wrong-script-end-of-line-encoding /usr/share/doc/nomadsync-0.4.2/readme
E: nomadsync script-without-shellbang /usr/share/doc/nomadsync-0.4.2/authors
[mpeters@utility result]$ 

The "script-without-shellbang" errors can be fixed by removing execution bit on
those files.
The "wrong-script-end-of-line-encoding" errors can be fixed with sed.

In %prep after extraction do something like:

%{__sed} -i 's?\r??' ChangeLog
%{__sed} -i 's?\r??' readme

-=-
Please don't %doc the install file.
It is already installed once the user installs the rpm.
Comment 6 Linus Walleij 2005-12-18 02:37:40 EST
I fixed these things, but no cigar, because now it doesn't compile under
GCC 4.0.2 so more things need to be done...
Comment 7 Linus Walleij 2005-12-18 02:39:35 EST
Hm the package is set into review, but who reviews it? It is still assigned
to Greg, which is also default. Greg, is it really you who review this package?
Comment 8 Michael A. Peters 2005-12-18 07:27:07 EST
I think I set it to review and forgot to set it to me.
Comment 9 Linus Walleij 2005-12-18 17:00:40 EST
OK thanks Michael, I'll get back as soon as I find a proper patch for
GCC 4.0.2.
Comment 10 Linus Walleij 2006-01-14 03:19:52 EST
Fixed package:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-5.src.rpm

Turns out there was no compiler issue, it was the unicodification of wxGTK
2.6 that was the culprit, so this requires 2.4.x. I saw that package was 
pulled from Extras anyway... Will need some compat package in devel I guess?
Comment 11 Linus Walleij 2006-01-14 15:35:29 EST
I figure I will replace wxGTK-devel for compat-wxGTK or compat-wxGTK2 in
the devel (FC5) version. Do you want me to provide a separate spec for
devel?
Comment 12 Michael A. Peters 2006-01-14 18:41:55 EST
That would be best.
Comment 13 Linus Walleij 2006-01-15 16:27:57 EST
Fixed package:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-6.src.rpm

I actually detect FC version instead since it's a oneliner. Doesn't hurt to
have a generic specfile in this case.
Comment 14 Michael A. Peters 2006-01-15 16:58:42 EST
Would it better to simply

BuildRequires %{_bindir}/wxgtk-2.4-config 

rather than fedora specific macros ??

How does the 64-bit package for wxgtk-devel handle that? (can 32 and 64 bit be
|| installed?)
Comment 15 Linus Walleij 2006-03-07 16:14:06 EST
Yes, that works much better. Sorry for fat delay on this, I missed it somehow:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-7.src.rpm
Comment 16 Michael A. Peters 2006-03-08 07:25:36 EST
Good:

* Builds fine in mock (fc4 i386)
* rpmlint clean
* owns all directories it installs
* proper rpm Group
* clean rpm spec file
* proper summary and description in American English
* spec file understabdable
* proper and consistent use of macros
* proper installation of Desktop file, with X-Fedora category and fedora vendor
* proper permissions on files

BAD
md5sum in src.rpm: f8d50c14fae3f8cdedc57c491c7d65d4 
../SOURCES/nomadsync-0.4.2.tar.gz
md5sum upstream: fd55927c54cd0737cdecb88e4bfbf0b7  nomadsync-0.4.2.tar.gz

-=-
looking at the two - nomadsync is upstream source unpacked, nomadsync-0.4.2.src
is from the src.rpm

[mpeters@jerusalem delete_me]$ diff -u nomadsync nomadsync-0.4.2.src
Only in nomadsync-0.4.2.src: Makefile
Only in nomadsync-0.4.2.src: config.log
Only in nomadsync-0.4.2.src: config.status
Common subdirectories: nomadsync/cvs and nomadsync-0.4.2.src/cvs
Common subdirectories: nomadsync/debug and nomadsync-0.4.2.src/debug
Common subdirectories: nomadsync/doc and nomadsync-0.4.2.src/doc
Only in nomadsync-0.4.2.src: libtool
Common subdirectories: nomadsync/macros and nomadsync-0.4.2.src/macros
Common subdirectories: nomadsync/optimized and nomadsync-0.4.2.src/optimized
Common subdirectories: nomadsync/src and nomadsync-0.4.2.src/src
Common subdirectories: nomadsync/templates and nomadsync-0.4.2.src/templates
[mpeters@jerusalem delete_me]$ 

It looks like the source tarball you have had configure run in it before it was
archived.

Use the pristine upstream source tarball.
Comment 17 Linus Walleij 2006-03-08 14:38:53 EST
Not strange since the upstream source was updated today (file dated 2005-03-08),
updated to latest upstream:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/nomadsync-0.4.2-8.src.rpm
Comment 18 Michael A. Peters 2006-03-08 20:08:03 EST
Hey yeah - you're right.
Odd they did not version tarball ...

md5sum matches now.

approved.
Comment 19 Linus Walleij 2006-03-15 17:13:20 EST
OK (after some patching on devel) it now builds on all archs.

Michael: THANK YOU for reviewing this RPM!

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