src/mplib/portlint.pm

use v5.16;
package mplib::portlint;
use strict;
use warnings FATAL => 'all';
use Exporter 'import';
use mplib::resolve qw(effective_deps);
 
our @EXPORT_OK = qw(lint_port KNOWN_HUGE_REPOS);
 
# every check below exists because it's a real bug this tree actually
# shipped and a real install caught -- not a hypothetical. see each
# check's own comment for which port. severity: "high" = confirmed to
# actually break an install/remove in the exact shape seen; "medium" =
# very likely broken, not individually re-verified for every possible
# shape; "low" = worth a second look, not necessarily wrong.
 
# repos big enough that a full-history "git clone" (this tree's own
# default -- see mk/b.sh's own convention) is a real, worthwhile-to-flag
# cost, as opposed to the ~250 ordinary git clones this tree leaves alone
# on purpose (a static audit this session confirmed that's the tree's own
# deliberate convention, not 250 independent oversights). keyed on a
# lowercase substring match against the clone URL.
our @KNOWN_HUGE_REPOS = qw(
	gcc-mirror/gcc sourceware.org/git/glibc binutils-gdb
	llvm/llvm-project python/cpython php/php-src erlang/otp
	ocaml/ocaml mariadb/server postgres/postgres github.com/git/git
	apache/subversion
);
 
# tool => [ how it can show up in text, what to require in effective deps ]
# each entry: [ qr/invocation pattern/, [ list of dep tokens, ANY of which
# satisfies it -- a literal name or a %tag ], "human name for the msg" ].
my @TOOL_SIGNATURES = (
	[ qr/\bcmake\b/,                 ['cmake'],                              'cmake' ],
	[ qr/\bmeson\b/,                 ['%meson', 'meson'],                    'meson' ],
	[ qr/\bautoreconf\b|\bautoconf\b|\bautoheader\b/,
	                                 ['gnu-autoconf'],                       'autoconf/autoreconf' ],
	[ qr/\bautomake\b|\baclocal\b/,  ['gnu-automake'],                       'automake' ],
	[ qr/\blibtoolize\b/,            ['gnu-libtool'],                        'libtool' ],
	[ qr/\bpkg-config\b/,            ['%pkg-config', 'pkg-config', 'pkgconf'], 'pkg-config' ],
	[ qr/\bbison\b|\byacc\b/,        ['gnu-bison'],                          'bison' ],
	[ qr/\bflex\b/,                  ['flex'],                               'flex' ],
	[ qr/\bmsgfmt\b|\bxgettext\b/,   ['gnu-gettext'],                        'gettext (msgfmt/xgettext)' ],
);
 
sub _finding { return { severity => $_[0], msg => $_[1] }; }
 
# strips shell comment lines (this codebase's own convention: a "#" as
# the first non-blank character of the line -- same rule
# mplib::portformat::read_port already applies to metadata lines) before
# any regex check that cares about actually-invoked commands, not prose
# explaining a design decision. without this, e.g. gnu-m4/gnu-texinfo's
# own comments explaining exactly why they DON'T depend on gnu-autoconf
# (to avoid a real bootstrap cycle) get misread as evidence that they DO
# invoke autoreconf.
sub _strip_comments {
	my ($text) = @_;
	return join("\n", grep { !/^\s*#/ } split /\n/, $text) . "\n";
}
 
# a legacy add.sh cram everything (fetch, configure, build, install) into
# one "install" phase, but a new-format class-based port (inherit=cmake,
# say) splits it: pkg_fetch synthesizes "fetch", and the actual
# cmake/./configure/meson invocation lives in "build", with "install"
# itself often just "make install" or "meson install -C build". any check
# that cares about "what does building this actually run" needs all
# three, comment-stripped, or it silently only ever sees legacy ports.
sub _build_text {
	my ($pc) = @_;
	return _strip_comments(
	    ($pc->{_phases}{fetch}   || '') . "\n"
	    . ($pc->{_phases}{build}   || '') . "\n"
	    . ($pc->{_phases}{install} || ''));
}
 
# high: install runs (or looks like) a cmake build, but remove doesn't
# use install_manifest.txt -- either "rm -rf build" (throws the manifest
# away before ever reading it, so mp remove silently leaves every
# installed file behind) or a bare "make uninstall"/"cmake --build ...
# uninstall" (cmake's default generated Makefile never defines an
# uninstall target at all, so this fails outright under set -e). found in
# 19 ports this session (brotli, cppcheck, bear, doxygen, ccache, znc,
# weechat, leveldb, mariadb, taglib, flac, qpdf, exiv2, ledger,
# taskwarrior, llvm, minisign, clamav, mbedtls) -- the correct pattern
# (classes/cmake.mp already has it right) is
# "cd <dir>/build && xargs rm -f < install_manifest.txt".
sub check_cmake_uninstall {
	my ($pc) = @_;
	my $install = _build_text($pc);
	my $remove  = _strip_comments($pc->{_phases}{remove}  || '');
	return () unless $install =~ /\bcmake\b/;
	return () if $remove =~ /install_manifest\.txt/;
	return () unless $remove =~ /\bmake\s+uninstall\b|\brm\s+-rf\s+\S*build\S*\b|--target\s+uninstall/;
	return _finding('high',
	    'cmake build, but remove doesn\'t use install_manifest.txt -- cmake\'s default '
	    . 'Makefile has no uninstall target, and "rm -rf build" throws the manifest away '
	    . 'before ever using it. use: cd <dir>/build && xargs rm -f < install_manifest.txt');
}
 
# medium: a known build/codegen tool is invoked in the install phase, but
# nothing in the effective (class-merged) pkg_deps looks like it could
# provide that tool. found repeatedly this session: less/mtr/par2cmdline/
# gawk (autotools triad), php/postgresql (bison), htop/gnu-nano
# (gettext), tmux (a case where the dep WAS declared but happened to be a
# stale sandbox ghost -- this check can't see that, only the declaration).
sub check_missing_tool_deps {
	my ($pc, $canon) = @_;
	my $install = _build_text($pc);
	my @deps = effective_deps($pc, $canon);
	my %have = map { $_ => 1 } @deps;
	# a port providing the tag/name a satisfier list names is exempt from
	# needing it declared as its OWN dependency (mirrors resolve.pm's own
	# GLOBAL_DEPS bootstrap-provider exemption, same reasoning: cmake's
	# own add.sh mentions "cmake" constantly -- cloning cmake's source,
	# bootstrapping cmake -- without ever needing a SEPARATE cmake
	# dependency to do it).
	my %provides = map { $_ => 1 } (split(' ', $pc->{pkg_tags} || ''), $pc->{pkg_name});
	my @out;
	for my $sig (@TOOL_SIGNATURES)
	{
		my ($re, $satisfiers, $label) = @$sig;
		next unless $install =~ $re;
		next if grep { $have{$_} } @$satisfiers;
		next if grep { my $s = $_; $s =~ s/^%//; $provides{$s} } @$satisfiers;
		push @out, _finding('medium',
		    "install phase invokes $label, but pkg_deps has none of: " . join(', ', @$satisfiers));
	}
	return @out;
}
 
# medium: pkg_ver="git" (this tree's overwhelming default -- 281/288
# ports) but the install/fetch phase fetches a fixed, versioned tarball
# instead of doing a git clone at all. found on 13 ports this session
# (lua, ruby, gnu-global, gperf, gnu-gettext, mpg123, potrace, giflib,
# gnu-enscript, links, gnu-a2ps, help2man, gnu-units) -- xz/gnu-m4/
# gnu-texinfo do the same tarball-fetch shape correctly, with a real
# pinned pkg_ver and an explanatory comment (bootstrap-cycle avoidance).
sub check_stale_pkg_ver_git {
	my ($pc) = @_;
	return () unless ($pc->{pkg_ver} || '') eq 'git';
	my $text = _strip_comments(($pc->{_phases}{fetch} || '') . ($pc->{_phases}{install} || ''));
	return () if $text =~ /\bgit\s+clone\b/;
	return () unless $text =~ /\.tar\.(gz|bz2|xz|lz)\b/;
	return _finding('medium',
	    'pkg_ver="git" but this fetches a versioned tarball, not a git checkout -- '
	    . 'set pkg_ver to the real embedded version instead (see xz/gnu-m4/gnu-texinfo '
	    . 'for the same shape done correctly, with pkg_ver pinned).');
}
 
# high: legacy add.sh/del.sh only -- the remove phase "cd"s into a
# directory name that doesn't match what the install phase's "git clone
# <url> [destdir]" would actually create (destdir if given, else the
# repo's own basename minus .git). found in init/runit this session:
# add.sh cloned into "runit-2.3.1", del.sh cd'd into "runit" -- del.sh
# failed outright, every single time, under set -e.
sub check_del_cd_mismatch {
	my ($pc) = @_;
	return () unless $pc->{_legacy};
	my $install = _strip_comments($pc->{_phases}{install} || '');
	my $remove  = _strip_comments($pc->{_phases}{remove}  || '');
	return () unless $install =~ /^\s*git\s+clone\s+(.*)$/m;
	# skip any number of "--flag" / "--flag=value" tokens (--depth 1,
	# --recursive, --branch X, ...) to reach the url, then an optional
	# destdir token right after it -- same shape "git clone" itself
	# accepts, just without a real arg parser.
	# flags known to take their value as a SEPARATE next token (as opposed
	# to --recursive, or a --flag=value form, which split on whitespace
	# already leaves as one self-contained token).
	my %TAKES_VALUE = map { $_ => 1 } qw(--depth --branch -b);
	my @tok = split ' ', $1;
	while(@tok && $tok[0] =~ /^-/)
	{
		my $flag = shift @tok;
		shift @tok if $TAKES_VALUE{$flag} && @tok;
	}
	return () unless @tok;
	my ($url, $destdir) = @tok;
	my $expect = (defined $destdir && $destdir ne '') ? $destdir : do {
		my ($base) = $url =~ m{([^/]+?)(?:\.git)?$};
		$base;
	};
	return () unless defined $expect && $expect ne '';
	# a plain "mv <clonedname> <newname>" right after (sxiv/nsxiv-style
	# rename-after-clone) moves the real target -- check that name instead.
	if($install =~ /^\s*mv\s+\Q$expect\E\s+(\S+)\s*$/m)
	{
		$expect = $1;
	}
	return () unless $remove =~ /^\s*cd\s+(\S+)/m;
	my $actual = $1;
	# an exact match, or a cd into a SUBDIRECTORY of the right checkout
	# (e.g. "cd brotli/build", this tree's own correct cmake del.sh
	# pattern) are both fine -- only a genuinely different top-level name
	# is the real bug (init/runit: cloned into "runit-2.3.1", cd'd into
	# "runit").
	return () if $actual eq $expect || $actual =~ m{^\Q$expect\E/};
	return _finding('high',
	    "remove phase cd's into \"$actual\", but install's git clone would create \"$expect\" -- "
	    . 'remove will fail outright the first time it runs.');
}
 
# low (medium if this port opted out of the safety net below): remove
# phase assumes "make uninstall" works, but nothing in the install
# phase looks like autotools/cmake/meson (all three reliably generate a
# real uninstall target) -- a hand-written Makefile very often has none
# at all. found in dropbear this session ("No rule to make target
# 'uninstall'", every file left behind after every mp remove) -- since
# fixed at the mp level, not per-port: mplib::resolve::
# clean_manifest_files now sweeps whatever a package's own install
# manifest still lists once remove: has had its chance (mp remove even
# tolerates remove: failing outright now, exactly dropbear's case, and
# still runs the sweep afterward), so this is a style/clarity nit for
# most ports, not a real "files left behind" risk anymore. a port that
# opted out with pkg_manifest_cleanup="no" has no such backstop, so it
# keeps the sharper original wording.
sub check_uninstall_target_risk {
	my ($pc) = @_;
	my $install = _build_text($pc);
	my $remove  = _strip_comments($pc->{_phases}{remove}  || '');
	return () unless $remove =~ /\bmake\s+(?:-\S+\s+)*uninstall\b/;
	return () if $install =~ /\bautoreconf\b|\bautoconf\b|\.\/configure\b|\bcmake\b|\bmeson\b|\bautogen\.sh\b|\bbootstrap\b/;
	my $swept = !(defined $pc->{pkg_manifest_cleanup} && $pc->{pkg_manifest_cleanup} =~ /^(n|no|0|false)$/i);
	return _finding($swept ? 'low' : 'medium',
	    '"make uninstall" in remove, but install doesn\'t look like autotools/cmake/meson -- '
	    . 'confirm upstream\'s own Makefile really defines an uninstall target '
	    . ($swept
	        ? '(mp remove sweeps this port\'s own install manifest for anything left behind either way, '
	          . 'so this is a clarity nit, not a real risk -- see pkg_manifest_cleanup in ports/template/pkg.conf).'
	        : '(this port opted out of mp\'s own manifest-cleanup safety net with pkg_manifest_cleanup="no", '
	          . 'so a missing uninstall target here really does leave every file behind; '
	          . 'mp new can write an explicit file list instead).'));
}
 
# high: install phase creates a bin/ symlink or copies a file into
# $MP_PREFIX/bin, but that same basename never appears anywhere in the
# remove phase -- so mp remove leaves it behind (a dangling symlink, or a
# straight-up orphaned file). found in languages/ruby this session: 6
# "ln -sf" targets in add.sh, zero of them in del.sh.
sub check_missing_symlink_cleanup {
	my ($pc) = @_;
	my $install = _strip_comments($pc->{_phases}{install} || '');
	my $remove  = _strip_comments($pc->{_phases}{remove}  || '');
	my %targets;
	while($install =~ /\b(?:ln\s+-sf|cp|install)\b[^\n]*\$MP_PREFIX\/bin\/(\S+)/g)
	{
		(my $name = $1) =~ s/["']$//;
		# a computed name ($f.new, "$name", ...) -- typically a loop
		# variable, not a literal basename -- can't be reliably
		# cross-checked against remove's own text this way, so skip it
		# rather than flag a false mismatch (meta/mp's own install loop,
		# "cp \"$f\" \"$MP_PREFIX/bin/$f.new\"", is exactly this shape).
		next if $name =~ /\$/;
		# the atomic "cp X $MP_PREFIX/bin/Y.new; mv -f $MP_PREFIX/bin/Y.new
		# $MP_PREFIX/bin/Y" idiom (meta/mp's own bootstrap does this) -- the
		# .new path is a transient rename target, immediately renamed away
		# to the real (already-tracked, or intentionally never removed)
		# name on the very next line, not a second file needing its own
		# cleanup entry.
		next if $name =~ /\.new$/ && $install =~ /\bmv\s+(?:-\S+\s+)*\S*\Q$name\E\s/;
		$targets{$name} = 1;
	}
	my @missing = grep { $remove !~ /\Q$_\E/ } sort keys %targets;
	return () unless @missing;
	return _finding('high',
	    'install phase creates $MP_PREFIX/bin/{' . join(',', @missing) . '}, but remove never '
	    . 'mentions ' . (@missing == 1 ? 'it' : 'them') . ' -- left behind on every mp remove.');
}
 
# low: a full-history "git clone" (no --depth) of a repo known to have a
# genuinely enormous history. this tree's OWN convention is full-history
# clones almost everywhere on purpose (confirmed by static audit this
# session), so this only fires for the specific short list of repos
# where that convention gets expensive rather than every git-based port.
sub check_no_depth_huge_repo {
	my ($pc) = @_;
	my $text = _strip_comments(($pc->{_phases}{fetch} || '') . ($pc->{_phases}{install} || ''));
	return () unless $text =~ /\bgit\s+clone\s+(\S+)/;
	my $url = $1;
	return () if $text =~ /\bgit\s+clone\s+--depth\b/;
	for my $huge (@KNOWN_HUGE_REPOS)
	{
		return _finding('low', "git clone of $url has no --depth, and this repo's history is huge")
		    if lc($url) =~ /\Q\L$huge\E/;
	}
	return ();
}
 
# low: a bare "make" with no explicit CFLAGS=/LDFLAGS= on the same line
# (or exported earlier), and no ./configure step before it -- mpx always
# exports CFLAGS/LDFLAGS as real env vars (the -I/-L/rpath a dependency
# needs), but a plain "make" only inherits them from the environment,
# which a Makefile with its own hardcoded "CFLAGS = ..." (a bare "="
# assignment, extremely common upstream) silently OVERRIDES -- make's own
# variable-precedence rule, not a bug in mpx. explicitly passing them on
# the make command line always wins regardless.
#
# genuinely low severity, not just unconfirmed caution: real devtest
# runs this session found the SUGGESTED fix itself breaks close to a
# quarter of the ports it was tried against. "make CFLAGS=..." doesn't
# merge with a project's own Makefile CFLAGS (even one that builds it
# with "+="), it REPLACES it outright, dropping whatever that project's
# own build genuinely needed -- dvtm's own -DVERSION define (a straight
# compile error once gone), giflib's own -fPIC (its .so link fails
# without it), micropython's mpy-cross sub-build, and zstd all broke
# this exact way; sl/tree/sinit/dinit/figlet/lz4/cowsay/luajit/quickjs/
# janet/pforth/samurai did not. there is no reliable way to tell which
# case a given port is without actually building it both ways -- fix
# this per port, verified with "mp devtest", never as a blind sweep.
sub check_cflags_not_forwarded {
	my ($pc) = @_;
	my $install = _build_text($pc);
	# configure/cmake/meson all capture CFLAGS/LDFLAGS from the
	# environment into their OWN generated build files at THAT step, not
	# at make-time -- a bare "make" afterward is fine regardless (this
	# check is specifically about a build with no such capture step at
	# all, only a raw Makefile make itself already has no memory of what
	# the environment looked like when it runs).
	# case-insensitive: OpenSSL and perl5 both use "./Configure" (capital
	# C, their own perl-based configure equivalent, still a real capture
	# step the same reasoning above applies to) -- a case-sensitive match
	# missed both, flagging two ports that already explicitly forward
	# CFLAGS (perl5's own -Dccflags=, openssl's own Configure) as if they
	# didn't. no trailing \b on cmake: neovim's own "make
	# CMAKE_BUILD_TYPE=... CMAKE_INSTALL_PREFIX=..." (a real cmake build,
	# just invoked via a wrapping "make" target) has no word boundary
	# between "cmake" and the "_" that follows it, the identical
	# underscore-is-a-word-char gap CFLAGS= just got fixed for below.
	return () if $install =~ /\.\/configure\b|\bcmake|\bmeson\b/i;
	return () unless $install =~ /^\s*make\b[^\n]*$/m;
	# CFLAGS mentioned ANYWHERE in the phase (a separate compile-only make
	# call, an export line, ...) is enough signal the author already
	# forwards it somewhere -- checking per-line instead flags an
	# install-only "make install" line that never needed CFLAGS at all.
	# no \b before CFLAGS: a project's own build-specific override var
	# (bmake's EXTRA_CFLAGS=, a common HOST_EXTRACFLAGS=/MYCFLAGS=-style
	# convention) still contains the literal substring and is exactly as
	# much a "the author already thought about this" signal as a bare
	# CFLAGS= would be -- a leading \b only demanded an exact, unprefixed
	# match, missing bmake's own genuinely-already-handled case.
	return () if $install =~ /CFLAGS=/;
	return _finding('low',
	    'bare "make" with no CFLAGS="$CFLAGS" LDFLAGS="$LDFLAGS" and no ./configure step -- '
	    . 'if upstream\'s Makefile hardcodes its own CFLAGS with a bare "=", a dependency\'s '
	    . '-I/-L/rpath flags mpx injects via the environment could be silently dropped.');
}
 
my @CHECKS = (
	\&check_cmake_uninstall,
	\&check_missing_tool_deps,
	\&check_stale_pkg_ver_git,
	\&check_del_cd_mismatch,
	\&check_uninstall_target_risk,
	\&check_missing_symlink_cleanup,
	\&check_no_depth_huge_repo,
	\&check_cflags_not_forwarded,
);
 
# runs every check against one already-resolved port ($pc from
# resolve_port/try_pkgconf, $canon its slot-qualified name) and returns
# the combined finding list, most-relevant order (the order @CHECKS is
# in, which is roughly "most confirmed / most severe first").
sub lint_port {
	my ($pc, $canon) = @_;
	my @out;
	push @out, $_->($pc, $canon) for @CHECKS;
	return @out;
}
 
1;