This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 225791 - Merge Review: gettext
Merge Review: gettext
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On: 223689
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:42 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-16 09:05:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+


Attachments (Terms of Use)
rpmlint log for gettext 0.16.1-5 on FC-devel i386 (15.38 KB, text/plain)
2007-03-12 02:25 EDT, Mamoru TASAKA
no flags Details
mock build log of gettext-0.16.1-6 on FC-devel i386 (714.59 KB, text/plain)
2007-03-13 05:54 EDT, Mamoru TASAKA
no flags Details
normal rpmbuild log of 0.16.1-6 on FC-devel i386 (494.89 KB, text/plain)
2007-03-13 09:12 EDT, Mamoru TASAKA
no flags Details

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

http://cvs.fedora.redhat.com/viewcvs/devel/gettext/
Initial Owner: petersen@redhat.com
Comment 1 Mamoru TASAKA 2007-03-12 00:27:51 EDT
Assigning to me.
Comment 2 Mamoru TASAKA 2007-03-12 02:25:06 EDT
Created attachment 149807 [details]
rpmlint log for gettext 0.16.1-5 on FC-devel i386

Well, for 0.16.1-5:
Some cleanup seems to be needed.

A spec file
* Summary:
  - Please don't end with dots.

* Prereq
  - Deprecated. Use Requires(post) and Requires(preun)

* Source2
  - SOURCE files should not have executable permission.

? BuildRoot tag
  - This BuildRoot does not meet current Fedora policy.

* Parallel make
  - Please check if parallel make is possible.

* %{makeinstall} macro
  - Avoid this if possible.

* Timestamps
---------------------------------------------
install -m 755 %SOURCE2 ${RPM_BUILD_ROOT}/%{_bindir}/msghack
---------------------------------------------
  - Change to "install -p -m 755'. This is a script is keeping timestamp
    is recommended.
  - Well, perhaps also for this package
---------------------------------------------
make install INSTALL="%{__install} -p" .....
---------------------------------------------
    works well to keep timestamps on some files.

* gettext mo files
---------------------------------------------
rm -f $RPM_BUILD_DIR/%{name}-%{version}/trans.list
pushd %{buildroot}/%{_datadir}/locale
for foo in `find . -maxdepth 1 -mindepth 1 -type d` ; do
  lang=`echo $foo | cut -c 3-`
  echo "%lang($lang) %{_datadir}/locale/$foo/*/*" >> \
    $RPM_BUILD_DIR/%{name}-%{version}/trans.list
done
popd
---------------------------------------------
  Cannot this be treated by %find_lang macro?

* One line command in %postun etc...
  - Please use "-p" option.

B. File check
  - Directory ownership issue
----------------------------------------------
[tasaka1@localhost gettext]$ ( for f in `rpm -ql gettext gettext-devel | sort`
; do if [ -d $f ] ; then for g in `rpm -qf $f` ; do echo -e "$f\t$g" ; done ;
fi ; done ) | sed -e '/^[^ \t][^ \t]*\tgettext.*/d' 
/usr/share/locale/en@boldquot	filesystem-2.4.2-1.fc7
/usr/share/locale/en@boldquot/LC_MESSAGES	filesystem-2.4.2-1.fc7
/usr/share/locale/en@quot	filesystem-2.4.2-1.fc7
/usr/share/locale/en@quot/LC_MESSAGES	filesystem-2.4.2-1.fc7
----------------------------------------------
   - Static archive
     * Would you explain why static archive is needed, or just split
       these to -static subpackage?

C. rpmlint issue
   ------ This is attached --------
   NOTES:
   - Please use %% in %changelog for macros.
   - Would you explain if the status of
---------------------------------------------
     unversioned-explicit-provides devel(libintl)
---------------------------------------------
     is proper (and why this is required)?
   - Would you explain the status of /usr/lib/preloadable_libintl.so?
     * Currently this has 0644 permission
     * This is not stripped, debuginfo rpm does not contain the debug
       information for this file.
   - There are many 'spurious-executable-perm' complaint.
   - And somes are 'wrong-script-interpreter'
   - Check if some zero-length files are needed.

D. NOTES:
   - /usr/share/aclocal is not owned by this package and
     -devel package has some files under this directory.

     Generally this status is not allowed, however, it is still under
     discussion about
     * should all packages which have some files under /usr/share/aclocal
       require "automake" to satisfy the directory ownership requirement?
     * or should all packages own this directory?
     * or should filesystem  own this directory? 
     So for now I leave this at it is.
Comment 3 Jens Petersen 2007-03-12 04:33:28 EDT
> * Summary:
>   - Please don't end with dots.
> 
> * Prereq
>   - Deprecated. Use Requires(post) and Requires(preun)

fixing

> * Source2
>   - SOURCE files should not have executable permission.

Not sure how to fix this: the python script is executable in cvs.

> ? BuildRoot tag
>   - This BuildRoot does not meet current Fedora policy.

Fixing.

> * Parallel make
>   - Please check if parallel make is possible.

I will try a build with parallel make.

> * %{makeinstall} macro
>   - Avoid this if possible.

replacing

> * Timestamps
> ---------------------------------------------
> install -m 755 %SOURCE2 ${RPM_BUILD_ROOT}/%{_bindir}/msghack
> ---------------------------------------------
>   - Change to "install -p -m 755'. This is a script is keeping timestamp
>     is recommended.
>   - Well, perhaps also for this package
> ---------------------------------------------
> make install INSTALL="%{__install} -p" .....
> ---------------------------------------------
>     works well to keep timestamps on some files.

fixing and adding

> * gettext mo files
>   Cannot this be treated by %find_lang macro?

Will look into this.

> * One line command in %postun etc...
>   - Please use "-p" option.

fixing

> B. File check
>   - Directory ownership issue
> /usr/share/locale/en@boldquot	filesystem-2.4.2-1.fc7
> /usr/share/locale/en@boldquot/LC_MESSAGES	filesystem-2.4.2-1.fc7
> /usr/share/locale/en@quot	filesystem-2.4.2-1.fc7
> /usr/share/locale/en@quot/LC_MESSAGES	filesystem-2.4.2-1.fc7

removing dirs

>    - Static archive
>      * Would you explain why static archive is needed, or just split
>        these to -static subpackage?

I added --disable-static for now.

> C. rpmlint issue
>    - Please use %% in %changelog for macros.

fixing

>    - Would you explain if the status of
>      unversioned-explicit-provides devel(libintl)
>      is proper (and why this is required)?

No idea.  Probably some historical artefact.  Perhaps it can just be removed?

>    - Would you explain the status of /usr/lib/preloadable_libintl.so?
>      * Currently this has 0644 permission

Let me look into it.

>    - There are many 'spurious-executable-perm' complaint.
>    - And somes are 'wrong-script-interpreter'
>    - Check if some zero-length files are needed.

To be honest I am tempted just to remove all the examples completely from
gettext-devel or at least move them to another subpackage.

> D. NOTES:
>    - /usr/share/aclocal is not owned by this package and
>      -devel package has some files under this directory.
> 
>      Generally this status is not allowed, however, it is still under
>      discussion about

Ok.


Let me update here again when the changes are in cvs and the buildsystem.
Comment 4 Mamoru TASAKA 2007-03-12 07:49:41 EDT
(In reply to comment #3)

> > * Source2
> >   - SOURCE files should not have executable permission.
> 
> Not sure how to fix this: the python script is executable in cvs.
Well, it seems that to fix this you have to once remove
this script from cvs and readd??
If so, leave this as it is for now.

> >    - Would you explain if the status of
> >      unversioned-explicit-provides devel(libintl)
> >      is proper (and why this is required)?
> 
> No idea.  Probably some historical artefact.  Perhaps it can just be removed?
Well, on my system 'devel(libintl)' is not required by
any package.


> >    - There are many 'spurious-executable-perm' complaint.
> >    - And somes are 'wrong-script-interpreter'
> >    - Check if some zero-length files are needed.
> 
> To be honest I am tempted just to remove all the examples completely from
> gettext-devel or at least move them to another subpackage.
Well, these files doesn't seem to be needed...

Other points:
* It may be better that "./ChangeLog" is added to %doc.
Now I am checking all sources...
Comment 5 Mamoru TASAKA 2007-03-12 08:25:35 EDT
From looking at sources:

* There are some test files under
-------------------------------------- 
  ./autoconf-lib-link/tests/
  ./gettext-runtime/tests/
  ./gettext-tools/tests/
--------------------------------------
  Can any tests be done using the files under the directories
  above at %check stage?
Comment 6 Jens Petersen 2007-03-13 03:37:37 EDT
(In reply to comment #3)
> > * gettext mo files
> >   Cannot this be treated by %find_lang macro?
> 
> Will look into this.

Adding find_lang macros.

> >    - Would you explain the status of /usr/lib/preloadable_libintl.so?
> >      * Currently this has 0644 permission
> 
> Let me look into it.

I sent a mail upstream to ask about it.  Currently I set it 755 in the spec file.
I note however that LD_PRELOAD=/usr/lib/preloadable_libintl.so seems
to work also when the file is 644.

(In reply to comment #4)
> Well, it seems that to fix this you have to once remove
> this script from cvs and readd??
> If so, leave this as it is for now.

Ok, thanks.

> Well, on my system 'devel(libintl)' is not required by
> any package.

Ok, I removed it.

> > >    - There are many 'spurious-executable-perm' complaint.
> > >    - And somes are 'wrong-script-interpreter'
> > >    - Check if some zero-length files are needed.
> > 
> > To be honest I am tempted just to remove all the examples completely from
> > gettext-devel or at least move them to another subpackage.
> Well, these files doesn't seem to be needed...

I'm removing examples.

> Other points:
> * It may be better that "./ChangeLog" is added to %doc.
> Now I am checking all sources...

I added it to the -devel package for now, not sure how useful it is though.

(In reply to comment #5)
> * There are some test files under
:
>   Can any tests be done using the files under the directories
>   above at %check stage?

I am going to add "make check" for now, and a patch for one test that currently
fails (reported upstream). :-/
I am not sure it can always be turned on, but it is good to have it documented
in the spec file anyway.  Also the checks take quite a time to run.
I haven't used %check before but it does seem a little intrusive,
for example "make install-short" also runs it for me.


I committed the changes to cvs: they should be in gettext-0.16.1-6.fc7.
Comment 7 Mamoru TASAKA 2007-03-13 05:54:56 EDT
Created attachment 149911 [details]
mock build log of gettext-0.16.1-6 on FC-devel i386

mockbuild of ettext-0.16.1-6 on FC-devel i386 failed
on FC-devel i386.

log says:
-----------------------------------------------
make[3]: Entering directory
`/builddir/build/BUILD/gettext-0.16.1/gettext-tools/m4'
make[3]: Nothing to be done for `install-exec-am'.
test -z "/var/tmp/gettext-0.16.1-6.fc7-Z20472/usr/share/aclocal" || /bin/mkdir
-p
"/var/tmp/gettext-0.16.1-6.fc7-Z20472/var/tmp/gettext-0.16.1-6.fc7-Z20472/usr/share/aclocal"

 /usr/bin/install -p -m 644 '../../autoconf-lib-link/m4/lib-ld.m4'
'/var/tmp/gettext-0.16.1-6.fc7-Z20472/var/tmp/gettext-0.16.1-6.fc7-Z20472/usr/share/aclocal/lib-ld.m4'

-----------------------------------------------
Comment 8 Mamoru TASAKA 2007-03-13 05:58:12 EDT
Oh, spec file seems updated again.
I will check it later.
Comment 9 Mamoru TASAKA 2007-03-13 09:12:59 EDT
Created attachment 149922 [details]
normal rpmbuild log of 0.16.1-6 on FC-devel i386

* About rpmbuild as non-root users
  - Well, as the attached log shows, currently
    normal rpmbuild fails when it is done by non-root users,
    more precisely, by users who do not have /sbin in
    their path.
    After explicitly adding /sbin to PATH, rpmbuild
    succeeds.

    Perhaps the related points are in 
    ./gettext-runtime/libasprintf/Makefile.in (other files
    may be also related)
-------------------------------------------------
   868		@$(POST_INSTALL)
   869		@if (install-info --version && \
   870		     install-info --version 2>&1 | sed 1q | grep -i -v debian)
>/dev/null 2>&1; then \
   871		  list='$(INFO_DEPS)'; \
   872		  for file in $$list; do \
   873		    relfile=`echo "$$file" | sed 's|^.*/||'`; \
   874		    echo " install-info --info-dir='$(DESTDIR)$(infodir)'
'$(DESTDIR)$(infodir)/$$relfile'";\
   875		    install-info --info-dir="$(DESTDIR)$(infodir)"
"$(DESTDIR)$(infodir)/$$relfile" || :;\
   876		  done; \
   877		else : ; fi
-------------------------------------------------
    Well, when "install-info" is in their path, install-info
    is executed and %{buildroot}/usr/share/info/dir is created, then
-------------------------------------------------
    87	
    88	rm ${RPM_BUILD_ROOT}%{_infodir}/dir
    89	
-------------------------------------------------
    in spec file succeeds. However when install-info
    is not in their path, dir file is not created and
    this fails.

    NOTE:
    - Currently mockbuild does:
--------------------------------------------------------
DEBUG: Executing /usr/sbin/mock-helper chroot
/var/lib/mock/fedora-development-i386-core/root /sbin/runuser - root -c "cd
/;/sbin/runuser -c 'rpmbuild --rebuild	--target i386 --nodeps
/builddir/build/SRPMS/gettext-0.16.1-6.fc7.src.rpm' mockbuild"
--------------------------------------------------------
       So when mockbuild is done, the path is set as same
       as which root sets.
    So:
    * The most simple way is just to "rm -f"

* rpmlint
  - rpmlint for 0.16.1-6 is
----------------------------------------------
W: gettext unstripped-binary-or-object /usr/lib/preloadable_libintl.so
E: gettext no-ldconfig-symlink /usr/lib/preloadable_libintl.so
W: gettext-devel unused-direct-shlib-dependency /usr/lib/libasprintf.so.0.0.0
/lib/libm.so.6
E: gettext-devel zero-length
/usr/share/doc/gettext-devel-0.16.1/javadoc2/package-list
-----------------------------------------------
   - For the first one (unstripped-binary-or-object), rpmbuild
     doesn't strip libraries when libraries does not have
     executable permissions when %install stage is done
     (adding %attr does not make sense for stripping)

     To strip this, the permission of preloadable_libintl.so
     has to be changed explicitly to 0755.
   - I don't know well about preloadable_libintl.so, however
     can the second rpmlint error (about ldconfig symlink)
     ignored?
   - What is the file "package-list"?
Comment 10 Jens Petersen 2007-03-14 23:27:45 EDT
(In reply to comment #9)
>     * The most simple way is just to "rm -f"

ok

> W: gettext unstripped-binary-or-object /usr/lib/preloadable_libintl.so
|
>    - For the first one (unstripped-binary-or-object), rpmbuild
>      doesn't strip libraries when libraries does not have
>      executable permissions when %install stage is done
>      (adding %attr does not make sense for stripping)
> 
>      To strip this, the permission of preloadable_libintl.so
>      has to be changed explicitly to 0755.

fixing

> E: gettext no-ldconfig-symlink /usr/lib/preloadable_libintl.so
|
>    - I don't know well about preloadable_libintl.so, however
>      can the second rpmlint error (about ldconfig symlink)
>      ignored?

The explanation reads:

'''The package should not only include the shared library itself, but
also the symbolic link which ldconfig would produce. (This is
necessary, so that the link gets removed by rpm automatically when
the package gets removed, even if for some reason ldconfig would not be
run at package postinstall phase.)'''

There is no symlink for preloadable_libintl.so since it is not versioned,
so I think it can be ignored.

> E: gettext-devel zero-length
> /usr/share/doc/gettext-devel-0.16.1/javadoc2/package-list
|
>    - What is the file "package-list"?

I don't know - I can try to find out.

> W: gettext-devel unused-direct-shlib-dependency /usr/lib/libasprintf.so.0.0.0
> /lib/libm.so.6

This seems to be due to the libtool in gettext-runtime/libasprintf/.
Not sure if it is worth fixing.
Comment 11 Mamoru TASAKA 2007-03-15 05:05:13 EDT
(In reply to comment #10)

> > W: gettext-devel unused-direct-shlib-dependency /usr/lib/libasprintf.so.0.0.0
> > /lib/libm.so.6
> 
> This seems to be due to the libtool in gettext-runtime/libasprintf/.
> Not sure if it is worth fixing.

Fix this if you want. Currently this warning is ignored.
This means that linkage against libm.so.6 is not needed.
Comment 12 Jens Petersen 2007-03-15 20:48:04 EDT
I fixed infodir/dir removal and the perms on the preload module in
gettext-0.16.1-7.fc7.
Comment 13 Mamoru TASAKA 2007-03-16 09:05:37 EDT
Well, 
* please check if 
  "/usr/share/doc/gettext-devel-0.16.1/javadoc2/package-list" is needed.

Other things are okay. Closing.
 
Comment 14 Jens Petersen 2007-03-18 20:27:06 EDT
Thanks for the review.

(In reply to comment #13)
> * please check if 
>   "/usr/share/doc/gettext-devel-0.16.1/javadoc2/package-list" is needed.

I asked some Java people about it and one possibility is that it may be
a gjdoc bug.  I was recommended to try java-1.5.0-gcj and sinjdoc.
So I will do that.
Comment 15 Jens Petersen 2007-03-18 22:56:08 EDT
Hrrm, actually I just noticed package-list is part of the upstream tarball,
so this may be an upstream issue actually.

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