tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Removing Bitrig support - code review



Am 28.09.2022 um 20:51 schrieb Charlotte Koch:
Hi!

In pkgsrc-users@ I asked if we should bother supporing Bitrig in pkgsrc
going forward -- and people seemed to agree that there's no point
really.

https://mail-index.netbsd.org/pkgsrc-users/2022/09/21/msg036207.html

So I prepared this diff in the meanwhile, but didn't commit it because
the freeze was on, and also I wanted to make sure I'm on the right
track, and verify that I'm not missing anything:

https://github.com/dressupgeekout/pkgsrc/commit/86125240a5725392edf38da7b4b7f35eb38808dd

When I first saw the patch, I was surprised that it deleted only 191
lines. This would mean that it would only take these 191 lines in
reverse to add support to a new operating system.

What do we think? I'm guessing this change should be documented
somewhere important, too.

As this is a change to the pkgsrc infrastructure, it should be
documented in doc/CHANGES-2022, in a line like this one from 2020:

	mk/subst.mk: removed SUBST_NOOP_OK [rillig 2020-10-06]

I did a case-insensitive full-text search for bitrig and found:

* one mention in the pkgsrc guide

* pkglint4, easy to patch, I'll take care of that

* pkglint, I'll take care of that

* several packages that refer to Bitrig in patches or their Makefiles.
It's easy to clean them up in a follow-up patch. As soon as the Bitrig
files in mk/ are removed, pkglint will complain about the unknown
operating system in makefiles, but not in patch comments.

In summary, the patch looks complete, even though it is rather small.
Since there is no other operating system with a similar name, chances
are high that there are no makefile conditions like '${OPSYS:M*rig}'. If
the operating system name had been LegacyBSD instead, it might be worth
checking each condition of the form '${OPSYS:M*BSD}' to see whether it
is still needed.

Looks good to me.

Roland



Home | Main Index | Thread Index | Old Index