Bug 450189 - Review Request: guake - Drop-down terminal for GNOME
Review Request: guake - Drop-down terminal for GNOME
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
: 444788 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-05 15:24 EDT by Jean-François Martin
Modified: 2011-11-09 08:23 EST (History)
9 users (show)

See Also:
Fixed In Version: 0.2.2-5.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-17 10:14:56 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jean-François Martin 2008-06-05 15:24:36 EDT
Spec URL: http://lokthare.fedorapeople.org/temp/guake.spec
SRPM URL: http://lokthare.fedorapeople.org/temp/guake-0.2.2-1.fc9.src.rpm
Description: 
Guake is a drop-down terminal for Gnome Desktop Environment,
so you just need to press a key to invoke him,
and press again to hide.
Comment 1 Jean-François Martin 2008-06-05 15:27:44 EDT
*** Bug 444788 has been marked as a duplicate of this bug. ***
Comment 2 Nicoleau Fabien 2008-06-05 15:40:21 EDT
Informal comments :
It seems that you need to build a -devel package and use %config for guake.schemas
Comment 3 Jean-François Martin 2008-06-05 19:45:49 EDT
Spec URL: http://lokthare.fedorapeople.org/temp/guake.spec
SRPM URL: http://lokthare.fedorapeople.org/temp/guake-0.2.2-2.fc9.src.rpm
- Fix gconf schema install
- Disable static library

Has i undestand in
https://fedoraproject.org/wiki/Packaging/Guidelines#Exclusion_of_Static_Libraries,
i should build a devel package for %{_libdir}/guake/globalhotkeys.la, right?
Comment 4 Chitlesh GOORAH 2008-06-08 04:47:40 EDT
#001: /etc/gconf/schemas/ should _NOT_ own /etc/gconf/schemas/

chitlesh(~)[0]$rpm -qf /etc/gconf/schemas/
GConf2-2.22.0-1.fc9.i386
guake-0.2.2-2.fc9.i386

#002: 
chitlesh(~)[0]$rpmlint guake
guake.i386: E: zero-length /usr/share/doc/guake-0.2.2/NEWS
guake.i386: W: conffile-without-noreplace-flag /etc/gconf/schemas/guake.schemas

#003: I would suggest a -libs subpackage rather than -devel subpackage. This
-libs should entail all entries under /usr/lib/

#004: In general, packagers are strongly encouraged not to ship static libs
unless a compelling reason exists.

delete %{_libdir}/guake/globalhotkeys.la after make install.
Then ensure that guake still works without the *.la file.
Comment 5 Chitlesh GOORAH 2008-06-08 04:48:28 EDT
Correction:
#001: guake should _NOT_ own /etc/gconf/schemas/
Comment 6 Jean-François Martin 2008-06-08 08:50:21 EDT
Spec URL: http://lokthare.fedorapeople.org/temp/guake.spec
SRPM URL: http://lokthare.fedorapeople.org/temp/guake-0.2.2-3.fc9.src.rpm
- Don't own /etc/gconf/schemas/
- Don't replace /etc/gconf/schemas/guake.schemas config file
- Remove globalhotkeys.la

About the third point in comment #4, i don't think a -lib subpackage is needed.
Others comments should be fixed now.
Comment 7 Chitlesh GOORAH 2008-06-08 15:08:14 EDT
(In reply to comment #6)
> About the third point in comment #4, i don't think a -lib subpackage is needed.

Mind to explain me how your decision will solve multi-libs issues ?

Comment 8 Jean-François Martin 2008-06-20 03:44:10 EDT
Can you clarify what you mean by multi-libs issues ?
Comment 9 Mamoru TASAKA 2008-06-20 04:11:58 EDT
Just a note:

We consider gconf schemas files (under %_sysconfdir/gconf/schemas)
as _not_ config files (perhaps should be moved under %_datadir,
however on Fedora they are put under %_syscondir).

So usually we don't make gconf schemas files 
have the attribute of %config (even if rpmlint warns about it).
Comment 10 Mamoru TASAKA 2008-06-28 14:18:38 EDT
By the way your srpm does not build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=685293
At least Gconf is missing from BuildRequires.

Note: as you are already sponsored, you can check if your srpm actually builds on
koji beforehand by:
$ koji build --scratch <build target> <srpm you want to try building>
where <build target> can be either dist-f10, dist-f9-updates-candidate or
dist-f8-updates-candidate.
When the build is successful, the rebuilt binary rpms and some logs are put under
http://koji.fedoraproject.org/scratch/<your FAS name>/task_<task id>/ .
Comment 11 Jean-François Martin 2008-07-01 14:42:31 EDT
Spec URL: http://lokthare.fedorapeople.org/temp/guake.spec
SRPM URL: http://lokthare.fedorapeople.org/temp/guake-0.2.2-4.fc9.src.rpm
- Add BR for GConf
- Fix schemas file

(In reply to comment #10)
> Note: as you are already sponsored, you can check if your srpm actually builds on
> koji beforehand by:
> $ koji build --scratch <build target> <srpm you want to try building>
> where <build target> can be either dist-f10, dist-f9-updates-candidate or
> dist-f8-updates-candidate.
> When the build is successful, the rebuilt binary rpms and some logs are put under
> http://koji.fedoraproject.org/scratch/<your FAS name>/task_<task id>/ .

Thanks you for the tip.
I have build guake with koji without problems.
http://koji.fedoraproject.org/koji/taskinfo?taskID=690316
Comment 12 Mamoru TASAKA 2008-07-02 03:55:26 EDT
For 0.2.2-4:

* Dependency
  - The following packages seems also needed:
    dbus-python (dbusiface.py)

* %doc
  - NEWS has zero length so please remove this from %doc.

! Note: guake-0.2.2-fix_vte.patch
  - Well, this is actually for Fedora specific...
    Actually without this package %configure because of
-------------------------------------------------------------
$ ( unset DISPLAY ; python -c "import vte" )
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: could not import gtk
--------------------------------------------------------------
    This means that when DISPLAY is not set, importing gtk
    module fails.
    However as you see %{_docdir}/pygtk2-2.12.1/NEWS
    http://bugzilla.gnome.org/show_bug.cgi?id=316877
    http://live.gnome.org/PyGTK/WhatsNew210
    this should not happen and only warnings should be printed out.

    However Fedora adds the following patch:
   
http://cvs.fedoraproject.org/viewcvs/*checkout*/rpms/pygtk2/devel/pygtk-nodisplay-exception.patch
    due to bug 208608 (which I cannot see...)
Comment 13 Pavel Alexeev 2008-07-04 05:23:27 EDT
> this should not happen and only warnings should be printed out.
But on this case too warning will not satisfies condition in configure script:
if test -z "$ac_pvte_result"

As I understood, you must patch this check too.

I'm use patch for that in my rpm packet (in My RPM-repository):
http://hubbitus.net.ru/rpm/Fedora9/guake/guake-0.2.2-1.fc9.Hu.2.src.rpm
Comment 14 Mamoru TASAKA 2008-07-04 05:48:24 EDT
(In reply to comment #13)
> > this should not happen and only warnings should be printed out.
> But on this case too warning will not satisfies condition in configure script:
> if test -z "$ac_pvte_result"

You can actually try what happens if you revert pygtk-nodisplay-exception.patch 
on pygtk :) Throwing out the warning to stdout or stderr differs.

(Again on Fedora anyway a patch is needed to deal with this issue)
Comment 15 Pavel Alexeev 2008-07-04 07:04:03 EDT
>Throwing out the warning to stdout or stderr differs.
>(Again on Fedora anyway a patch is needed to deal with this issue)
Undoubtedly. But nevertheless in your patch checking disabled compleatly (if
test -z ""; then). In my patch check is working successfully: if test -z "$(
echo $ac_pvte_result | grep -v 'could not import gtk' )"; then. In case if
occurs other error, it is not be greped.
Comment 16 Mamoru TASAKA 2008-07-04 07:22:20 EDT
(In reply to comment #15)
> Undoubtedly. But nevertheless in your patch checking disabled compleatly (if
> test -z ""; then). 
Not "my" patch...

> In my patch check is working successfully: if test -z "$(
> echo $ac_pvte_result | grep -v 'could not import gtk' )"; then. In case if
> occurs other error, it is not be greped.
Well, your patch may be better, however I leave it to the submitter
which to choose. The point is this configure check requests that 
this srpm must have "Requires: vte, pygtk2".
If a packager is aware of it, the difference of the patch does not really matter.

By the way, when adopting your patch, please keep in mind that your patch
adds some BuildRequires (perhaps you are usually using Mandriva, however
Fedora people are very sensitive to Requires/BuildRequires and many other packging
issue ).

Comment 17 Pavel Alexeev 2008-07-04 07:57:22 EDT
(In reply to comment #16)

> this srpm must have "Requires: vte, pygtk2".
I'm right. I'm add "Requires: vte" dependency.
> If a packager is aware of it, the difference of the patch does not really matter.
In this case very likely.
> 
> By the way, when adopting your patch, please keep in mind that your patch
> adds some BuildRequires
What?? Patch "add" only dependency "grep", but it is now successfully used in
configure script without my patch. So, my patch do not add any additional
dependency!

>(perhaps you are usually using Mandriva, however Fedora people are very
sensitive to Requires/BuildRequires and many other packging issue ).

No, no, no. I'm Fedora user is not one year. Did not even try Mandriva. You make
this assumption by macroses like
%post_install_gconf_schemas/%preun_uninstall_gconf_schemas?? Yes, this macroses
comes from Mandriva (as I remember, it is from internet) it is just usefull
macroses, I'm may replace it's by hardcoded instructions, if will need.
Comment 18 Mamoru TASAKA 2008-07-04 08:15:03 EDT
(In reply to comment #17)
> > By the way, when adopting your patch, please keep in mind that your patch
> > adds some BuildRequires
> What?? Patch "add" only dependency "grep", but it is now successfully used in
> configure script without my patch. So, my patch do not add any additional
> dependency!

You are confusing "Requires" vs "BuildRequires" by the word "dependency".

"BuildRequires (here not mentioning about Requires): pygtk2, vte" 
are also needed if your patch is applied.
With the patch by Jean-François, "BuildRequires (not Requires) pygtk2, vte" is
_not_ needed. (*Requires*: pygtk2, vte is needed anyway)

> >(perhaps you are usually using Mandriva, however Fedora people are very
> sensitive to Requires/BuildRequires and many other packging issue ).
> 
> No, no, no. I'm Fedora user is not one year. Did not even try Mandriva. You make
> this assumption by macroses like
> %post_install_gconf_schemas/%preun_uninstall_gconf_schemas?? Yes, this macroses
> comes from Mandriva (as I remember, it is from internet) it is just usefull
> macroses, I'm may replace it's by hardcoded instructions, if will need.

  I guessed so because of the line
"--add-category="X-MandrivaLinux-System-Terminals"

Comment 19 Pavel Alexeev 2008-07-05 07:45:16 EDT
(In reply to comment #18)
> "BuildRequires (here not mentioning about Requires): pygtk2, vte" 
> are also needed if your patch is applied.
> With the patch by Jean-François, "BuildRequires (not Requires) pygtk2, vte" is
> _not_ needed. (*Requires*: pygtk2, vte is needed anyway)
Ok, thank you for detailed explanation.
But, it is not my patch *add* this BuildRequires (which I added in my spec), my
patch *leave* it "as is" opposite cutting off it by patch by Jean-François!!

>   I guessed so because of the line
> "--add-category="X-MandrivaLinux-System-Terminals"
Thanks, it is too corrected.

Comment 20 Mamoru TASAKA 2008-07-09 02:15:42 EDT
ping?
Comment 21 Pavel Alexeev 2008-07-09 11:11:19 EDT
For recent versions (for configure script generated by autogen.sh) has to be
added BuildRequires: pygtk2-devel (pkg-config used)

Also, for git-builds has to be added BuildRequires: gnome-common intltool
autoconf automake libtool pkgconfig
Comment 22 Jean-François Martin 2008-07-09 19:30:14 EDT
Spec URL: http://lokthare.fedorapeople.org/temp/guake.spec
SRPM URL: http://lokthare.fedorapeople.org/temp/guake-0.2.2-5.fc9.src.rpm

- Remove NEWS from the doc
- Add dbus-python in Requires
Comment 23 Mamoru TASAKA 2008-07-10 10:54:59 EDT
------------------------------------------------------------------------------
       This package (guake) is APPROVED by me
------------------------------------------------------------------------------
Comment 24 Jean-François Martin 2008-07-11 06:03:18 EDT
New Package CVS Request
=======================
Package Name: guake
Short Description: Drop-down terminal for GNOME
Owners: lokthare
Branches: F-9
InitialCC: 
Cvsextras Commits: yes
Comment 25 Kevin Fenzi 2008-07-11 12:17:02 EDT
cvs done.
Comment 26 Fedora Update System 2008-07-11 15:14:04 EDT
guake-0.2.2-5.fc9 has been submitted as an update for Fedora 9
Comment 27 Fedora Update System 2008-07-17 10:14:53 EDT
guake-0.2.2-5.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 28 Pierre-Yves 2008-07-25 19:11:52 EDT
Could you build it for F-8 ??

Thanks
Comment 29 Pierre-YvesChibon 2008-08-14 05:04:13 EDT
New branch CVS Request
=======================
Package Name: guake
Short Description: Drop-down terminal for GNOME
Owners: pingou
Branches: F-8
InitialCC: 
Cvsextras Commits: yes
Comment 30 Toshio Kuratomi 2008-08-23 15:10:20 EDT
cvs done.
Comment 31 Pavel Alexeev 2008-08-27 06:57:24 EDT
Guake 0.3.1 released will you update it?
Comment 32 Pierre-YvesChibon 2008-08-27 07:07:12 EDT
It is already built and waiting to be pushed in updates-testing
Comment 33 Pierre-YvesChibon 2011-11-09 05:01:08 EST
Package Change Request
======================
Package Name: guake
New Branches: el6
Owners: pingou
Comment 34 Gwyn Ciesla 2011-11-09 08:23:20 EST
Git done (by process-git-requests).

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