This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 244171 - Review Request: SteGUI - SteGUI is a graphical front-end to Steghide
Review Request: SteGUI - SteGUI is a graphical front-end to Steghide
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Michel Alexandre Salim
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-14 06:52 EDT by Pierre-Yves
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version: 0.0.1-12.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-08 10:58:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
michel: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pierre-Yves 2007-06-14 06:52:27 EDT
Spec URL: http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI.spec
SRPM URL: http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI-0.0.1-5.fc6.src.rpm
Description: 
SteGUI is a graphical front-end to Steghide. It lets users view the 
images and play the sounds that Steghide allows as cover files, 
and command the program all with one tool. It also embeds a simple 
text editor to manage text payload files.


I am seeking for a sponsor
Comment 1 Tyler Owen 2007-06-14 07:28:54 EDT
A quick look at the spec file I noticed that you are not using a vendor for the
desktop-file-install command.  According to the Packaging guidelines
(http://fedoraproject.org/wiki/Packaging/Guidelines#desktop) you should use
"fedora" if upstream does not provide a vedor_id
Comment 2 Pierre-Yves 2007-06-14 09:06:53 EDT
Thank you for your look, I have made the changes needed, and I have also add the
strip -s command as I obtained a unstripped-binary-or-object error message with
rpmlint

The new src.rpm
http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI-0.0.1-6.fc6.src.rpm
The new spec file
http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI.spec

Thanks
Comment 3 Michael Schwendt 2007-06-27 07:58:30 EDT
Just a brief look at the spec, not at the src.rpm:

> Name:           SteGUI
> Summary:        SteGUI is a graphical front-end to Steghide
> Summary(fr):    SteGUI est un interface graphique pour Steghide

Repeating the package/software name in the summary is considered
bad taste often, since in many package database interfaces it
usually results in displaying the package %{name} multiple times, e.g.:
SteGUI - SteGUI is a graphical front-end to Steghide

> Requires:	fltk

A dependency on the fltk shared libs is automatically added by
rpmbuild. If not, then your package doesn't link against fltk
actually, and in that case you should add a comment in the spec file
as why it requires fltk.

> %install

Below this you're missing:  rm -rf $RPM_BUILD_ROOT

> strip -s $RPM_BUILD_ROOT%{_bindir}/%{name}

Don't strip binaries, since that makes the -debuginfo packages useless.
Comment 4 Pierre-Yves 2007-06-27 12:13:40 EDT
Based on your comment I changed the spec :-)

There are the new spec and src.rpm files:
SPEC: http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI.spec
SRPM: http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI-0.0.1-7.fc6.src.rpm

Thanks for your comment :-)
Comment 5 Michael Schwendt 2007-06-29 15:25:21 EDT
So, let's try building:

$ rpmlint SteGUI-0.0.1-7.fc6.src.rpm 
W: SteGUI strange-permission SteGUI.spec 0755
W: SteGUI mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 14)


* Package doesn't adhere to $RPM_OPT_FLAGS. The spec tries to modify
CXXFLAGS and CFLAGS, but the code compiles with different flags, e.g:

g++ -Wall -pedantic -O2 `fltk-config --cxxflags`   -c -o src/MsgWindow.o
src/MsgWindow.cc


$ desktop-file-validate /usr/share/applications/fedora-SteGUI.desktop
/usr/share/applications/fedora-SteGUI.desktop: warning: The 'Application'
category is not defined by the desktop entry specification.  Please use one of
"AudioVideo", "Audio", "Video", "Development", "Education", "Game", "Graphics",
"Network", "Office", "Settings", "System", "Utility" instead


Further: SteGUI in the "Office" category? Isn't "Utility"
more appropriate?

And in the menu entry "steGUI" is not spelled with upper-case "S".
Comment 6 Pierre-Yves 2007-06-29 16:05:50 EDT
Ok I think I corrected every things that you indicated to me :)

I removed the CFLAGS and CXXFLAGS to let the code compile with it owns

Here is the new spec:
http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI.spec

Here is the new sprm:
http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI-0.0.1-8.fc6.src.rpm

Thanks for your help :-)
Comment 7 Jason Tibbitts 2007-06-29 23:37:42 EDT
This now fails to build for me:

+ desktop-file-install --vendor=fedora --dir
/var/tmp/SteGUI-0.0.1-8.fc8-root-mockbuild/usr/share/applications/ --mode 0644
/builddir/build/SOURCES/SteGUI.desktop
/var/tmp/SteGUI-0.0.1-8.fc8-root-mockbuild/usr/share/applications/fedora-SteGUI.desktop:
error: value "0.0.1" for key "Version" in group "Desktop Entry" is not a known
version
desktop-file-install created an invalid desktop file!
error: Bad exit status from /var/tmp/rpm-tmp.61612 (%install)

The Version key contains the version of the desktop entry specification that the
particular desktop file is compliant with.  It has nothing to do with the
version of your program.  If you're going to use Version at all, you probably
want to set it to "1.0" unless you're specifically following a different version
of the specification.
Comment 8 Michael Schwendt 2007-06-30 06:38:09 EDT
> I removed the CFLAGS and CXXFLAGS to let the code compile with it owns

Instead, you need to patch it to build with $RPM_BUILD_FLAGS or
find other ways to make it accept the custom CXXFLAGS passed in from
the outside. For example:

  make CXXFLAGS="$RPM_OPT_FLAGS" CFLAGS="$RPM_OPT_FLAGS" all

would work.

See comment 5, which shows an excerpt from the build log
where the code did not accept the flags yet. Here for clarity:

CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protec
tor --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unw
ind-tables'
CXXFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-prot
ector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-u
nwind-tables'
[...]
make all
g++ -Wall -pedantic -O2 `fltk-config --cxxflags`   -c -o src/Callback.o src/Call
back.cc
[...]
gcc -O2   -c -o src/Player.o src/Player.c
Comment 9 Pierre-Yves 2007-06-30 11:00:59 EDT
OK
I changed the desktop file to a version 1.0
I change the make all to make CXXFLAGS="$RPM_OPT_FLAGS" CFLAGS="$RPM_OPT_FLAGS" all

rpmlint is clean here, and mock works (but I am not very good to find the error,
all that I saw is that the line before and after are different)

SPEC : http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI.spec
SPRM : http://pingoured.dyndns.org/public/RPM/SteGUI/SteGUI-0.0.1-9.fc6.src.rpm

Thank you for your help :-)
Comment 10 Michel Alexandre Salim 2007-09-22 13:34:26 EDT
- License guideline has been updated -- license field now needs to be GPLv2+
- Please separate changelog entries with blank lines
- Would be nice to have an icon..

I added a blocker on FE-NEEDSPONSOR, hopefully a qualified developer will see
this request
Comment 11 Pierre-Yves 2007-09-22 19:03:23 EDT
Thanks for updating this review.

I have update the license tag (sorry for forgotten to do it before)
I have change the changelog
For the icon I use one present I believe by default on the system
Icon=/usr/share/icons/hicolor/48x48/apps/esc.png

Should I had it as source, and include it in the rpm ??

Regards
Comment 12 Pierre-Yves 2007-09-22 19:13:43 EDT
Here are the new spec and srpm

SPEC
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI.spec
SRPM
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI-0.0.1-10.fc6.src.rpm

>I added a blocker on FE-NEEDSPONSOR, hopefully a qualified developer will see
>this request
I have already been sponsored... I believe I do not need this qualification any
more...

Thanks for helping any way

Regards
Comment 13 Michel Alexandre Salim 2007-09-22 21:30:23 EDT
That icon is actually from the 'esc' package -- so if you want to use it, you
should probably package it yourself. Probably need permission from the author
though. How about this image, from Wikipedia's Steganography article?

http://en.wikipedia.org/wiki/Image:Inkblot.svg

The advantage is that Wiki pictures are free of copyright restrictions so we're
safe using that.

You don't need to hardcode the path in Exec= and Icon= , so just

Exec=SteGUI
Icon=foo

is enough. Note that the extension for the icon should be left out as well, that
way if an icon theme comes along with, say, SVG icons, your desktop file does
not need updating (see http://fedoraproject.org/wiki/Packaging/Guidelines)

One last thing -- for Comment you probably want to say "Hides and extracts
information within files" (describes what the program does), and for
GenericName, "Steganography tool"? Not that GenericName is really used right now
(it might be used by the KDE panel, not sure).

Perhaps you could include French translations in the desktop file as well, to
match the French translations in the spec file?

Apart from that, ready to go. Pity that steghide itself is currently broken :(
Comment 14 Pierre-Yves 2007-09-24 03:03:32 EDT
I have made the changes that you advised me, the only one that I did not do is
the translation of the desktop file in french, I never did it.
How should I do it ?

Comment(fr)= ??
GenericName(fr)= ??

You can find the last Desktop file here :
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI.desktop

Thanks
Comment 15 Michel Alexandre Salim 2007-09-24 12:32:42 EDT
The GenericName and Comment fields are swapped. Concrete example:
Name=Firefox Web Browser
GenericName=Web Browser <= the product name has been removed
Comment=Browse the Web

So in case of SteGUI, the generic name can be just "Steganography tool" and the
comment explains what it does.

For desktop file translations, use:

Name[fr]=
GenericName[fr]=
Comment[fr]=

Also, you can leave out the extension when specifying the icon file in
SteGUI.desktop -- don't hardcode it.

Looks good -- upload the final RPM and I'll approve it.
Comment 16 Pierre-Yves 2007-09-24 13:29:25 EDT
Ok I have done the changes on the desktop file.

you can find the last version there :
SRPM
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI-0.0.1-11.fc6.src.rpm
SPEC
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI.spec
Desktop file
http://www.pingoured.fr/public/RPM/SteGUI/SteGUI.desktop

Thanks for your help :)
Comment 17 Michel Alexandre Salim 2007-09-24 15:42:43 EDT
All good! The Name[fr] in desktop is not required if the name is identical, by
the way, so you might want to chop it off when committing the spec to CVS.

APPROVED

MUST

• rpmlint: OK
• package name: OK
• spec file name: OK
• package guideline-compliant: OK
• license complies with guidelines: OK
• license field accurate: OK
• license file not deleted: OK
• spec in US English: OK
• spec legible: OK
• source matches upstream: OK
• builds under >= 1 archs, others excluded: OK
• build dependencies complete: OK
• own all directories: OK
• no dupes in %files: OK
• permission: OK
• %clean RPM_BUILD_ROOT: OK
• desktop file uses desktop-file-install: OK
• clean buildroot before install: OK
• filenames UTF-8: OK

SHOULD
• desc and summary contain translations if available: OK
• package build in mock on all architectures: OK
• package functioned as described: OK
• require package not files: OK
Comment 18 Pierre-Yves 2007-09-24 16:03:40 EDT
New Package CVS Request
=======================
Package Name: SteGUI
Short Description: SteGUI is a graphical front-end to Steghide
Owners: pingoufc4@yahoo.fr
Branches: FC-6 F-7
InitialCC: 
Cvsextras Commits: yes
Comment 19 Jason Tibbitts 2007-09-24 21:37:05 EDT
CVS done.

Note: Owners: should be your FAS ID, not your email address.  Fortunately I
remember what yours is.
Comment 20 Parag AN(पराग) 2007-09-28 03:51:56 EDT
I saw cvs commits for this package in fedora-extras-commits mailing list.
This mean package got built successfully for all requested branches.
Therefore,
closing this review ticket.
Comment 21 Fedora Update System 2007-09-28 17:20:27 EDT
SteGUI-0.0.1-12.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2007-10-08 10:58:07 EDT
SteGUI-0.0.1-12.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

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