Bug 247930 - Review Request: linkchecker - checks HTML documents for broken links
Review Request: linkchecker - checks HTML documents for broken links
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-11 22:43 EDT by W. Michael Petullo
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-12 21:07:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+


Attachments (Terms of Use)
mock build log of linkchecker 4.7-1 on rawhide i386 (88.35 KB, text/plain)
2007-07-12 02:18 EDT, Mamoru TASAKA
no flags Details
mock build log of linkchecker 4.7-2 on rawhide i386 (125.61 KB, text/plain)
2007-07-13 12:12 EDT, Mamoru TASAKA
no flags Details
mock build log of linkchecker 4.7-3 on rawhide i386 (88.60 KB, text/plain)
2007-07-15 10:53 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description W. Michael Petullo 2007-07-11 22:43:49 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-1.fc7.src.rpm
Description: This package was previously approved (Bug #168611), was orphaned and is now maintained by Mike Petullo again.  Because it was orphaned for more than three months, it needs to be reviewed again.
Comment 1 Mamoru TASAKA 2007-07-12 02:18:10 EDT
Created attachment 159034 [details]
mock build log of linkchecker 4.7-1 on rawhide i386

Some comments.

* SourceURL
  - For sourceforge source, please check the section "Sourceforge.net" of:
    http://fedoraproject.org/wiki/Packaging/SourceURL

* Mock build fails (build log attached)
  - Note: Now .pyo, .pyc must not be excluded.

* File entry
  - The two lines
----------------------------------------------
%dir %{_datadir}/linkchecker/
%{_datadir}/linkchecker/*
----------------------------------------------
    can be replaced with one line:
----------------------------------------------
%{_datadir}/linkchecker/
----------------------------------------------
  - And similarly, IMO the part # Collate list of python files
    can be removed and can be simplified.

* ExcludeArch
----------------------------------------------
# Do not have one of these to test on:
ExcludeArch: x86_64
----------------------------------------------
  - What do you mean by this? (see "sitearch" comment
    below)

* sitearch
  - .so module is arch-dependent and should be
    installed under site_arch directory (on x86_64 and
    ppc64, site_lib and site_arch directory differs)

* Requires
  - Check Requires for this package
    * For example, this package requires python-docutils
    * python-imaging is available on Fedora
    * And maybe this package requires more packages.
Comment 2 W. Michael Petullo 2007-07-12 16:23:59 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-2.fc7.src.rpm

Implemented Mamoru's recommendations.
Comment 3 Mamoru TASAKA 2007-07-13 12:12:18 EDT
Created attachment 159201 [details]
mock build log of linkchecker 4.7-2 on rawhide i386

For 4.7-2:

* File entry
----------------------------------------
%{python_sitearch}/*
%{python_sitearch}/linkcheck
----------------------------------------
  - This way of listing causes many duplicate file entry
    on i386 (check the mock build log attached).

* again site_lib vs site_arch
  - While arch-dependent files (usually .so file) must be installed
    under site_arch directory, arch-independent .py{,o,c} files
    must be installed under site_lib directory (so you have to
    use both site_lib and site_arch)

* Directory ownership
  - /usr/share/man/{de,fr}/man1 are already owned by man

* rpmlint
  - Some files' permissions are not correct.
-----------------------------------------
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/doc/examples/check_urls.sh
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/cgi-bin/lc.cgi
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/cgi-bin/lc.fcgi
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/doc/rest2htmlnav
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/doc/examples/check_blacklist.sh
W: linkchecker spurious-executable-perm
/usr/share/doc/linkchecker-4.7/doc/examples/check_for_x_errors.sh
-----------------------------------------

* Documents
  * gettext files
  - /usr/share/doc/linkchecker-4.7/doc/{de,fr}.po must be converted
    by msgfmt (in gettext) and be installed into proper directories
    and must not be installed as documents
  * man files
  - /usr/share/doc/linkchecker-4.7/doc/{de,fr}/linkchecker.1 must
    not be needed (man files are installed)
Comment 4 W. Michael Petullo 2007-07-14 22:44:24 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-3.fc7.src.rpm
Comment 5 Mamoru TASAKA 2007-07-15 10:53:23 EDT
Created attachment 159277 [details]
mock build log of linkchecker 4.7-3 on rawhide i386

* This time mockbuild fails again.
* And please check directory ownership issue.
  For example, the directory %{python_sitearch}/linkcheck/ is
  not owned by any package.
  Note: on i386, ppc, this directory is the same as
  %{python_sitelib}/linkcheck/ and on x86_64 and ppc64 this directory
  is also unowned (for example)
Comment 6 W. Michael Petullo 2007-07-16 21:55:22 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-4.fc7.src.rpm
Comment 7 Mamoru TASAKA 2007-07-18 12:11:31 EDT
* Umm.. still some directories are not owned.
  Currently, the file entry of your spec file can be simplified as:
-----------------------------------------------------------
%files
%defattr(-,root,root,-) <- Please add this
%{_bindir}/linkchecker
%{_datadir}/linkchecker/
%{python_sitelib}/linkcheck/
%{python_sitelib}/_linkchecker_configdata.*
%ifarch x86_64 ppc64
%{python_sitearch}/linkcheck/
%endif
%{_mandir}/man1/linkchecker.1*
%lang(de) %{_mandir}/de/man1/linkchecker.1*
%lang(fr) %{_mandir}/fr/man1/linkchecker.1*
%doc TODO doc/ cgi-bin/ README COPYING
---------------------------------------------------------------

* And still gettext po files are installed.
  By the way did you check that these .po file actually don't work?

* And I noticed that the two files under /usr/share/linkchecker
  seem to be configuration files. IMO these files should be moved
  under %_sysconfdir and marked as %config(noreplace).
Comment 8 W. Michael Petullo 2007-07-21 20:16:15 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-5.fc7.src.rpm
Comment 9 Mamoru TASAKA 2007-07-23 09:49:26 EDT
This time:

* linkchecker won't work:
-----------------------------------------------------
[root@localhost mock]# linkchecker http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/
Traceback (most recent call last):
  File "/usr//bin/linkchecker", line 527, in <module>
    config.init_logging(debug=options.debug)
  File "/usr/lib/python2.5/site-packages/linkcheck/configuration/__init__.py",
line 161, in init_logging
    logging.config.fileConfig(filename)
  File "/usr/lib/python2.5/logging/config.py", line 76, in fileConfig
    formatters = _create_formatters(cp)
  File "/usr/lib/python2.5/logging/config.py", line 107, in _create_formatters
    flist = cp.get("formatters", "keys")
  File "/usr/lib/python2.5/ConfigParser.py", line 511, in get
    raise NoSectionError(section)
ConfigParser.NoSectionError: No section: 'formatters'
-------------------------------------------------------
  - linkcheck/configuration/__init__.py really uses logging.conf
  - The configutaion directory path is set in _linkchecker_configdata.py

* gettext .mo files
  - File entries of gettext .mo files should be marked by using
    %find_lang.

* rpmlint
  - rpmlint complains:
--------------------------------------------------------
E: linkchecker non-executable-script /usr/share/linkchecker/examples/lc.cgi 0644
E: linkchecker non-executable-script
/usr/share/linkchecker/examples/check_blacklist.sh 0644
E: linkchecker non-executable-script /usr/share/linkchecker/examples/lc.fcgi 0644
E: linkchecker non-executable-script
/usr/share/linkchecker/examples/check_for_x_errors.sh 0644
E: linkchecker non-executable-script
/usr/share/linkchecker/examples/check_urls.sh 0644
---------------------------------------------------------
Comment 10 W. Michael Petullo 2007-07-24 21:23:55 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-5.fc7.src.rpm

This should fix the three problems listed in comment #9.
Comment 11 W. Michael Petullo 2007-07-24 21:24:37 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-6.fc7.src.rpm

This should fix the three problems listed in comment #9.
Comment 12 Mamoru TASAKA 2007-07-25 12:28:37 EDT
Well, 

* Still _linkchecker_configdata.py (and configuration/__init__.py)
  expects that linkcheckerrc and logging.conf are installed under
  /usr/share/linkchecker, not under /etc/linkchecker/.

  _linkchecker_configdata.py should be modified so that config_dir points
  to /etc/linkchecker. Also both logging.conf and linkcheckerrc should be 
  moved under /etc/linkchecker and be marked as %config(noreplace)

* And it seems to me that files under /usr/share/linkchecker/examples
  are documents and can be moved under /usr/share/doc/linkchecker-<version>.
Comment 13 W. Michael Petullo 2007-07-25 20:34:21 EDT
Spec URL: http://flyn.org/SRPMS/linkchecker.spec
SRPM URL: http://flyn.org/SRPMS/linkchecker-4.7-7.fc7.src.rpm

I got rid of the examples entirely.  The installer does not put them is a good
spot.  I'm not sure the value of installing example code in /usr/share/doc anyway.  

I did modify the installer to put the configuration files in /etc/linkchecker.
Comment 14 Mamoru TASAKA 2007-07-26 11:59:08 EDT
Almost okay, assiging.
Comment 15 Mamoru TASAKA 2007-07-26 12:19:20 EDT
Okay. for -7:

* gettext .po files
  - Well, actually "LANG=de_DE linkchecker <URL>" works, however
    non-ascii characters are corrupted (same for fr, perhaps and
    for es)
    Changing the encoding from ISO-8859-1 to UTF-8 (by iconv)
    seems to work.

Other things seems okay. Please fix the issue above and I think
I can approve this package (if I did not overlook anything).
Comment 16 W. Michael Petullo 2007-07-26 19:04:00 EDT
On my system, I don't see any corrupted characters.

Both "LANG=de_DE man linkchecker" and "LANG=de_DE linkchecker
http://www.flyn.org" look fine.

No corrupt characters with linkchecker-4.7-7.fc7 here.  I tested this using
gnome-terminal and a text console.
Comment 17 Mamoru TASAKA 2007-07-26 23:21:26 EDT
Ah... I had to specify locale explicitly "LANG=de_DE.UTF-8"...
Okay.

--------------------------------------------------------
   This package (linkchecker) is APPROVED by me
--------------------------------------------------------
Comment 18 Mamoru TASAKA 2007-08-01 12:43:37 EDT
Please close this bug as NEXTRELEASE when rebuild is done.
Comment 19 Mamoru TASAKA 2007-08-08 12:54:19 EDT
ping again?

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