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.

Assessment

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

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.

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.

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

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.

More from libwww-perl/HTTP-Message

All issues in libwww-perl/HTTP-Message

Similar issues

More Perl issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.