[Bug 458338] Review Request: DivFix++ - A program to repair broken AVI file streams by rebuilding index part of file

[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.


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





--- Comment #4 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2008-10-08 14:16:49 EDT ---
Well,

* SourceURL
  - seems 404. The correct one is perhaps:
    http://downloads.sourceforge.net/divfixpp/%{name}_v%{version}-src.tar.bz2

* Compilation flags
(In reply to comment #3)
> > * Compilation flags
> >   - Fedora specific compilation flags are not correctly honored.
> >     You can check what compilation flags are used by
> >     $ rpm --eval %optflags
> >     https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
> >   - Also compilation flag "-Os" supersedes "-O2" flag in %optflags so
> >     please remove this.
> Excuse me, but I do ot completely understand you. I'm do ot provide manually
> any compilation flags, so, default must be used.

  - So what I am saying is that you must modify cflags what is used
    on this tarball by default so that Fedora cflags are used correctly
    (i.e. you must use %optflags manually).
    Also this tarball uses "-Os" option, which should be removed.

    One of the solution is:
--------------------------------------------------------
%prep
%setup -q -n %{name}_v%{version}
sed -i.flags -e 's|-Os||' makefile
......
......
%build
make %{?_smp_mflags} WXCONFIG=wx-config CPP="g++ %optflags"
-------------------------------------------------------

* Desktop
  - Please set Category. Currently no Category is found
    (by the way --remove-category=Application can be removed
     if you don't use Application as Category from the
     beginning)

* Macros
> >   - %distname is not defined.
> %distname replaced by %{distribution}

  - My system does not define %distribution macro. Koji seems to
    define it, however its value (string) is "Unknown" so
    this is still wrong.
    Just use "--vendor=fedora".

  ! By the way if you put macros in braces please them consistently
    for cosmetic issue (i.e. use %{__install})

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.