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

Đang mở Phù hợp với người mới
  • Có gì không ổn?
#228 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Đánh giá

Độ khó
1/5
Thời gian dự kiến
Dưới một giờ
Mức phù hợp với người mới
88/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Ít trao đổi
Công nghệ
perl
Lĩnh vực
api

Hướng nghiên cứu

Mở lib/HTTP/Message.pm và kiểm tra hai câu lệnh require ở cấp cao nhất cùng với các import tương ứng trong lib/HTTP/Headers.pm và lib/HTTP/Request/Common.pm. Chỉ thay đổi các dòng cấp cao nhất đó để sử dụng các danh sách import rỗng, giữ nguyên các lazy requires được thụt lề và phần phá vỡ phụ thuộc vòng. Hoàn tất khi các import ở cấp cao nhất khớp với các import của các module cùng nhóm mà không thay đổi hành vi lúc chạy.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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 <[email protected]>
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 <[email protected]>
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.

Ngôn ngữ chính
Perl
Star
32
Fork
62
Không có pull request nào được merge trong 30 ngày

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của libwww-perl/HTTP-Message

Tất cả issue của libwww-perl/HTTP-Message

Issue tương tự

Thêm issue về Perl

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.