Bug 591190

1.There are some documention in doc/, please also include them %doc

2.URL should be http://kitenet.net/~joey/code/debhelper/

3.RPM cannot pick up all dependencies automatically, you may need at least add some of the depends listed in http://packages.debian.org/en/sid/debhelper

4.Please also intall manpage for locale fr and es.

5.Considering update rpm to the latest version(7.4.19)

(In reply to comment #1)
> 
> 1.There are some documention in doc/, please also include them %doc
> 

Included.

> 2.URL should be http://kitenet.net/~joey/code/debhelper/
> 

URL Changed.

> 3.RPM cannot pick up all dependencies automatically, you may need at least add
> some of the depends listed in http://packages.debian.org/en/sid/debhelper
> 

Added the dependencies

> 4.Please also intall manpage for locale fr and es.
> 

Done.

> 5.Considering update rpm to the latest version(7.4.19)    

Done.

New SPEC: http://git.ergo-project.org/?p=kolab-fedora.git;a=blob;f=f12/custom-f12-buildsys/SPECS/debhelper.spec;hb=HEAD
New SRPM: http://koji.ergo-project.org/packages/debhelper/7.4.20/2.fc12.buildsys/src/debhelper-7.4.20-2.fc12.buildsys.src.rpm

rpmlint debhelper-7.4.20-2.fc12.buildsys.src.rpm 
debhelper.src: W: spelling-error Summary(en_US) debian -> Debian
debhelper.src: W: spelling-error %description -l en_US debian -> Debian
debhelper.src: W: spelling-error %description -l en_US debconf -> debonair, deacon, debouch
debhelper.src:11: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 11)

The last warning should be fixed.

Some other comments about spec:

Requires:       man is not needed.


%doc examples/ doc/ could be changed to %doc examples/ doc/*.

BuildRequires:  dpkg-devel seems not needed, also po4a is not available in fedora.

For man page,  lang(locale) should add before %{_mandir}/locale/ or use gettext (%find_lang --with-man)

(In reply to comment #3)
> also po4a is not available in
> fedora.

Wrong, po4a is available in Fedora.

Fixed comment 3:
> debhelper.src:11: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line
11)
Done

> Requires:       man is not needed.
Removed

> %doc examples/ doc/ could be changed to %doc examples/ doc/*.
Done

> BuildRequires:  dpkg-devel seems not needed,
Removed (Also Debian does not list dpkg-dev as BR of debhelper)

> also po4a is not available in fedora.
po4a is in Fedora (comment 4), so it was added as a BuildRequires.

> For man page,  lang(locale) should add before %{_mandir}/locale/
> or use gettext (%find_lang --with-man)
Done.

SPEC URL:
    http://oron.fedorapeople.org/deb-package/debhelper.spec
SRPM URL:
    http://oron.fedorapeople.org/deb-package/debhelper-8.0.0-1.fc13.src.rpm

rpmlint output:

$ rpmlint ./debhelper.spec /usr/local/src/rpmbuilder/rpmbuild/SRPMS/debhelper-8.0.0-1.fc13.src.rpm /usr/local/src/rpmbuilder/rpmbuild/RPMS/noarch/debhelper-8.0.0-1.fc13.noarch.rpm
debhelper.src: W: spelling-error %description -l en_US debconf -> deb conf, deb-conf, debonair
debhelper.noarch: E: devel-dependency dpkg-devel
debhelper.noarch: W: spelling-error %description -l en_US debconf -> deb conf, deb-conf, debonair
debhelper.noarch: W: spurious-executable-perm /usr/share/doc/debhelper-8.0.0/examples/rules.multi2
debhelper.noarch: W: spurious-executable-perm /usr/share/doc/debhelper-8.0.0/examples/rules.multi
debhelper.noarch: W: spurious-executable-perm /usr/share/doc/debhelper-8.0.0/examples/rules.arch
debhelper.noarch: W: spurious-executable-perm /usr/share/doc/debhelper-8.0.0/examples/rules.indep
debhelper.noarch: W: spurious-executable-perm /usr/share/doc/debhelper-8.0.0/examples/rules.tiny
debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-8.0.0/examples/rules.multi2 /usr/bin/make
debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-8.0.0/examples/rules.multi /usr/bin/make
debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-8.0.0/examples/rules.arch /usr/bin/make
debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-8.0.0/examples/rules.indep /usr/bin/make
debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-8.0.0/examples/rules.tiny /usr/bin/make
2 packages and 1 specfiles checked; 1 errors, 12 warnings.

Notes:
 * The examples/ directory contain "debian/rules" files which are, by
   definition executable "scripts" run by /usr/bin/make
   [10 out of 12 rpmlint warnings]
 * rpmlint spell warning about "debconf"
   [2 out of 12 rpm warnings, one from SRPM, another from RPM]
 * Dependency devel rpmlint error is unjustified, since debhelper
   is a devel package itself.

My fixes are meant to help unblock bug 591192 and eventually, bug 591388.
I am willing to co-maintain these packages if needed.

Forgot to mention in comment 5: bumped version to 8.0.0 to match Squeeze version.

(In reply to comment #5)
> Fixed comment 3:

> > BuildRequires:  dpkg-devel seems not needed,
> Removed (Also Debian does not list dpkg-dev as BR of debhelper)

BR: dpkg-devel is required to pull-in dpkg-parsechangelog

Without it, your package doesn't build correctly:
...
+ make build
/bin/sh: dpkg-parsechangelog: command not found
printf "package Debian::Debhelper::Dh_Version;\n\$version='';\n1" > \
		Debian/Debhelper/Dh_Version.pm
/bin/sh: dpkg-parsechangelog: command not found
cat debhelper.pod | \
		perl -e ' undef $/; foreach (@ARGV) { open (IN, $_) or die "$_: $!"; $file=<IN>; close IN; if ($file=~m/=head1 .*?\n\n(.*?) - (.*?)\n\n/s) { my $item="=item $1(1)\n\n$2\n\n"; if ($2!~/deprecated/) { $list.=$item; } else { $list_deprecated.=$item; } } } END { while (<STDIN>) { s/#LIST#/$list/; s/#LIST_DEPRECATED#/$list_deprecated/; print; }; }' `find . -maxdepth 1 -type f -perm +100 -name "dh_*" | sort` | \
		pod2man -c Debhelper -r "" --name="debhelper" --section=7  > debhelper.7
/bin/sh: dpkg-parsechangelog: command not found
/bin/sh: dpkg-parsechangelog: command not found
/bin/sh: dpkg-parsechangelog: command not found
..

It also fails to exercise its testsuite:
make test
...
t/buildsystems/buildsystem_tests .. 1/300 Can't exec "dpkg-architecture"


>  * The examples/ directory contain "debian/rules" files which are, by
>    definition executable "scripts" run by /usr/bin/make
>    [10 out of 12 rpmlint warnings]
Please chmod -x them.

Inside of the Fedora package, these are meant to be mere informative documentation and not supposed to be executable/to be run by anybody.


Furthermore:

* The package comes with a testsuite. Please add a %check section to your spec executing "make test".

Doing so, triggers a couple of errors. AFAIS, some of them seem to be originating from perl-5.10/5.12 incompatibilities in Fedora's dpkg (Could be an indication of Fedora's dpkg packages to be outdated or broken) or of Fedora's dpkg to be incompatibile to this debhelper package (I haven't investigated in depth).

* Consider (Not a blocker) to make the Makefile/configury aware of rpm's installation directories. AFAIS, upstream has hard-coded the installation paths.

That was quick, thanks, fixed. Details below.

SPEC URL:
   http://oron.fedorapeople.org/deb-package/debhelper.spec
SRPM URL:
   http://oron.fedorapeople.org/deb-package/debhelper-8.0.0-2.fc13.src.rpm

Fixed issues:
1. BR dpkg-devel, re-added.
   /me should mail Debian BTS. They may have forgotten about this.
   ( http://packages.debian.org/source/squeeze/debhelper )

2. Made examples non-executable (with a comment in the .spec)

3. Added %check:
   * The failure is due to a bug in dpkg-architecture (from dpkg-devel):
     - Opened a Fedora bug 642160 (contains a 2 liner fix)
     - Marked bug 642160 as a blocker to this review
     - Tested with a locally fixed dpkg-architecture -- all PASS
   * Added perl-Test-Pod as BR, so 'make test' does not skip some tests.

3. Added another patch to Makefile so installation paths are parametric
   and used it in the %install phase.

Another issue:
4. Requires: po-debconf
   - This means Fedora bug 591389 (review request) prevent debhelper
     from really being installed/tested.
   - Marked bug 591389 as blocker to this review
   - More work...

'%files -f %{name}.lang' should be moved down and replace the '%files' line.

> 4. Requires: po-debconf
>    - This means Fedora bug 591389 (review request) prevent debhelper
>      from really being installed/tested.

I'm still waiting for the packages there.

Fixed comment 9:
  - new SPEC URL:
    http://oron.fedorapeople.org/deb-package/debhelper.spec
  - new SRPM URL:
    http://oron.fedorapeople.org/deb-package/debhelper-8.0.0-3.fc13.src.rpm

No rush, we still wait the SRPM of po-debconf ...

*** Bug 457211 has been marked as a duplicate of this bug. ***

You may want to adopt html2tex as it's currently orphaned.

http://lists.fedoraproject.org/pipermail/devel/2011-April/150167.html

Please note that this blocks #591192 which had its Review Request done.
  https://bugzilla.redhat.com/show_bug.cgi?id=591192#c13

Chen, as current reviewer, I haven't seen any feedback or comment here from you for awhile.  are you still interested in helping to finish this review?

Continuing comment 14 above, is Jeroen van Meeuwen still interested?
For the record, since I want to have the debian build tools on Fedora,
I'm willing to help with any related package -- this means I'll
be happy to maintain/co-maintain/review (whatever helps most) this package.

New SRPM: http://hosted.kolabsys.com/~vanmeeuwen/debhelper-9.20120322-1.fc16.src.rpm
New SPEC: http://hosted.kolabsys.com/~vanmeeuwen/debhelper.spec

debhelper.noarch: W: incoherent-version-in-changelog 9.20120322 ['9.20120322-1.fc18', '9.20120322-1']
You are missing that release number "-1" in changelog entry.

debhelper.noarch: W: doc-file-dependency /usr/share/doc/debhelper-9.20120322/examples/rules.tiny /usr/bin/make
There is make, which is automatically added to requires. As it is not used anywhere else in real code, you should filter this dependency out:
http://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#.25_docdir_filtering

What happened with %check section?

Just saving Jeroen some valuable time...

New SRPM: http://oron.fedorapeople.org/deb-package/debhelper-9.20120322-2.fc15.src.rpm
New SPEC: http://oron.fedorapeople.org/deb-package/debhelper.spec

 * Filtered auto-requires for %_docdir
 * Added an %check:
   - Added BuildRequires: perl-Test-Harness (used by 'make test')
   - Also need perl-Test-More (which is not in Fedora yet)
   - So we run 'make test', but ignore the results

(In reply to comment #18)

>    - Added BuildRequires: perl-Test-Harness (used by 'make test')
>    - Also need perl-Test-More (which is not in Fedora yet)
Both changes are wrong.

* You should not require perl-<module> but use BR: perl(<module>), instead.

* perl(Test::More) is part of Fedora for ages.
  BR: perl(Test::More).

>    - So we run 'make test', but ignore the results
Why? We should not ignore the test results.

Thanks, I knew (and forgot) about the perl() thingy.
What's more -- I had Test::More installed on my host, but
foolishly tried: 'yum search perl-Test-More' instead of the correct:
  yum whatprovides 'perl(Test::More)'
which shows it to belong perl-Test-Simple (Aha!)

New SRPM: http://oron.fedorapeople.org/deb-package/debhelper-9.20120322-3.fc15.src.rpm
New SPEC: http://oron.fedorapeople.org/deb-package/debhelper.spec

Now all tests are good. Also added BR: perl(Test::Pod) which
enables some optional tests if present.

http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

Assigned reviewer has not responded to query in comment 14 (2012-01-15).

Assigned reviews are not listed at
  http://fedoraproject.org/PackageReviewStatus/NEW.html
so that bears a higher risk that a review stalls and no other reviewers become aware of the review request.

Who will be a maintainer: Oron Peled (need to open new entry) or Jeroen van Meeuwen? Both?
Just to clear it out.

IMHO, we should follow 
http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
and close this review request + somebody else resubmit a new one.

* I'm still alive and willing to maintain this.

* It was blocked on rhbz#591389 (RR for po-debconf):
  - If you can, please help solve an irritating bug on '%check'
    (documented on the new .spec file linked from the RR)
  - It has a working and up-to-date SRPM:
    http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16+nmu2-1.fc18.src.rpm

* Also, having up to date debhelper is now blocked
  on rhbz#648384 (dpkg version too old)
  - It's stalled for 2 years now
  - I added a link to proposed .spec + srpm:
    http://oron.fedorapeople.org/deb-package/dpkg-1.16.10-1.fc18.src.rpm
  - I've stated I'm willing to maintain/co-maintain it
    [as I write this, I see Andrew Colin Kissa updated the BR and
    said he'll fix it RSN -- thank you]

* Here is an up-to-date debhelper (same as Debian/wheezy -- hooray).
  For this to build, you need to use the po-debhelper and dpkg linked
  to bugs I mentioned above:

  Spec URL: http://oron.fedorapeople.org/deb-package/debhelper.spec
  SRPM URL:
      http://oron.fedorapeople.org/deb-package/debhelper-9.20120322-3.fc15.src.rpm

Oron Peled (In reply to comment #22)
> Who will be a maintainer: Oron Peled (need to open new entry) or Jeroen van
> Meeuwen? Both?
> Just to clear it out.

Oron could you open a new  Review Request: debhelper , to move on , I will review it

* Opened rhbz#961141
* Jeroen, thanks a lot for making it happen. If you still want to co-maintain it, just let me know.

*** This bug has been marked as a duplicate of bug 961141 ***