Bug 223422 - Review Request: mrxvt - Multi-tabbed terminal emulator.
Review Request: mrxvt - Multi-tabbed terminal emulator.
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
: 223421 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-18 22:45 EST by Adam M. Dutko
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version: 0.5.2-9.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-11 11:20:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
Latest RPM (762.29 KB, application/x-rpm)
2007-05-21 07:11 EDT, Adam M. Dutko
no flags Details
mock build log of mrxvt-0.5.2-1.fc8.4 on F-devel i386 (59.79 KB, text/plain)
2007-06-05 12:38 EDT, Mamoru TASAKA
no flags Details
normal rpmbuild log of mrxvt-0.5.2-1.fc8.4 (34.80 KB, text/plain)
2007-06-05 12:41 EDT, Mamoru TASAKA
no flags Details
mock build log of mrxvt 0.5.2-6 on F-devel i386 (4.34 KB, text/plain)
2007-06-15 02:13 EDT, Mamoru TASAKA
no flags Details
spec file with some cleanup (3.07 KB, text/plain)
2007-06-17 11:19 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description Adam M. Dutko 2007-01-18 22:45:33 EST
Spec URL: http://littlehat.homelinux.org:8000/RPMS/mrxvt.spec
SRPM URL: http://littlehat.homelinux.org:8000/RPMS/mrxvt-0.5.2-1.src.rpm
Description: Mrxvt (previously named materm) is a lightweight, powerful multi-tabbed terminal emulator for the X window system. mrxvt is based on rxvt version 2.7.11 CVS and aterm. It implements many useful features seen in some modern X terminal emulators (like gnome-terminal and konsole) but aims to be fast, lightweight and independent of standard toolkits or desktop environments (e.g. Gnome / KDE).  NOTE: I have the permission from the maintainer of mrxvt to submit this package for review.  It has been tested on FC6, but not on Fedora Core versions < FC6.  This is my first package and I need a sponsor: PLEASE.
Comment 1 Adam M. Dutko 2007-01-18 23:01:38 EST
*** Bug 223421 has been marked as a duplicate of this bug. ***
Comment 2 Jeremy Hinegardner 2007-03-27 02:08:01 EDT
This is not an official review as I am not able to sponsor you. But this can get
you started. There are a considerable number of items at issue here.

You may want to read http://fedoraproject.org/wiki/Packaging/Guidelines

1) Source in src.rpm does not match upstream

3144a0cc066348557d2613b5cb2b4d14  mrxvt-0.5.2.tar.gz
ed87b7dd9f4fb482de0f14f085085027  mrxvt-0.5.2.tar.gz.1 (upstream)

2) Spec file and macros. I would suggest using rpmdevtools and starting
   anew with a fresh spec.

 a) %{buildroot} not %buildroot

 b) make use of %{_bindir}, %{_datadir}, %{_sysconfdir} in %files.

 c) all items listed in %files that are in /usrs/share/doc should be listed via %doc

 d) Items in /etc/ should be marked with %config 

 e) %defattr is required for %files section

 f) Source0 should be the full URL to the upstream source, in this case
 most likely
 http://downloads.sourceforge.net/materm/%{name}-%{version}.tar.gz

 Please see http://fedoraproject.org/wiki/Packaging/SourceURL

3) %clean must contain rm -rf %{buildroot}

4) mrxvt is a gui application and as such requires a .desktop file.

    http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

5) Fails to build in Mock.  This appears to be because if incorrect BuildRequires.


Comment 3 Adam M. Dutko 2007-03-27 23:52:48 EDT
Thanks for the tips.  I've been in contact with the maintainer and need to
discuss some possible source tree modifications. 
Comment 4 Mamoru TASAKA 2007-04-28 11:54:29 EDT
What is the status of this bug report?
Comment 5 Mamoru TASAKA 2007-05-09 13:58:55 EDT
ping again?
Comment 6 Mamoru TASAKA 2007-05-19 06:32:43 EDT
Again ping?
Comment 7 Jeremy Hinegardner 2007-05-20 19:53:05 EDT
I assume that Adam is talking with uptstream about some issues.  
Comment 8 Adam M. Dutko 2007-05-21 07:11:21 EDT
Created attachment 155086 [details]
Latest RPM
Comment 9 Adam M. Dutko 2007-05-21 07:12:28 EDT
[@laptop FEDORA]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-05b-1.i386.rpm
E: mrxvt description-line-too-long  Mrxvt (previously named materm) is a
lightweight, powerful multi-tabbed terminal emulator for the X window system.
mrxvt is based on rxvt version 2.7.11 CVS and aterm. Mrxvt aims to be fast,
lightweight and independent of standard toolkits or desktop environments (e.g.
Gnome / KDE).
W: mrxvt incoherent-version-in-changelog 0.5.2-1 05b-1
W: mrxvt unstripped-binary-or-object /usr/bin/mrxvt
W: mrxvt wrong-file-end-of-line-encoding /usr/share/doc/mrxvt/scripts/mrxvt.vbs
W: mrxvt non-standard-dir-in-usr man
W: mrxvt non-standard-dir-in-usr etc

The above is the most recent rpmlint output.  I checked out the latest svn
source and built another rpm.  The first two errors are easy to fix.  Since this
is my first "formal" rpm and everyone became interested really quickly after
such a lull... :-)  I will strive to determine how I should fix the remaining
three errors.  Ideas/suggestions are definitely welcome!!!  You can find the
latest rpm build in the attachment above this post.
Comment 10 Mamoru TASAKA 2007-05-21 08:29:43 EDT
We cannot check what is the problem until the spec/srpm
is provided. 

Please provide the URL where we can download your
newest srpm (and please don't "attach" srpm to bugzilla)
Comment 11 Adam M. Dutko 2007-05-21 08:51:12 EDT
I will implement the suggestions Jeremy suggested this evening once I get home
from work and will provide a URL for the spec and srpm.  Again, sorry for the wait.
Comment 12 Adam M. Dutko 2007-05-21 22:29:01 EDT
OK I've tried to implement a few things Jeremy suggested but I'm running into
some issues... 

You can find the latest build, spec file and desktop file here...
http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/

latest rpmlint...

[@laptop FEDORA]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-05b-1.i386.rpm
E: mrxvt description-line-too-long Mrxvt (previously named materm) is based on
rxvt version 2.7.11 CVS and aterm. mrxvt aims to be independent of a user's
desktop environment.
W: mrxvt wrong-file-end-of-line-encoding /usr/share/doc/mrxvt/scripts/mrxvt.vbs
W: mrxvt non-standard-dir-in-usr man
W: mrxvt non-standard-dir-in-usr etc

I took the latest svn snapshot from sourceforge and built this rpm, but it still
has some problems...  In particular I think the files in /share and /doc need to
be moved around (those are the src tree modifications I alluded to earlier). 
Due to my inexperience with rpm building and the sparse (somewhat unreliable)
info on building them on the fedora wiki, I'm not comfortable recommending these
changes to the maintainer without specific reasons; I've conversed with the
maintainer once and only hinted that the source tree might require changes --
beyond that I've been busy with school and now am on summer break, and ready to
further that discussion with him.  Any ideas? suggestions?  Thanks for the help.
Comment 13 Mamoru TASAKA 2007-05-22 12:28:31 EDT
Well, I saw your spec file and actually there are
many points to be fixed.

(In reply to comment #12)
> I'm not comfortable recommending these
> changes to the maintainer without specific reasons;

I don't see the reason for now why upstream should change
directory tree of installation... For me every files are 
installed into where Fedora rpm packaging expects *when I compiled 
mrxvt correctly*. For this (mrxvt) it is actually due to the 
rpm packaging issue that files are not installed into the correct
directories.

So, first check the following wiki page first:

http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/NamingGuidelines

and follow fedora rpm packaging way. And you can start from
the spec file which already exists in Fedora, for example:

http://cvs.fedora.redhat.com/viewcvs/*checkout*/devel/xterm/xterm.spec?root=core
Comment 14 Adam M. Dutko 2007-05-31 07:06:17 EDT
OK.  I've followed the guidelines and built new rpms.  Please find the new rpms,
spec file, and desktop file here:

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/0.5.2-1/

Also, here is the latest rpmlint:

[a@laptop mrxvt_devel]$ sudo rpmlint
/usr/src/redhat/RPMS/i386/mrxvt-0.5.2-1.i386.rpm
W: mrxvt incoherent-version-in-changelog 1 0.5.2-1
W: mrxvt wrong-file-end-of-line-encoding /usr/share/doc/mrxvt/scripts/mrxvt.vbs
W: mrxvt non-conffile-in-etc /etc/mrxvt/mrxvtrc.sample
W: mrxvt non-conffile-in-etc /etc/mrxvt/mrxvtrc
W: mrxvt non-conffile-in-etc /etc/mrxvt/submenus.menu
W: mrxvt non-conffile-in-etc /etc/mrxvt/default.menu

I believe the changelog version warning is OK considering the sources are for
0.5.2 and we don't have a minor release number (Am I incorrect?).  The
end-of-line-encoding is because it's a visual basic script -- this should be
fine b/c it shouldn't kill the build process, correct?  The non-conffile
errors...is that a big deal??? considering a user should know where to grab
example configuration information?  I know it doesn't conform to the LSB.  Is
there a precedent for including this? or should they be moved?  If they need to
be moved, can someone suggest a proper location?  As always, thanks for the help.
Comment 15 Mamoru TASAKA 2007-05-31 12:14:26 EDT
Well, much improvement!! However, still some points
to be fixed.

* %changelog
  - Please use changelog properly. i.e. If you change/modify
    spec file/srpm, write a brief summary in %changelog (see
    specs of other packages for example)
  - And please increment release number when you modify spec file
    with version unchanged.

* sourceURL
  - For Source0, please refer to
    http://fedoraproject.org/wiki/Packaging/SourceURL

* Unused definition
  - Please remove unused definition. It doesn't seem that 
    %x11_app_defaults_dir is used anywhere.

* Parallel make
  - Please support parallel make when possible (please check
    the section "Parallel make" of
    http://fedoraproject.org/wiki/Packaging/Guidelines

* attr
  - Usually, %attr(755,root,root) for binary should not be needed
    when installation is done properly.

* Directory ownership
  - Please own directories which are created by this package and
    are not owned by other packages. For example, this package
    should own directory %{_sysconfdir}/%{name}

* Macros
  - Please use macros. %{_sysconfdir} for /etc, for example.

* noreplace for config files
  - For config files, please use %config(noreplace)

* documentatin directory
  - Documentations should be under %{_datadir}/doc/%{name}-%{version}.
Comment 16 Adam M. Dutko 2007-06-03 16:18:20 EDT
Latest rpmlint:

[a@laptop mrxvt_devel]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-0.5.2-1.2.i386.rpm
W: mrxvt incoherent-version-in-changelog 1.2 0.5.2-1.2
W: mrxvt wrong-file-end-of-line-encoding /usr/share/doc/mrxvt/scripts/mrxvt.vbs

Latest spec and rpms can be found here (I'm trying to create a "stable" branch
rpm too.): http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/

I believe I implemented all you asked with two exceptions:

1) Directory ownership -- I'm not sure how you'd like me to, but I will continue
to look for examples.
2) I'm still getting a changelog "barf."  Not sure how to go about fixing this,
despite incrementing what I believe to be "correct."  

As always, thanks for the help.  Now, what to do about that pesky vb script?
Comment 17 Adam M. Dutko 2007-06-03 22:08:02 EDT
I've also completed an rpm for the "stable" 0.4.2 branch.  The spec file, srpm
and rpm can also be found at:
http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/ 

Cheers.
Comment 18 Mamoru TASAKA 2007-06-04 02:44:30 EDT
Well, for 0.5.2-1.2:

* Souce URL
  - The format of URL for Source0 and Patch1 is still incorrect.
    For sources on sourceforge.net, again please refer to
    http://fedoraproject.org/wiki/Packaging/SourceURL
    (especially, the section "Sourceforge.net")

* Directory ownership
  - What I want to say is that the directory %{_sysconfdir}/mrxvt/
    is not owned by this package. 
-------------------------------------------------
[root@localhost ~]# LANG=C rpm -qf /etc/mrxvt
file /etc/mrxvt is not owned by any package
-------------------------------------------------

You must add:
-----------------------------------------------
%dir %{_sysconfdir}/mrxvt
-----------------------------------------------
    to %files list. i.e. You must use:
-----------------------------------------------
%dir %{_sysconfdir}/%{name}
%config(noreplace) %{_sysconfdir}/%{name}/default.menu
%config(noreplace) %{_sysconfdir}/%{name}/mrxvtrc
%config(noreplace) %{_sysconfdir}/%{name}/mrxvtrc.sample
%config(noreplace) %{_sysconfdir}/%{name}/submenus.menu
-----------------------------------------------
   ... or just:
-----------------------------------------------
%config(noreplace) %{_sysconfdir}/%{name}/
-----------------------------------------------

NOTE: 
--------------------------------------------
%files
foo/
--------------------------------------------
   (where foo/ is a directory) means the directory foo/ itself
   and all files/directories/etc.. under foo/, while
--------------------------------------------
%files
%dir foo/
--------------------------------------------
   means the directory foo/ only.
   - Also, the directory %{_datadir}/doc/%{name}/scripts/ should
     be owned by this package.

* Macros
  - "%{_prefix}/share" should be %{_datadir}.
  - And,
---------------------------------------------
%{_prefix}/share/pixmaps/mrxvt-csh.png
%{_prefix}/share/pixmaps/mrxvt-csh.xpm
%{_prefix}/share/pixmaps/mrxvt-root.png
%{_prefix}/share/pixmaps/mrxvt-root.xpm
%{_prefix}/share/pixmaps/mrxvt.png
%{_prefix}/share/pixmaps/mrxvt.xpm
----------------------------------------------
    can be replaced with
----------------------------------------------
%{_datadir}/pixmaps/%{name}*
----------------------------------------------

* Documentation
  - And all the files under %{_datadir}/doc/%{name} must be moved
    to %{_datadir}/doc/%{name}-%{version}.

* Unneeded macros, comments
  - Remove unneeded macros and comments. 
    * It doesn't seem that the macro %enable_trace is used
      anywhere.

* Changelog formats
  - The correct format is like:
------------------------------------------------------
* Thu May 31 2007 Adam M. Dutko <gnome at dux-linux org> - 0.5.2-1.1
- Updated spec file per suggestions in bug #: 223422
------------------------------------------------------
  - And the release number should be integer + %{?dist}.
    (although there are some exceptions: please check:
     "Package Release" of
     http://fedoraproject.org/wiki/Packaging/NamingGuidelines )
Comment 19 Adam M. Dutko 2007-06-04 07:51:43 EDT
Latest rpmlint:

[a@laptop mrxvt_devel]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-0.5.2-1.3.i386.rpm
W: mrxvt wrong-file-end-of-line-encoding /usr/share/doc/mrxvt/scripts/mrxvt.vbs

1)Source URL -- FIXED
2)Directory Ownership -- FIXED
3)Documentation -- NOT FIXED b/c I get rpmbuild errors when I use
%{name}-%{version} instead of just %{name}.  When I look under /var/tmp/... in
the active build area, it's because the documents do not exist under
%{name}-%{version}.  Do I have to move them in the source tree to fix this?
4)Unneeded Macros -- FIXED
5)Changelog Formats -- FIXED

I've also implemented a new spec file for the 0.4.2 branch.  

I've also incremented the release stamp like the directions told me to for each
change to the spec file.  I'm going to work on getting the "dist" tag to work,
but at the moment, the fedoraproject.org/wiki is down.

Thank you again for your help and patience.

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/
Comment 20 Adam M. Dutko 2007-06-04 07:52:51 EDT
One more thing, the reccomendation of integer + %{?dist} fails on rpmbuild,
which is why I did what I did in the spec file for the Release tag.
Comment 21 Mamoru TASAKA 2007-06-04 12:23:04 EDT
(In reply to comment #19)
> 1)Source URL -- FIXED
> 2)Directory Ownership -- FIXED
Okay.

> 3)Documentation -- NOT FIXED 
For this, please once remove all files under 
%{_datadir}/doc/%{name} by:
-----------------------------------------------
%install
rm -rf $RPM_BUILD_ROOT
make DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" install 

desktop-file-install --vendor=fedora \
        --dir=$RPM_BUILD_ROOT%{_datadir}/applications \
        %{SOURCE1}

rm -rf $RPM_BUILD_ROOT%{_datadir}/doc
------------------------------------------------
  and later add the proper documents by:
------------------------------------------------
%files
%defattr(-,root,root,-)
%doc doc/README* doc/*.txt*
%doc share/scripts/
%doc AUTHORS ChangeLog COPYING NEWS README TODO
------------------------------------------------
Here two modification is added:
* Timestamps
  - This installs some text/image files which are not modified
    or created during build stage and keeping timestamps on
    these files is recommended.
    For this package, this can be done by adding 
    'INSTALL=%{__install} -p'

* Unneeded documents
  - "INSTALL" file is for people who wants to rebuild this
    by themselves and so it is not needed for rpm file.

> 4)Unneeded Macros -- FIXED
> 5)Changelog Formats -- FIXED
  - Well, please write a bit more precisely.. not just
    "updated spec file"...

(In reply to comment #20)
> One more thing, the reccomendation of integer + %{?dist} fails on rpmbuild,
> which is why I did what I did in the spec file for the Release tag.

- What do you mean by this?
  Usually release number should be:
-----------------------------------------------
Release: 2%{?dist}
-----------------------------------------------
  and the corresponding %changelog entry is:
-----------------------------------------------
* Sun Jun 3 2007 Adam M. Dutko <gnome at dux-linux org> - 0.5.2-2
- <some description>
------------------------------------------------

Comment 22 Adam M. Dutko 2007-06-04 22:05:27 EDT
Latest rpmlint:

[a@laptop mrxvt_devel]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-0.5.2-1.4.i386.rpm
W: mrxvt wrong-file-end-of-line-encoding
/usr/share/doc/mrxvt-0.5.2/scripts/mrxvt.vbs

I've also posted the current spec and rpms:

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/

With regard to the integer + %{?dist} comment, I simply misunderstood you...  I
thought you meant I should write:

Release: integer + %{?dist}

but you meant what you wrote above...  I assume the current way I'm using it is
correct?  Hopefully this time the spec file is good.

I do need to do three things:

1) Verify the buildrequires again...
2) Verify I can use this spec for build from the svn repository.
3) Apply a new patch from the maintainer that fixes a bug.
Comment 23 Mamoru TASAKA 2007-06-05 12:35:56 EDT
(In reply to comment #22)
> Latest rpmlint:
> 
> [a@laptop mrxvt_devel]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-0.5.2-1.4.i386.rpm
> W: mrxvt wrong-file-end-of-line-encoding
> /usr/share/doc/mrxvt-0.5.2/scripts/mrxvt.vbs

Fix this by:
--------------------------------------------------
sed -i 's|\r||' scripts/mrxvt.vbs
--------------------------------------------------
in %prep stage.

 
> With regard to the integer + %{?dist} comment, 
> I assume the current way I'm using it is
> correct?  
Still not. What I wanted to say is that please write release
number as
------------------------------------------
Release: 2%{?dist}
------------------------------------------
or
------------------------------------------
Release: 3%{?dist}
------------------------------------------
and not
------------------------------------------
Release: 1%{?dist}.4 
------------------------------------------

> I do need to do three things:
> 
> 1) Verify the buildrequires again...
mockbuild succeeds, however there seems to be some
missing BR.
( I attach the difference )

> 2) Verify I can use this spec for build from the svn repository.
I don't know what you want to say here, but please
don't use svn version source if possible.

> 3) Apply a new patch from the maintainer that fixes a bug.
Also I don't know what you want to say here...

Comment 24 Mamoru TASAKA 2007-06-05 12:38:05 EDT
Created attachment 156242 [details]
mock build log of mrxvt-0.5.2-1.fc8.4 on F-devel i386

The build log by mockbuild
Comment 25 Mamoru TASAKA 2007-06-05 12:41:45 EDT
Created attachment 156243 [details]
normal rpmbuild log of mrxvt-0.5.2-1.fc8.4

And normal rpmbuild log:

The important difference is:
---------------------------------------------------
@@ -397,10 +83,10 @@
 checking for XRegisterIMInstantiateCallback in -lX11... yes
 checking for SmcOpenConnection in -lSM... yes
 checking for IceConnectionNumber in -lICE... yes
-checking for utempter_add_record in -lutempter... no
+checking for utempter_add_record in -lutempter... yes
 checking for XpmCreateImageFromXpmImage in -lXpm... yes
-checking for jpeg_read_header in -ljpeg... no
-checking for png_check_sig in -lpng... no
+checking for jpeg_read_header in -ljpeg... yes
+checking for png_check_sig in -lpng... yes
 checking for XRenderCreatePicture in -lXrender... yes
 checking for fontconfig/fontconfig.h... yes
 checking for X11/Xft/Xft.h... yes
@@ -617,7 +303,7 @@
 Support wtmp records	     : yes
 Support lastlog records     : yes
 Support X session manager   : yes
-Use utempter library	     : no
+Use utempter library	     : yes
 
 Visual features:
 Support line space	     : yes
@@ -632,8 +318,8 @@
 Background features:
 Background image	     : yes
 XPM background image	     : yes
-JPEG background image	     : no
-PNG background image	     : no
+JPEG background image	     : yes
+PNG background image	     : yes
 Transparent background      : yes
 Background tinting	     : yes (XRender)
 
---------------------------------------------------
Perhaps libpng-devel, libjpeg-devel, libutempter-devel is missing
for BR.
Comment 26 Adam M. Dutko 2007-06-12 10:19:01 EDT
Sorry, but I've been a bit busy this past week.  I will post a new spec file,
and rpms this evening on my site, then post a message to this board when they're
ready.  Thanks.
Comment 27 Adam M. Dutko 2007-06-12 22:25:44 EDT
Latest rpmlint:
[a@laptop mrxvt_devel]$ rpmlint /usr/src/redhat/RPMS/i386/mrxvt-0.5.2-6.i386.rpm
[a@laptop mrxvt_devel]$

Nothing is there! YEAH!!!!

> 2) Verify I can use this spec for build from the svn repository.
I am going to start creating nightly rpm builds for the maintainer.  This is
"off-topic."  Just wanted to let you know it's a work in progress.

> 3) Apply a new patch from the maintainer that fixes a bug.
There are a few bug fixes the maintainer would like added. Again, I will do this
when I get the nightly builds running.

** I've added the extra build requires.
** I've added more comments and incremented the release number accordingly.
** I've fixed the end of line-encoding issue in the %prep section.

Everything is up on my site: 

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/0.5.2/

Please let me know if there are other outstanding issues that need to be
corrected? and if not, what steps need to be taken to get this included in the
F8 build process.  Thank you for all the help.
Comment 28 Mamoru TASAKA 2007-06-13 13:10:39 EDT
(This review request is currently blocked by desktop-file-utils
 0.13 issue)
Comment 29 Mamoru TASAKA 2007-06-15 02:13:10 EDT
Created attachment 157060 [details]
mock build log of mrxvt 0.5.2-6 on F-devel i386

The rebuild (on F-devel with mock) failed with
newer desktop-file-utils (0.13)

(Well, I didn't know that)
the entry "Version" is the version of "Desktop Entry Standard"
you used (see: http://standards.freedesktop.org/desktop-entry-spec/ ),
so
* This version value has no relation with the version of the
  application
* The value of Version should be either:
  0.9.[3-8] or 1.0
* And the Entry Version is *not needed*
So please remove the Version entry.
Comment 30 Adam M. Dutko 2007-06-16 17:49:39 EDT
Ok.  Removed the version entry in the desktop file, added comments in the spec,
incremented version and rebuilt.

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/0.5.2/
Comment 31 Mamoru TASAKA 2007-06-17 11:19:24 EDT
Created attachment 157223 [details]
spec file with some cleanup

0.5.2-8 with %%prep stage some cleanup.

Well, now mrxvt is okay.
However, as this is NEEDSPONSOR ticket, there is another step
before I can approve this.
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work.
But before I accept this package, someone (I am a candidate)
must sponsor you.

Once you are sponsored, you have the right to review other
submitters' review requests and approve the packages formally.
For this reason, the person who want to be sponsored (like you)
are required to "show that you have an understanding
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other
person's review request, please write the bug number on this bug report
so that I can check your comments or review request.

Fedora Extras package review requests which are waiting for someone to
review can be checked on:
https://bugzilla.redhat.com/bugzilla/buglist.cgi?cmdtype=runnamed&namedcmd=mtasaka-review-noone

NOTE: FE-NEW blockers are now not complete.

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 32 Adam M. Dutko 2007-06-17 23:43:08 EDT
Mamoru,

    I've done a review of bug #207896.  Please review my work when you have a
chance.  I have a few questions about the way their Makefile is structured, how
they left out the %configure section and whether or not they need to ask the
maintainer to separate the libs out from the pkg.  The configure section makes
sense and the way they use build does too, but I'm not sure if there is much
precedent for such actions.  I made a lot of comments, so please let me know
what you think.  I'm also incorporating the changes you made and posting a new
build at the usual place (it should be up in a few):

http://littlehat.homelinux.org:8000/FEDORA/mrxvt/current/0.5.2/

I will also be submitting another package for review in the near future.  I will
let you know when I'm done.  Thank you for all your help.

-Adam
Comment 33 Mamoru TASAKA 2007-06-18 10:02:27 EDT
One comment:

* Umm.. actually the "review style" is difficult issue.

  Many reviewers use some check list styles of their own, and
  it may have the benefit that it may show what the reviewers
  actually checked. Some other reviewers complain if there is
  no check list written on the review.
  However, there are also opinitions that the long verbose list
  is the same as just a copy/paste and makes it difficult to
  watch.

  So while the check list can exist, especially by the time
  you get used to reviewing, you should put the summary
  *at the end* to show briefly what the submitter should fix.
  But the most good way is to try anyway!!

  Again, reviewing style is somewhat difficult issue and actually
  there was a discussion about reviewing style
  (Some said that there should be check lists, some said that
   it is not needed)

Well,
------------------------------------------------------------
  This package (mrxvt) is APPROVED by me
------------------------------------------------------------

Please follow the procedure written on
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account"
When you requested someone to sponsor you (in the procedure
above), please make a note on this bug that you did so.

If you want to push this package also on F-7, you
also have to check:
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
after the URL above.

!! Well, recenctly Fedora package system changed a lot !!
   If you have some questions, please let me know.
Comment 34 Adam M. Dutko 2007-06-18 11:29:52 EDT
Mamoru,

      Awesome!  I made the request for sponsorship for cvsextras and fedorabugs
in my account information.  I will submit my certificate, join the mailing list
and finish the rest of the procedure this evening.

I will also look into bodhi.  :-)

I will also send you some clarification questions once I start my next package.

-Adam 
Comment 35 Adam M. Dutko 2007-06-19 21:53:31 EDT
Built in koji ... http://koji.fedoraproject.org/koji/taskinfo?taskID=42967

Will push through Bodhi procedure.  Thank you again for your help.  Now on to
more pkg reviews...
Comment 36 Adam M. Dutko 2007-06-19 21:53:55 EDT
New Package CVS Request
=======================
Package Name: mrxvt
Short Description: multi-tabbed terminal emulator
Owners: gnome@dux-linux.org
Branches: FC6 F7
InitialCC: mtasaka@ioa.s.u-tokyo.ac.jp
Comment 37 Kevin Fenzi 2007-06-20 01:01:07 EDT
cvs done.
Comment 38 Mamoru TASAKA 2007-06-23 11:14:27 EDT
Would you try to rebuild mrxvt on all branches?
Comment 39 Adam M. Dutko 2007-06-29 10:42:07 EDT
>Would you try to rebuild mrxvt on all branches?

Which branches? I assume FC7-updates-candidate, FC7-updates and etc?

CVS import will be done by Monday evening.

Comment 40 Mamoru TASAKA 2007-06-29 10:55:29 EDT
For FC-6, FC7 and devel, please.
Comment 41 Adam M. Dutko 2007-07-02 22:02:15 EDT
(In reply to comment #40)
> For FC-6, FC7 and devel, please.

dist-fc6-updates-candidate: http://koji.fedoraproject.org/koji/taskinfo?taskID=54654

dist-fc7-updates-candidate: http://koji.fedoraproject.org/koji/taskinfo?taskID=54673

dist-f8-candidate:
*wouldn't let me build for some reason

dist-f8:
http://koji.fedoraproject.org/koji/taskinfo?taskID=54684

Everything is imported into CVS (devel, F-7, FC-6).
Comment 42 Mamoru TASAKA 2007-07-02 22:20:00 EDT
I am now confused. Would you tell me what you actually did?

* http://koji.fedoraproject.org/koji/packageinfo?packageID=4551
  shows that there are no builds against mrxvt done.
* On FC-6, build must be done by plague buildsys, not by koji buildsys.
Comment 43 Mamoru TASAKA 2007-07-02 22:23:49 EDT
On koji buildsys, when rebuild is done properly the entries
like below should appear.
http://koji.fedoraproject.org/koji/packageinfo?packageID=2157

On plague buildsys the entries like below should appear.
http://buildsys.fedoraproject.org/build-status/job.psp?uid=34629
Comment 44 Adam M. Dutko 2007-07-02 22:54:15 EDT
(In reply to comment #43)
> On koji buildsys, when rebuild is done properly the entries
> like below should appear.
> http://koji.fedoraproject.org/koji/packageinfo?packageID=2157
> 
> On plague buildsys the entries like below should appear.
> http://buildsys.fedoraproject.org/build-status/job.psp?uid=34629

Ahh...  I was doing 

koji build --scratch 'target-release' mrxvt-*-.src.rpm 

and NOT building from CVS like

koji build --scratch 'target-release'
http://cvs.fedoraproject.org/viewcvs/rpms/mrxvt/

I'm getting the following... 

[a@buildbox SOURCES]$ koji build --scratch dist-fc7-updates-candidate
http://cvs.fedoraproject.org/viewcvs/rpms/mrxvt/
Uploading srpm: http://cvs.fedoraproject.org/viewcvs/rpms/mrxvt/
<type 'exceptions.IOError'>: [Errno 2] No such file or directory:
'http://cvs.fedoraproject.org/viewcvs/rpms/mrxvt/'

When I got that IO error before I resolved to copying the SRPM locally and doing... 

koji build --scratch 'target-release' /path/to/mrxvt.srpm

Any suggestions?  Thanks.
Comment 45 Mamoru TASAKA 2007-07-02 23:03:40 EDT
Well, I don't know how to use "koji" command directly and to get
the rebuild actually registered to koji buildsys.

Would you just try the following?
$ cvs co mrxvt
$ cd mrxvt/devel
$ make tag build
Comment 46 Adam M. Dutko 2007-07-03 07:58:17 EDT
Ok.  Got it.  I submitted the devel build, but my certificate is messed up.  I
will fix that when I get home from work this evening and resubmit the builds. 
Thank you for clarification on how to formally build in koji versus web
submission like I was doing earlier...
Comment 47 Adam M. Dutko 2007-07-03 08:49:36 EDT
All of the builds were submitted before I left for work.  They appear to have
built properly, but again, I don't think they're signed properly.  I will
regenerate my upload/server certs this evening and resubmit them to koji through
the CVS build mechanism.
Comment 48 Mamoru TASAKA 2007-07-03 11:44:33 EDT
* Well, for F-7 and F-devel, build (on koji) seems to have done
  properly 
  (by http://koji.fedoraproject.org/koji/packageinfo?packageID=4551 ).
  - Now for F-7, you have to request to push mrxvt to Fedora 7 updates.
    (please check: 
     http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT )

* For FC-6 (on plague buildsys), I still cannot see your rebuild.
Comment 49 Adam M. Dutko 2007-07-03 13:09:46 EDT
(In reply to comment #48)
> * Well, for F-7 and F-devel, build (on koji) seems to have done
>   properly 
>   (by http://koji.fedoraproject.org/koji/packageinfo?packageID=4551 ).
>   - Now for F-7, you have to request to push mrxvt to Fedora 7 updates.
>     (please check: 

Update created in bodhi:

Release:  	Fedora 7
Status: 	pending
Type: 	enhancement
...
Requested:  	push
...
Submitted: 	2007-07-03 10:03:17
...


>   http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT )
> 
> * For FC-6 (on plague buildsys), I still cannot see your rebuild.
> 

Will fix tonight.

Comment 50 Adam M. Dutko 2007-07-03 18:20:28 EDT
> > * For FC-6 (on plague buildsys), I still cannot see your rebuild.
> > 
> 
> Will fix tonight.

http://buildsys.fedoraproject.org/build-status/job.psp?email=gnome@dux-linux.org&uid=34736

It is building....
Comment 51 Adam M. Dutko 2007-07-04 10:21:03 EDT
There is another version of mrxvt (0.4.2) I would like to get included.  Do I
register another bug? create the spec, then have another package maintainer
review it before I can proceed?  
Comment 52 Mamoru TASAKA 2007-07-04 10:26:35 EDT
(In reply to comment #50)
> It is building....
Okay. This FC-6 build is okay.

(In reply to comment #51)
> There is another version of mrxvt (0.4.2) I would like to get included. 
First of all, why do you want it?
Comment 53 Adam M. Dutko 2007-07-04 10:55:20 EDT
(In reply to comment #52)
> (In reply to comment #50)
> > It is building....
> Okay. This FC-6 build is okay.

   Great.  I'm just waiting for the push to testing to go through in Bodhi; it's
still pending.

> (In reply to comment #51)
> > There is another version of mrxvt (0.4.2) I would like to get included. 
> First of all, why do you want it?

It's another branch.  0.5.2 is the latest while 0.4.2 is older (less and/or
different bugs I presume).
Comment 54 Mamoru TASAKA 2007-07-05 06:46:08 EDT
I don't think that we should support 0.4.2
- It is old by more than one and the half year
- And 9 months is passed since 0.5.2 is released. IMO we
  should regard 0.5.2 as "stable".
Comment 55 Adam M. Dutko 2007-07-05 10:36:11 EDT
(In reply to comment #54)
> I don't think that we should support 0.4.2
> - It is old by more than one and the half year
> - And 9 months is passed since 0.5.2 is released. IMO we
>   should regard 0.5.2 as "stable".

OK.  I will maintain an "unsponsored" version of 0.4.2 on my personal site,
which I will not push for inclusion into Fedora proper.  

UPDATE: I'm still waiting on bodhi approval for a testing push.   
Comment 56 Mamoru TASAKA 2007-07-05 10:43:25 EDT
(In reply to comment #55)
> UPDATE: I'm still waiting on bodhi approval for a testing push.   
This may take one or two days. This will be done by rel-eng team
manually.

Well, now F-devel/7, FC-6 builds are all okay so you can close this
as "CLOSED NEXTRELEASE". Even after closing this, please let me know if
you have some questions.

Comment 57 Adam M. Dutko 2007-07-05 10:56:22 EDT
Is it acceptable to e-mail you directly when I have questions, instead of
increasing the length of this bug?
Comment 58 Mamoru TASAKA 2007-07-05 11:03:26 EDT
Okay. And now you can also post your questions to
fedora-devel/maintainers mailing list.
Comment 59 Fedora Update System 2007-07-05 15:11:17 EDT
mrxvt-0.5.2-9.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
Comment 60 Fedora Update System 2007-07-11 11:20:21 EDT
mrxvt-0.5.2-9.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.