[Bug 669311] Review Request: mupdf - A lightweight PDF viewer and toolkit written in portable C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=669311

--- Comment #2 from Mohamed El Morabity <pikachu.2014@xxxxxxxxx> 2011-01-13 07:47:08 EST ---

* You can remove libX11-devel from the BuildRequires, since it is already
  required by libXext-devel

* The compilation logs are not very verbose. Especially compilation flags
cannot
  be checked. Fortunately the Makefile provides a "verbose" variable to view
all
  compilation commands. Please enable it:
     %build
     make %{?_smp_mflags} verbose=1

* The fix on the previous point leads to see that standard Fedora compilation
  flags ("%{optflags}") are not used att all. They *must* be called by the
  compiler:
     http://fedoraproject.org/wiki/PackagingGuidelines#Compiler_flags
  I suggest you the following fix:
  - add the following line in %prep:
       %prep
       [...]
       sed -i "s/CFLAGS := /CFLAGS ?=\nCFLAGS += /" Makerules
    It will allow passing custom CFLAGS values. Since it is a small fix, I
    thought using sed would be more appropriate than a patch.
  - add the following on in %build:
       %build
       export CFLAGS="%{optflags}"
       make %{?_smp_mflags} verbose=1

* You have removed the static library libmupdf.a, and it's probably a good
thing
  according to the guidelines. Unfortunately there is no corresponding shared
  library (.so file), and so your -devel package is completely useless. I
  suggest you to report also this issue to upstream.

  I attach to this review a patch to build a dynamic version of libmupdf.a
  (libmupdf.so), and to link the executables to this one, instead of embedding
  the static lib. Anyway it is maybe not usable as is: it is not versionned and
  that's a things to be discussed with upstream.

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]