From 53bafaa9dc9f8ec768dc1afa004340546b545fa3 Mon Sep 17 00:00:00 2001
From: espie <espie@openbsd.org>
Date: Tue, 16 May 2023 14:28:39 +0000
Subject: [PATCH] document a few small details, some of them non-obvious like
 the inheritance hierarchy or the exact interface for ->add

---
 usr.sbin/pkg_add/OpenBSD/PackingElement.pm | 42 +++++++++++++++++++---
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/usr.sbin/pkg_add/OpenBSD/PackingElement.pm b/usr.sbin/pkg_add/OpenBSD/PackingElement.pm
index 864fb13141e..4f2deb771f7 100644
--- a/usr.sbin/pkg_add/OpenBSD/PackingElement.pm
+++ b/usr.sbin/pkg_add/OpenBSD/PackingElement.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: PackingElement.pm,v 1.284 2022/11/04 13:22:15 espie Exp $
+# $OpenBSD: PackingElement.pm,v 1.285 2023/05/16 14:28:39 espie Exp $
 #
 # Copyright (c) 2003-2014 Marc Espie <espie@openbsd.org>
 #
@@ -27,6 +27,11 @@ require 5.008_000;
 # This is the basic class, which is mostly abstract, except for
 # create and register_with_factory.
 # It does provide base methods for stuff under it, though.
+
+# XXX PackingElement uses (very seldom) multiple inheritance:
+#	the subclasses ::DirBase and ::Unique are used as mix-ins
+# 	and thus contain very limited functionality !
+
 package OpenBSD::PackingElement;
 our %keyword;
 
@@ -84,6 +89,8 @@ sub register_manpage
 {
 }
 
+# plist keeps a "state" while reading a plist
+# $self->destate($plstate)
 sub destate
 {
 }
@@ -96,6 +103,14 @@ sub add_object
 	return $self;
 }
 
+# $class->add($plist, @args):
+# 	create an object with the correct arguments
+#	returns the actual object created, IF ANY (XXX see subclasses
+#	for instances of annotations like @symlink that DON'T create
+#	an actual object)
+#
+#	most add methods have ONE single argument, except for
+#	subclasses generated from comments !
 sub add
 {
 	my ($class, $plist, @args) = @_;
@@ -118,6 +133,7 @@ sub write
 	}
 }
 
+# specialized version to avoid copying digital signatures over
 sub write_no_sig
 {
 	my ($self, $fh) = @_;
@@ -208,6 +224,9 @@ sub cwd
 	return ${$_[0]->{cwd}};
 }
 
+# most objects should be fs relative, but there are
+# exceptions, such as sample files that will get installed
+# under /etc, or rc files !
 sub absolute_okay() { 0 }
 sub compute_fullname
 {
@@ -320,6 +339,7 @@ our @ISA=qw(OpenBSD::PackingElement::Object);
 package OpenBSD::PackingElement::Meta;
 our @ISA=qw(OpenBSD::PackingElement);
 
+# XXX mix-in class, see comment at top of file
 package OpenBSD::PackingElement::Unique;
 our @ISA=qw(OpenBSD::PackingElement::Meta);
 
@@ -514,6 +534,12 @@ sub destate
 
 sub dirclass() { "OpenBSD::PackingElement::Sampledir" }
 
+# TODO @ghost data is not yet used
+# it's meant for files that used to be "registered" but are
+# somewhat autogenerated or something, and should vanish in a transparent way.
+#
+# the keyword was introduced very early but is (still) not used
+
 package OpenBSD::PackingElement::Ghost;
 our @ISA = qw(OpenBSD::PackingElement::FileObject);
 
@@ -596,7 +622,8 @@ __PACKAGE__->register_with_factory;
 sub register_manpage
 {
 	my ($self, $state, $key) = @_;
-	# XXX don't bother register stuff from partial packages
+	# optimization: don't bother registering stuff from partial packages
+	# (makewhatis will complain that the names don't match anyway)
 	return if defined $self->{tempname};
 	my $fname = $self->fullname;
 	if ($fname =~ m,^(.*/man(?:/\w+)?)/((?:man|cat)[1-9n]\w*/.*),) {
@@ -724,7 +751,11 @@ our @ISA=qw(OpenBSD::PackingElement::FileBase);
 sub keyword() { "ltlib" }
 __PACKAGE__->register_with_factory;
 
-# Comment is very special
+# Comment is very special:
+#	- some annotations are comments for historic reasons
+#	- CVSTags need to be recognized for register-plist (obsolescent)
+#	- tools like update-plist will recognize @comment'ed entries
+#	and thus destate needs to run on normal comments
 package OpenBSD::PackingElement::Comment;
 our @ISA=qw(OpenBSD::PackingElement::Meta);
 
@@ -900,7 +931,7 @@ our @ISA=qw(OpenBSD::PackingElement::UniqueOption);
 
 sub category() { 'manual-installation' }
 
-# XXX don't incorporate this in signatures.
+# don't incorporate this in signatures for obvious reasons
 sub write_no_sig()
 {
 }
@@ -2156,7 +2187,8 @@ for my $k (qw(src display mtree ignore_inst dirrm pkgcfl pkgdep newdepend
 	__PACKAGE__->register_old_keyword($k);
 }
 
-# Real pkgpath objects, with matching properties
+# pkgpath objects are parsed in extrainfo and pkgpath objects
+# so that erroneous pkgpaths will be flagged early
 package OpenBSD::PkgPath;
 sub new
 {
-- 
2.20.1