[Bug 892335] Review Request: AudioCuesheetEditor (v0.2.1)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=892335

--- Comment #19 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
Okay, a src.rpm that can be built. Let's try to squash some of the issues:


> %{_libdir}/%{name}/*

* Are you aware of the Mono Packaging Guidelines?

   https://fedoraproject.org/wiki/Packaging:Guidelines
   -> https://fedoraproject.org/wiki/Packaging:Mono


> BuildRequires:  gtk-sharp2,mono-core,gtk-sharp2-devel,mono-devel

A bit too many here. Typically, the -devel packages depend on their base
packages already:

  gtk2-sharp2-devel  ->  gtk2-sharp 
  mono-devel  ->  mono-core


> Requires:       gtk-sharp2,mono-core

This is still an issue (bug 880763 comment 7). There are several automatic
dependencies for Mono. Check out:

  rpm -qR AudioCuesheetEditor|grep ^mono

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


* rpmlint. You don't need to run it for the spec file when you run it for the
src.rpm, which contains the spec file.

There is also this warning: 

  AudioCuesheetEditor.src: W: strange-permission AudioCuesheetEditor.spec 0777L

$ rpmls -p AudioCuesheetEditor-0.2.1-1.fc17.src.rpm
-rw-rw-r--  AudioCuesheetEditor-src-v0.2.1.tar.gz
-rwxrwxrwx  AudioCuesheetEditor.spec


> %description

You've changed it to a much shorter one compared with 0.2.0, why not mention
that it features a graphical user-interface (GUI)? When users search for a
graphical cuesheet editor, that could be helpful.


> %install
> cp -R %{_builddir}/%{name}-%{version}/bin/Release/resources %{buildroot}%{_libdir}/%{name}/resources

> %build
> chmod a-x %{_builddir}/%{name}-%{version}/*.TXT
> chmod a-x %{_builddir}/%{name}-%{version}/bin/Release/resources/*.xml

At the beginning of each section, such as %build or %install, you are inside
the right build directory. The directory you've %setup in %prep. So, explicitly
referring to paths in %_builddir is not necessary in the whole spec file:

  chmod a-x *.TXT bin/Release/resources/*.xml

Further, many more files inside the RPM package executable. For example, the
icons and the EXE (the .exe doesn't need to be executable, does it?).
Basically, you could chmod -x many more files at the end of %prep already.


> echo '[Desktop Entry]' >> %{buildroot}%{_datadir}/applications/%{name}.desktop
> echo 'Type=Application' >> %{buildroot}%{_datadir}/applications/%{name}.desktop
> echo 'Exec=AudioCuesheetEditor' >> %{buildroot}%{_datadir}/applications/%{name}.desktop
> ...

Much more readable and much less effort to type it or change it:

cat > %{buildroot}%{_datadir}/applications/%{name}.desktop <<EOF
[Desktop Entry]
Type=Application
Exec=AudioCuesheetEditor
...
EOF


> desktop-file-install --dir=${RPM_BUILD_ROOT}%{_datadir}/applications %{buildroot}%{_datadir}/applications/%{name}.desktop

Two guidelines are relevant here:

 
https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

 
https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=N1jZSLWjbd&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]