[Bug 226535] Merge Review: w3m

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: w3m


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226535


pnemade at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |urgent




------- Additional Comments From pnemade at redhat.com  2007-03-27 00:33 EST -------
(In reply to comment #3)
> Created an attachment (id=150834)
 --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=150834&action=view) [edit]
> w3m.spec with some fixes (0.5.1-18)
> 
> Well, for 0.5.1-17.fc7:
> 
> * Source0
>   - Please check:
>     http://fedoraproject.org/wiki/Packaging/SourceURL
> (A) It seems that newest gc is 6.8.
> 
> * CFlags
>     (Please see the attached mock build log)
>   - Fedora specific compilation flags are not passed
>     (for gc.a).
>     For gc.a, it seems okay when ABI_FLAG is set as
>     RPM_OPT_FLAGS
>     And.. actually this changes debuginfo contents.
     Agree.

> (B) NOTE:
>     Usually gc should be seperated from this package (w3m)
>     and other packages for gc should be created.
>     In this case, gc package should provide shared library
>     (not static archive). 
>     However is this too late for F7?
      Don't think so. Yes static library should be removed.
> 
> * Version provides
> ------------------------------------------------
> Provides:  webclient = 0.5.1
> ------------------------------------------------
>   - Any reason to provide version-dependent virtual dependency?
>     I don't see the reason, and other package which provide
>     "webclient" virtual dependency does not specify version.
  
      Ok.
> 
> * Requires
>   - For main package:
> ------------------------------------------------
>     Requires:  perl, openssl
> ------------------------------------------------
>     These should be removed. rpmbuild automatically
>     finds these dependencies.
   yes this should be removed.

> 
> * spec file description
> ------------------------------------------------
> [ -n "$RPM_BUILD_ROOT" -a "$RPM_BUILD_ROOT" != / ] && rm -rf $RPM_BUILD_ROOT
> ------------------------------------------------
>   - The part [ ...... ] is redundant and should be removed.
>     RPM_BUILD_ROOT _MUST_ not be empty or / .

 Ok.

> 
> * Timestamps
> ------------------------------------------------
> install -m 644 %{SOURCE10} $RPM_BUILD_ROOT%{_sysconfdir}/w3m/config
> ------------------------------------------------
>   - Please keep timestamp. i.e. use "install -p".
   Yes.

> 
> * Misc
>   * w3m.lang usage
> ------------------------------------------------
> find $RPM_BUILD_ROOT%{_libexecdir} -type f -print | grep -v w3mimgdisplay | sed
> -e "s,$RPM_BUILD_ROOT,," >> w3m.lang
> ........
> %files
> ........
> %exclude %{_libexecdir}/w3m/w3mimgdisplay
> ------------------------------------------------
>     - At the first description, w3mimgdisplay are already excluded.
>       Anyway, these can be unified. Simply,
> ------------------------------------------------
> %files
> .......
> %{_mandir}/man1/w3mman.1*
> %{_libexecdir}/%{name}/
> %exclude %{_libexecdir}/w3m/w3mimgdisplay
> ------------------------------------------------
>       should be okay.

     Agree.
> 
> * Conditional dependency
>   - Well, I found that when I rebuild w3m locally, inline image handler
>     is enabled for x11 and fb,
>     while for mockbuild only x11 image handler is enabled
> 
>     This is because configure reads:
> ------------------------------------------------
>   5785	    enable_image=x11
>   5786	    case "`uname -s`" in
>   5787	    Linux|linux|LINUX)
>   5788		if test -c /dev/fb0; then
>   5789		  enable_image=x11,fb
>   5790		fi;;
>   5791	    esac
>   5792	  fi
>   5793	  save_ifs="$IFS"; IFS=",";
>   5794	  for img in $enable_image; do
> ------------------------------------------------
>     However, on mockbuild /dev/fb0 is not created, so fb image handler
>     will not be enabled.
>     To fix this, configure option must handle this explicitly.
> 
> * lang
>   - for Japanese documents, these should be treated as %lang(ja).

         yes.
> 
> * Documentation
> (C) doc*/w3m.1 seems unneeded, as they are already included as
>     man files. However, I leave this as how you judge.
> 
     Ok. will remove from doc.

> (D) Question:
>     - Should all the documents in doc-jp/ files should be
>       converted from EUC-JP to UTF-8?
>       I think so, however, if you want to do so, please keep
>       timestamps on these files even after encodings are converted,
>       as these documents are 3-6 years old.

        Agree.

> 
> -------------------------------------------------
> My attached spec file should fix all the issues above
> expect (A)-(D).
> Please check my spec file and comment on (A)-(D)
> 
> * NOTE
>   I have not checked yet
>   * what documentation should be added to this rpm
>   * whether license is correct and has no problem
>   * some other issues may exist



-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.