libwww-perl / libwww-perl/HTTP-Message

HTTP::Message uses top-level require where its siblings use "use Foo ()"

Open Beginner friendly
#228 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Perl
Stars
32
Forks
63
Avg merge
5h 14m
Merged PRs (30d)
1

Description

lib/HTTP/Message.pm opens with two top-level requires:

require HTTP::Headers;
require Carp;

Its sibling lib/HTTP/Headers.pm does the same job with the modern idiom:

use Carp ();

as does lib/HTTP/Request/Common.pm (use Carp();). Message.pm is the odd one out. Digging into why turned out to be more interesting than "old code".

History

The require Carp is not drift -- it was a deliberate change, and the commit message says so outright:

commit 25b4071747eb0bcaddd3978e0a2f17f7d84ce34c
Author: Gisle Aas <gisle@aas.no>
Date:   Sun Aug 27 22:32:25 1995 +0000

    Cange "use Carp;" into "require Carp;"
 require HTTP::Headers;
-use Carp;
+require Carp;

The intent was load it, don't import it, and the call sites were qualified to match. That intent still holds today: Message.pm has 14 fully-qualified Carp:: calls and zero bare croak/carp. HTTP::Headers exports nothing at all, so neither line wants an import.

Critically, that 1995 commit touched both Message.pm and Headers.pm -- they were changed in lockstep.

They stopped being in lockstep two and a half years later. In January 1998, in a commit whose message is just Added $VERSION., Headers.pm was modernized as a drive-by:

commit 7d89cffe...
Author: Gisle Aas <gisle@aas.no>
Date:   Tue Jan 6 1998
+use vars qw($VERSION);
+$VERSION = sprintf("%d.%02d", q$Revision: 1.31 $ =~ /(\d+)\.(\d+)/);
-require Carp;
+use Carp ();

Message.pm never got the same treatment. So the inconsistency is a 28-year-old missed edit, buried in a commit that wasn't about imports and wouldn't turn up in any search for one.

require HTTP::Headers is older still -- it dates from ef6daa1c (9 Aug 1995, "Renamed to HTTP::Message, and use HTTP::Headers.") and was written as a require from the start.

What should NOT change

Message.pm has ~25 further requires indented inside subs. Almost all are deliberate and need to stay:

  • Optional / heavy dependencies loaded on demand -- Compress::Raw::Zlib, Compress::Raw::Bzip2, IO::Uncompress::*, IO::Compress::*, Encode, MIME::Base64, MIME::QuotedPrint, IO::HTML. Lazy loading is the entire point.
  • A circular-dependency break at lines 793-794:
    require HTTP::Request;
    require HTTP::Response;
    
    Both of those do use parent 'HTTP::Message', so Message.pm cannot load them at file scope. This one is load-bearing.

Any "prefer use over require" sweep over this file would be wrong for roughly 23 of the 25.

Proposed change

Only the two top-level lines:

-require HTTP::Headers;
-require Carp;
+use HTTP::Headers ();
+use Carp ();

This is cosmetic. At file scope both forms execute during the enclosing load, so there is no behaviour change; use merely fails at compile time rather than run time, and the file stops contradicting its own siblings.

I checked for a dependency cycle before proposing it: HTTP::Headers does not reference HTTP::Message anywhere (the require HTTP::Headers in Headers.pm is inside the SYNOPSIS pod, after __END__). Within the distribution only Request.pm and Response.pm load HTTP::Message, and that cycle is already handled at lines 793-794. So promoting these two to compile time is safe.

Happy to send a PR if this is wanted. Equally happy for it to be closed as not worth the churn -- the code has been correct for 30 years and this is purely a consistency argument.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Open lib/HTTP/Message.pm and inspect its two top-level require statements alongside the corresponding imports in lib/HTTP/Headers.pm and lib/HTTP/Request/Common.pm. Change only those top-level lines to use with empty import lists, leaving the indented lazy requires and circular-dependency break unchanged. Done means the top-level imports match the sibling modules without altering runtime behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
perl
Domain
api
Issue type
Refactor
Difficulty
1/5
Estimated time
Under an hour
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.