Bug 810223 - Circular build dependency in perl-HTTP-Message-6.03-1.fc18
Summary: Circular build dependency in perl-HTTP-Message-6.03-1.fc18
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: perl-HTTP-Message
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-04-05 12:15 UTC by Paul Howarth
Modified: 2012-04-16 13:55 UTC (History)
3 users (show)

Fixed In Version: perl-HTTP-Message-6.03-2.fc18
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-06 11:39:36 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Paul Howarth 2012-04-05 12:15:17 UTC
perl-HTTP-Message buildrequires perl(HTML::Parser), which requires perl(HTTP::Headers), which is provided by perl-HTTP-Message. This will be a problem when it comes to bootstrapping perl 5.16

Suggested workaround, dropping perl(HTML::Parser) buildreq and affected test when bootstrapping:

diff --git a/perl-HTTP-Message.spec b/perl-HTTP-Message.spec
index 747d01d..95608aa 100644
--- a/perl-HTTP-Message.spec
+++ b/perl-HTTP-Message.spec
@@ -12,7 +12,10 @@ BuildRequires:  perl(Encode) >= 2.12
 BuildRequires:  perl(Encode::Locale) >= 1
 BuildRequires:  perl(Exporter)
 BuildRequires:  perl(ExtUtils::MakeMaker)
+# HTML::Parser -> HTTP::Headers (provided by this package)
+%if 0%{!?perl_bootstrap:1}
 BuildRequires:  perl(HTML::Parser) >= 3.33
+%endif
 BuildRequires:  perl(HTTP::Date) >= 6
 BuildRequires:  perl(IO::Compress::Bzip2) >= 2.021
 BuildRequires:  perl(IO::Compress::Deflate)
@@ -77,7 +80,12 @@ find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;
 %{_fixperms} $RPM_BUILD_ROOT/*
 
 %check
+%if 0%{!?perl_bootstrap:1}
 make test
+%else
+# t/message-charset.t exercises HTTP::Message, which requires HTML::Parser
+make test TEST_FILES="$(echo $(find t/ -name '*.t' | grep -Fvx t/message-charset.t))"
+%endif
 
 %files
 %defattr(-,root,root,-)

Comment 1 Petr Pisar 2012-04-05 14:54:19 UTC
Thank you for finding this cycle, however I believe I've already fixed it on perl-HTML-Parser side since:

commit 5501683d6bad7205ef78c6c4b842a2952842d2bd
Author: Petr Písař <ppisar>
Date:   Wed Jan 18 16:46:46 2012 +0100

    Exclude HTTP::Headers dependency at Perl bootstrap

diff --git a/perl-HTML-Parser.spec b/perl-HTML-Parser.spec
index 6a1cc0f..7b7f88e 100644
--- a/perl-HTML-Parser.spec
+++ b/perl-HTML-Parser.spec
@@ -3,7 +3,7 @@
 Name:           perl-%{real_name}
 Summary:        Perl module for parsing HTML
 Version:        3.69
-Release:        2%{?dist}
+Release:        3%{?dist}
 License:        GPL+ or Artistic
 Group:          Development/Libraries
 Source0:        http://search.cpan.org/CPAN/authors/id/G/GA/GAAS/HTML-Parser-%{version}.tar.gz 
@@ -12,14 +12,18 @@ Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $versi
 BuildRequires:  perl(Carp)
 BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(HTML::Tagset) >= 3
-BuildRequires:  perl(HTTP::Headers)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(URI)
 BuildRequires:  perl(XSLoader)
 Requires:       perl(HTML::Tagset) >= 3
-Requires:       perl(HTTP::Headers)
 Requires:       perl(URI)
 Requires:       perl(XSLoader)
+%if %{undefined perl_bootstrap}
+# This creates cycle with perl-HTTP-Message. Weaken the dependency here because
+# it's just a recommended dependency per META.yml.
+BuildRequires:  perl(HTTP::Headers)
+Requires:       perl(HTTP::Headers)
+%endif
 
 %{?perl_default_filter}
 %{?perl_default_subpackage_tests}
@@ -58,6 +62,9 @@ make test
 
 
 %changelog
+* Wed Jan 18 2012 Petr Pisar <ppisar> - 3.69-3
+- Exclude HTTP::Headers dependency at Perl bootstrap
+
 * Fri Jan 13 2012 Fedora Release Engineering <rel-eng.org> - 3.69-2
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_17_Mass_Rebuild

Comment 2 Paul Howarth 2012-04-05 16:15:01 UTC
Hmm, you're right.

However, a problem with having a Requires: removed based on %perl_bootstrap is that the resulting package is actually different from a non-bootstrap build, which is why I prefer to just exclude build requirements and not runtime ones. It also makes it difficult to detect the broken cycle using a script without actually rebuilding all packages with the bootstrap macro set, which is a much bigger job than processing the metadata to find the runtime dependencies.

Comment 3 Petr Pisar 2012-04-06 08:01:24 UTC
(In reply to comment #2)
> However, a problem with having a Requires: removed based on %perl_bootstrap is
> that the resulting package is actually different from a non-bootstrap build,

Yes. One need to rebuild the package again once bootstrap is complete.

> which is why I prefer to just exclude build requirements and not runtime ones.

I understand, but in this case my solution reflects the relation between the two packages much better. HTML-Parser does not need HTTP-Headers, it can run without them. But the opposite is not true. Following upstream nature dependencies is better.

> It also makes it difficult to detect the broken cycle using a script without
> actually rebuilding all packages with the bootstrap macro set, which is a much
> bigger job than processing the metadata to find the runtime dependencies.

Of course. You need to rebuild the packages to get proper dependencies. Otherwise you just guess. Even it's not always possible to just drop build-requires and keep the run-requires. Otherwise you will end up with built binary package with unsatisfied run-dependencies, so you could not use it while building reverse dependencies.

I'm reluctant to follow the easier but error-prone way you selected.

I can put your change into perl-HTTP-Message if it makes you happy because it just disables a test, but I don't think it's the best way.

Comment 4 Paul Howarth 2012-04-06 09:17:36 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > However, a problem with having a Requires: removed based on %perl_bootstrap is
> > that the resulting package is actually different from a non-bootstrap build,
> 
> Yes. One need to rebuild the package again once bootstrap is complete.

Absolutely. All packages that use %perl_bootstrap should be rebuilt post-bootstrap in case previously-skipped tests cause FTBFS problems.

> > which is why I prefer to just exclude build requirements and not runtime ones.
> 
> I understand, but in this case my solution reflects the relation between the
> two packages much better. HTML-Parser does not need HTTP-Headers, it can run
> without them. But the opposite is not true. Following upstream nature
> dependencies is better.

I agree in principle with this. I try to find a sensible place to break cycles. My suggested fix only excludes HTML::Parser as a build requirement, not as a runtime dependency in the built package (which would be picked up automatically by rpm). It's no worse really than simply skipping the tests altogether, which a few packages do when bootstrapping. The resulting binary packages are identical with or without %perl_bootstrap.

On the other hand, your change in the perl-HTML-Parser package results in a different binary package when built using %perl_bootstrap, i.e. it has no dependency on perl(HTTP::Headers). So to be sure of not introducing FTBFS errors as a result of this, it would be necessary to rebuild not only perl-HTML-Parser post-bootstrap, but every package whose build pulls in perl-HTML-Parser, in case the addition of perl-HTTP-Message in the buildroot makes a difference.

> > It also makes it difficult to detect the broken cycle using a script without
> > actually rebuilding all packages with the bootstrap macro set, which is a much
> > bigger job than processing the metadata to find the runtime dependencies.
> 
> Of course. You need to rebuild the packages to get proper dependencies.

By only removing build requirements and not runtime dependencies during the bootstrap process, I believe all bootstrap-built packages should already have proper dependencies.

> Otherwise you just guess. Even it's not always possible to just drop
> build-requires and keep the run-requires. Otherwise you will end up with built
> binary package with unsatisfied run-dependencies, so you could not use it while
> building reverse dependencies.

I haven't come across a case where that necessarily applies yet, but I understand how it could be possible.

> I'm reluctant to follow the easier but error-prone way you selected.
> 
> I can put your change into perl-HTTP-Message if it makes you happy because it
> just disables a test, but I don't think it's the best way.

Well I disagree about the best way as outlined above but I can adjust my script to deal with it on a case-by-case basis.

Comment 5 Petr Pisar 2012-04-06 11:39:36 UTC
I added your changed into perl-HTTP-Message-6.03-2.fc18.

Comment 6 Paul Howarth 2012-04-06 12:09:30 UTC
Thanks Petr.

Comment 7 Paul Howarth 2012-04-06 14:31:36 UTC
Perhaps it might be a good idea to revert the changes in perl-HTML-Parser from Comment #1 now, given that it's no longer needed?

Comment 8 Paul Howarth 2012-04-12 14:15:20 UTC
(In reply to comment #7)
> Perhaps it might be a good idea to revert the changes in perl-HTML-Parser from
> Comment #1 now, given that it's no longer needed?

I've worked out that 66 packages should be rebuilt post-bootstrap because perl-HTTP-Message wouldn't be in their buildroots during the boot phase as a result of the disabled runtime dependency in perl-HTML-Parser's bootstrap build. So I really think it would be a good idea to revert the perl-HTML-Parser bootstrap changes now.

Comment 9 Petr Pisar 2012-04-16 07:01:36 UTC
Change 5501683d6bad7205ef78c6c4b842a2952842d2bd reverted in perl-HTML-Parser-3.69-4.fc18.

Comment 10 Paul Howarth 2012-04-16 10:14:58 UTC
I'm really sorry about this but the BuildRequires: perl(HTTP::Headers) needs to be excluded from perl-HTML-Parser when bootstrapping (the Requires: is OK). perl-HTTP-Message always has a runtime dependency on perl-HTML-Parser and this creates a build-dep cycle.

Again, really sorry about messing you about here.

Comment 11 Petr Pisar 2012-04-16 13:11:24 UTC
I removed the build-dependency only in perl-HTML-Parser-3.69-5.fc18.

Please make sure none of the two packages will be needed for building other packages (even transitively) during the bootstrap. Because now you can build both of them, but you cannot use any of them (the run-time dependencies will be unsatisfied).

Comment 12 Paul Howarth 2012-04-16 13:44:31 UTC
(In reply to comment #11)
> I removed the build-dependency only in perl-HTML-Parser-3.69-5.fc18.

Thanks.

> Please make sure none of the two packages will be needed for building other
> packages (even transitively) during the bootstrap. Because now you can build
> both of them, but you cannot use any of them (the run-time dependencies will be
> unsatisfied).

Why do you say that? Once they are both built, they are both installable, are they not?

perl-HTML-Parser is very low down in the build dependency hierarchy, and if it was uninstallable then it would impact the build of 772 other packages, which either build-require it or some other package that build-requires/requires it.

The build order as I see it at the moment is available here:
http://www.city-fan.org/~paul/perl-boot/buildorder

perl-HTML-Parser can be built in the 2nd pass and perl-HTTP-Message can be built in the 3rd pass, so they should both be usable for builds from the 4th pass onwards (where packages in pass N have at least one build dependency [or runtime dependency of one] built in the previous pass and all packages in a given pass can be built in parallel as they have no dependencies on each other).

I described the boot process as I understand it in an email to the perl-devel list last week:
http://lists.fedoraproject.org/pipermail/perl-devel/2012-April/050071.html

Comment 13 Petr Pisar 2012-04-16 13:55:42 UTC
Provided you will maintain decomposition of rebuilds into passes to minimize incoherent states as much as possible, then you are right. I pointed just the fact. Obviously I forgot the state where both packages have been already rebuilt. My apology.


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