[Bug 225902] New: Merge Review: intltool

[Red Hat Bugzilla]

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

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

Summary: Merge Review: intltool
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail]

Fedora Merge Review: intltool

http://cvs.fedora.redhat.com/viewcvs/devel/intltool/
Initial Owner: [e-mail]

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

--- Comment #1 from Robert Scheck   2009-01-13 16:50:32 EDT ---
intltool.i386: E: tag-not-utf8 %changelog

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

--- Comment #2 from Matthias Clasen   2009-01-13 21:50:10 EDT ---
Feel free to apply pedantic fixes like that.

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

Thomas Spura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]-mainz.
                   |                            |de

--- Comment #3 from Thomas Spura   2009-10-08 06:56:09 EDT ---
* changelog is now in utf8.

* rpmlint ../SRPMS/intltool-0.41.0-1.fc11.src.rpm
../RPMS/noarch/intltool-0.41.0-1.fc11.noarch.rpm 
intltool.src:18: W: unversioned-explicit-obsoletes xml-i18n-tools
intltool.noarch: E: devel-dependency gettext-devel
intltool.noarch: W: self-obsoletion xml-i18n-tools obsoletes xml-i18n-tools =
0.11
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

* rpmlint intltool.spec 
intltool.spec:18: W: unversioned-explicit-obsoletes xml-i18n-tools
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

Obsoletes: xml-i18n-tools < 0.11 solves all 3 issues in this direction.

Stays intltool.noarch: E: devel-dependency gettext-devel

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #4 from Jon Ciesla  2012-04-05 13:41:53 EDT ---
Fresh review:

Good:

- rpmlint checks return:

intltool.spec:18: W: unversioned-explicit-obsoletes xml-i18n-tools
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

intltool.noarch: W: self-obsoletion xml-i18n-tools obsoletes xml-i18n-tools =
0.11
The package obsoletes itself.  This is known to cause errors in various tools
and should thus be avoided, usually by using appropriately versioned Obsoletes
and/or Provides and avoiding unversioned ones

Fix.

intltool.src: W: spelling-error %description -l en_US bonobo -> Bono, bonbon,
Bonn
The value of this tag appears to be misspelled. Please double-check.

intltool.src: W: spelling-error %description -l en_US ui -> ii, u, i
The value of this tag appears to be misspelled. Please double-check.

intltool.src: W: spelling-error %description -l en_US po -> PO, pew, op
The value of this tag appears to be misspelled. Please double-check.

Ignore.

intltool.noarch: E: devel-dependency gettext-devel
Your package has a dependency on a devel package but it's not a devel package
itself.

Does this really need gettext-devel or just gettext?  Should perhaps the
Requires and BuildRequires for gettext and gettext-devel be reversed?  If this
is correct, that's fine.

Several incorrect FSF address errors, not a huge issue, fix bugs upstream if
you like.

- package meets naming guidelines
- package meets packaging guidelines
- license ( GPLv2 with exceptions ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Otherwise it looks pretty good.  Just the dependency questions.  Let me know if
you'd like me to commit anything.

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

--- Comment #5 from Matthias Clasen  2012-04-05 18:08:24 EDT ---
(In reply to comment #4)
> Otherwise it looks pretty good.  Just the dependency questions.  Let me know if
> you'd like me to commit anything.

By all means, if you want to change that to your liking, go ahead and commit.
Thanks !

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |RAWHIDE
               Flag|fedora-review?              |fedora-review+
        Last Closed|                            |2012-04-06 08:22:58

--- Comment #6 from Jon Ciesla  2012-04-06 08:22:58 EDT ---
Done, thanks!

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

Kalev Lember  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #7 from Kalev Lember  2012-04-10 04:41:02 EDT ---
Hi Jon,

Thanks for taking care of this. I took a look at the changes done here because
of new build failures on rawhide. A few comments:
> -BuildRequires: gettext
> +BuildRequires: gettext-devel

Why is it necessary to change this? It used to build fine with BR: gettext as
it only needs the tools from gettext package for building and doesn't use the
headers that are in -devel.
> -Requires: gettext-devel
> +Requires: gettext

Requiring gettext-devel was a deliberate change in
http://pkgs.fedoraproject.org/gitweb/?p=intltool.git;a=commit;h=b5cc8a4b . If
you ever need to find out why some line is the way it is, 'git blame' can help
for history digging.

Packages that used to rely on intltool dragging in gettext-devel are now
failing to build, e.g. gtranslator:
http://koji.fedoraproject.org/koji/buildinfo?buildID=312458

If you think it's important to depend on gettext instead of gettext-devel, can
you send out a heads up to fedora-devel list that packages will now have to
explicitly BR gettext-devel, instead of relying on intltool dragging it in? I
fear the ARM secondary arch people aren't very happy about new FTBFS failures,
so it's best to try to get package maintainers to fix these early.
> -Obsoletes: xml-i18n-tools
> -Provides: xml-i18n-tools = 0.11
> +#Obsoletes: xml-i18n-tools
> +#Provides: xml-i18n-tools = 0.11

Perhaps remove these lines completely, to avoid cluttering the spec file with
commented out lines? Any changes are permanently recorded in git history, so
it's always possible to revert them at a later date.

Thanks again for taking care of the merge review.

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from Jon Ciesla  2012-04-10 08:19:55 EDT ---
(In reply to comment #7)
> Hi Jon,
>
> Thanks for taking care of this. I took a look at the changes done here because
> of new build failures on rawhide. A few comments:
> > -BuildRequires: gettext
> > +BuildRequires: gettext-devel
>
> Why is it necessary to change this? It used to build fine with BR: gettext as
> it only needs the tools from gettext package for building and doesn't use the
> headers that are in -devel.
> > -Requires: gettext-devel
> > +Requires: gettext
>
> Requiring gettext-devel was a deliberate change in
> http://pkgs.fedoraproject.org/gitweb/?p=intltool.git;a=commit;h=b5cc8a4b . If
> you ever need to find out why some line is the way it is, 'git blame' can help
> for history digging.

Because typically, bits that are needed to build other tools against a given
package live in -devel, and bits needed at runtime are in the base package. 
-devel requires base.  I understand that this may not always be the case,
especially with tools used in development, such as intltool, but that this
breaks builds suggests that gettext's file layout might bear re-examination. 
Looking at that commit simply tells me that the BR was changed, not why.  No BZ
or problem is mentioned.
> Packages that used to rely on intltool dragging in gettext-devel are now
> failing to build, e.g. gtranslator:
> http://koji.fedoraproject.org/koji/buildinfo?buildID=312458
>
> If you think it's important to depend on gettext instead of gettext-devel, can
> you send out a heads up to fedora-devel list that packages will now have to
> explicitly BR gettext-devel, instead of relying on intltool dragging it in? I
> fear the ARM secondary arch people aren't very happy about new FTBFS failures,
> so it's best to try to get package maintainers to fix these early.

Agreed.  I'm adding the gettext maintainer for his thoughts on the best
resolution to this.
> > -Obsoletes: xml-i18n-tools
> > -Provides: xml-i18n-tools = 0.11
> > +#Obsoletes: xml-i18n-tools
> > +#Provides: xml-i18n-tools = 0.11
>
> Perhaps remove these lines completely, to avoid cluttering the spec file with
> commented out lines? Any changes are permanently recorded in git history, so
> it's always possible to revert them at a later date.

I do that on my specs, but typically leave them in place for faster reverting
on those belonging to others.
> Thanks again for taking care of the merge review.

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

--- Comment #9 from Kalev Lember  2012-04-16 19:02:46 EDT ---
This change was breaking a lot of builds in rawhide, so I went ahead and fixed
the BuildRequires/Requires in
http://pkgs.fedoraproject.org/gitweb/?p=intltool.git;a=commit;h=b813c08ae

/usr/share/aclocal/intltool.m4 uses gettext macros that are distributed in
gettext-devel (e.g. AM_GNU_GETTEXT_VERSION, AM_GNU_GETTEXT, AM_NLS) and because
of that, intlool needs to depend on gettext-devel.

Some examples of failed builds:
devhelp: http://koji.fedoraproject.org/koji/taskinfo?taskID=3996455
gdm: http://koji.fedoraproject.org/koji/taskinfo?taskID=3996427

[Bug 225902] Merge Review: intltool

[Red Hat Bugzilla]

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

--- Comment #10 from Jon Ciesla  2012-04-16 21:53:00 EDT ---
Ok, thank you.  At least we've documented that it needs to be this way.  Sorry
for the broken builds in the interim.