Bug 225759 - Merge Review: fontconfig
Merge Review: fontconfig
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:38 EST by Nobody's working on this, feel free to take it
Modified: 2013-03-03 23:21 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-03-03 23:21:54 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)
Some Changes to Original SPEC (3.05 KB, patch)
2007-03-01 07:01 EST, Parag AN(पराग)
no flags Details | Diff
Review of fontconfig-2.8.0-1 (9.40 KB, text/plain)
2010-07-08 23:40 EDT, Garrett Holmstrom
no flags Details
spec cleanup (6.96 KB, patch)
2010-10-26 02:32 EDT, Parag AN(पराग)
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:38:34 EST
Fedora Merge Review: fontconfig

http://cvs.fedora.redhat.com/viewcvs/devel/fontconfig/
Initial Owner: besfahbo@redhat.com
Comment 1 Parag AN(पराग) 2007-03-01 06:49:46 EST
Mock build for rawhide is fine.
rpmlint on SRPM reported
W: fontconfig prereq-use freetype >= %{freetype_version}, coreutils
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

W: fontconfig make-check-outside-check-section make check
Make check or other automated regression test should be run in %check, as
they can be disabled with a rpm macro for short circuiting purposes.

W: fontconfig macro-in-%changelog 5x
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

rpmlint on RPMS reported
W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/10-autohint.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/10-no-sub-pixel.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/10-sub-pixel-bgr.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/10-sub-pixel-rgb.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/10-sub-pixel-vbgr.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/10-sub-pixel-vrgb.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/10-unhinted.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/20-fix-globaladvance.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/20-lohit-gujarati.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/20-unhint-small-vera.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/30-amt-aliases.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/30-urw-aliases.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/40-generic.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/49-sansserif.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/50-user.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/51-local.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/60-latin.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/65-fonts-persian.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/65-nonlatin.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.avail/69-unifont.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/70-no-bitmaps.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/70-yes-bitmaps.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/80-delicious.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.avail/90-synthetic.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/20-fix-globaladvance.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/20-lohit-gujarati.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/20-unhint-small-vera.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/25-no-hint-fedora.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/30-aliases-fedora.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/30-amt-aliases.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/30-urw-aliases.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/40-generic-fedora.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/40-generic.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/49-sansserif.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/50-user.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/51-local.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/60-latin.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/64-nonlatin-fedora.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/65-fonts-persian.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/65-nonlatin.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/69-unifont.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag
/etc/fonts/conf.d/75-blacklist-fedora.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/80-delicious.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/conf.d/90-synthetic.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig conffile-without-noreplace-flag /etc/fonts/fonts.conf
A configuration file is stored in your package without the noreplace flag.
A way to resolve this is to put the following in your SPEC file:

%config(noreplace) /etc/your_config_file_here


W: fontconfig non-conffile-in-etc /etc/fonts/fonts.dtd
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

W: fontconfig non-conffile-in-etc /etc/fonts/conf.avail/README
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

W: fontconfig dangerous-command-in-%post rm

Comment 2 Parag AN(पराग) 2007-03-01 07:01:14 EST
Created attachment 149010 [details]
Some Changes to Original SPEC 

You SHOULD
1) Use attached SPEC
2) run rpmlint and check for further errors
3) I saw rpath is used for building this package. Remove rpath.
4) rpmlint reported rm is used in $post section. Can you give reasons for that?

5) Can't /etc/fonts/conf.avail/README be installed under /usr/share/doc
Comment 3 Behdad Esfahbod 2007-03-01 16:17:41 EST
We don't want to mark those config files as noreplace, as they should not be
touched.  That has saved us when switching to fontconfig 2.4 already.
Comment 4 Parag AN(पराग) 2007-03-14 01:43:37 EDT
In case you have %config and %config(noreplace) in your SPEC then You may like
to update the SPEC by removing that.
Check http://fedoraproject.org/wiki/Packaging/Minutes20070313
and http://fedoraproject.org/wiki/PackagingDrafts/UsrConfigs
Comment 5 Parag AN(पराग) 2007-03-30 09:28:41 EDT
any updates?
Comment 6 Behdad Esfahbod 2007-04-11 19:26:55 EDT
1) as I said, I don't want %config(noreplace).
2) what's the point in patching changelog entries?

Please attach a patch addressing this issues.
Comment 7 Garrett Holmstrom 2010-07-08 23:39:42 EDT
I, for one, agree with the above (ancient) commentary:  as long as you justify not using noreplace you should be fine.  All you have to do is briefly note it in the spec file.

There isn't a whole lot to do for this package; most everything is a simple one-line tweak.  If you need someone else to make a patch to fix these issues for some reason I can do so.

A full review for this package is attached.  Below are just the parts that need fixing.  (Hopefully bugzilla won't mangle my spacing.)

Mandatory review guidelines:
NO - rpmlint output
     prereq is deprecated:              Change PreReq to Requires
     make-check-outside-check-section:  Just put make check in a %check section
     conffile-without-noreplace-flag:   Justified in this bug; should also note
                                        in spec

NO - Package's files and directories don't conflict with others'
     Both fontconfig and fontpackages-filesystem own /usr/share/fonts

Packaging guidelines:
NO - Spec file lacks Packager, Vendor, PreReq tags
     Please change PreReq to Requires.
NO - %build honors applicable compiler flags or justifies otherwise
     Building with make %{?_smp_mflags} seems to work for me; if it actually
     doesn't please mention so in the spec file.
NO - %config files marked noreplace or justified
     Justified in this bug; should also note in spec file
Comment 8 Garrett Holmstrom 2010-07-08 23:40:32 EDT
Created attachment 430527 [details]
Review of fontconfig-2.8.0-1
Comment 9 Garrett Holmstrom 2010-07-09 00:07:43 EDT
(In reply to comment #7)
> NO - %build honors applicable compiler flags or justifies otherwise
>      Building with make %{?_smp_mflags} seems to work for me; if it actually
>      doesn't please mention so in the spec file.

Whoops, the description for this (optional) problem got matched with the wrong guideline.  It isn't mandatory, but if it works it probably won't hurt to add it anyway.
Comment 10 Parag AN(पराग) 2010-10-26 02:32:14 EDT
Created attachment 455694 [details]
spec cleanup

This git patch will clean this package according to packaging guidelines and also try to resolve bug 477054 and bug 645080.
Comment 11 Parag AN(पराग) 2010-10-26 08:26:39 EDT
mclasen,
   Can you please check the attached patch and approve to commit this?
Comment 12 Matthias Clasen 2010-10-26 09:43:43 EDT
your patch seems to add %config(noreplace) despite comment #3 and comment #6
Comment 13 Parag AN(पराग) 2010-10-26 10:58:20 EDT
Thank you for your quick reply. My patch is also based on what Nicolas Mailhot suggested in https://bugzilla.redhat.com/show_bug.cgi?id=477054#c6

There are 2 changes in spec related to %config(noreplace)

1)%config(noreplace) %{_fontconfig_confdir}/*.conf 
which is same as written in current spec in master
%config(noreplace) %{_sysconfdir}/fonts/conf.d/*.conf

2) %config(noreplace) %{_fontconfig_masterdir}/fonts.conf
which is NOT the same as compared to current spec in master
%config %{_sysconfdir}/fonts/fonts.conf

Therefore, I will drop the second change only and will commit rest.
Comment 14 Nicolas Mailhot 2010-10-26 13:32:20 EDT
(In reply to comment #13)
> Thank you for your quick reply. My patch is also based on what Nicolas Mailhot
> suggested in https://bugzilla.redhat.com/show_bug.cgi?id=477054#c6

Note that this patch depends on an upstream fontconfig patch that makes fontconfig install its templates in /usr/share. So it works because with that patch fontconfig no longer installs files the user should not modify in /etc. If someone could ping Behdad to get the upstream fontconfig patch merged, then we could simplify the fedora spec file.
Comment 15 Nicolas Mailhot 2010-10-26 13:32:52 EDT
See also
https://bugs.freedesktop.org/show_bug.cgi?id=29341
Comment 16 Parag AN(पराग) 2010-10-27 04:44:17 EDT
ping behdad
Comment 17 Parag AN(पराग) 2012-02-02 01:57:03 EST
Leaving this review as there is no response to fix this package.
Comment 18 Parag AN(पराग) 2012-03-09 02:25:00 EST
Matthias,
   I am not sure if anyone is actively working on Fedora fontconfig package maintenance. We have got new fontconfig project leader(Akira Tagoh) in upstream. Soon a new release of fontconfig will be available. If its ok with you can this package be reassigned to Akira?
Comment 19 Parag AN(पराग) 2012-03-12 04:44:32 EDT
fontconfig 2.9.0 is released now. Can we have it built for Fedora 17?
Comment 20 Akira TAGOH 2012-11-19 04:15:09 EST
I think current package should be better now. can someone start review again?
Comment 21 Parag AN(पराग) 2012-12-11 00:18:46 EST
let me see if I can review this.
Comment 22 Parag AN(पराग) 2012-12-11 02:28:59 EST
Issues:
=======
[!]: %config files are marked noreplace or the reason is justified.
     Note: %config %{_fontconfig_masterdir}/fonts.conf
See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files

[!]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 1034240 bytes in 195 files.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

[!]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in %package
     devel

[!]: I don't think you need to explicitly "Requires: pkgconfig" in spec file

[!]: Patches link to upstream bugs/comments/lists or are otherwise justified.

Rpmlint
-------
Checking: fontconfig-2.10.2-1.fc18.x86_64.rpm
          fontconfig-debuginfo-2.10.2-1.fc18.x86_64.rpm
          fontconfig-devel-2.10.2-1.fc18.x86_64.rpm
          fontconfig-2.10.2-1.fc18.src.rpm
fontconfig.x86_64: W: conffile-without-noreplace-flag /etc/fonts/fonts.conf
fontconfig.x86_64: W: non-conffile-in-etc /etc/fonts/conf.d/README
4 packages and 0 specfiles checked; 0 errors, 2 warnings.
Comment 23 Parag AN(पराग) 2013-02-05 08:12:46 EST
any updates here?
Comment 24 Akira TAGOH 2013-02-07 04:24:33 EST
(In reply to comment #22)
> Issues:
> =======
> [!]: %config files are marked noreplace or the reason is justified.
>      Note: %config %{_fontconfig_masterdir}/fonts.conf
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files

I thought it has already been settled with the above comments... if you want to modify, you should use local.conf.

> 
> [!]: Large documentation must go in a -doc subpackage.
>      Note: Documentation size is 1034240 bytes in 195 files.
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation
> 
> [!]: Fully versioned dependency in subpackages, if present.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in %package
>      devel

will fix the above two.

> [!]: I don't think you need to explicitly "Requires: pkgconfig" in spec file

Can you give me a pointer where it is mentioned? I can see for BR but R.

> [!]: Patches link to upstream bugs/comments/lists or are otherwise justified.

No plans to merge this one. it's a kind of hack and no reasonable point that it keeps working forever nor needs to keep applying forever for the issue. that said, I can't remove it because I have no idea how to figure out (before releasing) if the issue is really gone out without it. so I'm just keep applying.
Comment 25 Akira TAGOH 2013-02-12 05:08:04 EST
applied the above fixes into git. please review again.
Comment 26 Parag AN(पराग) 2013-02-14 04:03:45 EST
Looks good now :)

Thank you for having a look at this review and fixing all the needed issues.

And yes about pkgconfig, I found this reference
https://fedoraproject.org/w/index.php?title=Packaging%3AReviewGuidelines&diff=160629&oldid=146860

and this also,

https://fedoraproject.org/wiki/Archives:PackagingDrafts/PkgconfigAutoRequires


APPROVED.
Comment 27 Parag AN(पराग) 2013-03-03 23:21:54 EST
This is completed so lets close this review.

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