428637 - Documentation doesn't build (and even less on Perl 5.10)

Status: RESOLVED FIXED

(Bugzilla :: Documentation, defect)

Description

[Marc Schumann [:Wurblzap]]

Attachment: Patch for tip

This started out as the “documentation landfill” Tinderbox bustage fix, for which I set out to make makedocs.pl look at module names instead of at package names.

While doing so, it turned out that the documentation doesn't build on Perl 5.10 at all. That's because for Perl 5.10, the CGI module is not included in the list of optional modules, but makedocs.pl requires it to be there.

The attached patch fixes the bustage at hand as well as the broken building process for Perl 5.10.

Comment 1

[Marc Schumann [:Wurblzap]]

(In reply to comment #0)
> This started out as the “documentation landfill” Tinderbox bustage fix, for

... for what the patch of bug 428569 broke.

Comment 2

[Max Kanat-Alexander]

Blocks 3.2, but not 3.0.4 (most people don't build their own docs), but I'm probably just being picky--I'm about to review the patch and it will go on both branches, anyhow. :-)

Comment 3

[Max Kanat-Alexander]

Comment on attachment 315251 [details] [diff] [review]
Patch for tip

>@@ -83,7 +85,8 @@ print ENTITIES "\n <!-- Database Version
> my $db_modules = DB_MODULE;
> foreach my $db (keys %$db_modules) {
>     my $dbd  = $db_modules->{$db}->{dbd};
>-    my $name = $dbd->{package};
>+    my $name = $dbd->{module};
>+    $name =~ s/::/-/g;

  That change looks unrelated, and also unnecessary.

  Otherwise, this looks OK.

Comment 4

[Max Kanat-Alexander]

This is approved assuming that you tested it.

Comment 5

[Marc Schumann [:Wurblzap]]

(In reply to comment #3)
> >-    my $name = $dbd->{package};
> >+    my $name = $dbd->{module};
> >+    $name =~ s/::/-/g;
> 
>   That change looks unrelated, and also unnecessary.

This is for consistencies (sp?) sake.

I tested up to the “To build 'The Bugzilla Guide', you need to set the JADE_PUB environment variable first.” message, and I checked the bugzilla.ent file that is being generated for completeness and correctness. I don't have the facilities to build the whole docs; can you do that?

Comment 6

[Marc Schumann [:Wurblzap]]

Oh, and I think this is indeed a 3.0.4 blocker, because without this, we won't be able to ship any docs.

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> Oh, and I think this is indeed a 3.0.4 blocker, because without this, we won't
> be able to ship any docs.

  Oh, good point.

  You have a landfill account, no? landfill should have a working docs environment.

Comment 8

[Frédéric Buclin]

Wait a minute. Why not write
  my ($cgi_opt) = grep($_->{package} eq 'CGI.pm', @$opt_modules, @$modules); instead?

This would be a one-liner and doesn't require other changes.

Also, what do we want to read in the doc? e.g. RadiusPerl or Authen::Radius?

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> Also, what do we want to read in the doc? e.g. RadiusPerl or Authen::Radius?

Also, I prefer to read Authen::Radius than Authen-Radius, so I don't like the regexp in your patch.

Comment 10

[Frédéric Buclin]

Comment on attachment 315251 [details] [diff] [review]
Patch for tip

>-    my $name = $dbd->{package};
>+    my $name = $dbd->{module};
>+    $name =~ s/::/-/g;

OK, ignore my previous comment. I thought the list of required modules was generated dynamically from here. But only the required version is, not the name of the package, so that's fine. [Note that it would make sense to also generate the list of modules dynamically, so that we don't have to remember to fix the .xml file; but that's another problem.]

I'm with Max. This change above is not necessary. Could we stick with the real fix?

Comment 11

[Frédéric Buclin]

Comment on attachment 315251 [details] [diff] [review]
Patch for tip

OK, the docs compile correctly with this patch applied, and the consistency fix doesn't hurt. So let's go with this patch as is. r=LpSolit

Comment 12

[Marc Schumann [:Wurblzap]]

Attachment: Patch for 3.0-branch

Comment 13

[Frédéric Buclin]

Comment on attachment 315274 [details] [diff] [review]
Patch for 3.0-branch

You didn't need to attach a new patch for 3.0. That's the exact same patch, only the path to the XML file changed. r=LpSolit

Comment 14

[Marc Schumann [:Wurblzap]]

All right. Better safe than sorry.

Tip:
Checking in docs/makedocs.pl;
/cvsroot/mozilla/webtools/bugzilla/docs/makedocs.pl,v  <--  makedocs.pl
new revision: 1.20; previous revision: 1.19
done
Checking in docs/en/xml/installation.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/en/xml/installation.xml,v  <--  installation.xml
new revision: 1.156; previous revision: 1.155
done

Branch:
Checking in docs/makedocs.pl;
/cvsroot/mozilla/webtools/bugzilla/docs/makedocs.pl,v  <--  makedocs.pl
new revision: 1.16.2.2; previous revision: 1.16.2.1
done
Checking in docs/xml/installation.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/Attic/installation.xml,v  <--  installation.xml
new revision: 1.136.2.13; previous revision: 1.136.2.12
done