fix(freebsd): treat ${LOCALBASE}/bin as pkg-managed; resolve self via sysctl - #1777
Conversation
… sysctl Two FreeBSD gaps remain after the runtime rework of install/update (DeusData#1566) and the KERN_PROC_PATHNAME self-path fix landed on main. 1. cli_external_manager_name() matches only positive evidence (mise, Homebrew, nix, asdf, cargo). ${LOCALBASE}/bin was not among them, so a ports/pkg install of cbm was still copied into ~/.local/bin and a shell rc was edited -- exactly what externally-managed detection is meant to prevent. Recognise ${LOCALBASE}/bin and /sbin on FreeBSD so install skips the copy and update defers to `pkg upgrade`. LOCALBASE is configurable, so the FreeBSD port passes its real PREFIX via -DCBM_PKG_PREFIX and we honour it, falling back to the documented /usr/local default for a plain upstream build. --force-binary remains the escape hatch for anyone self-managing that prefix. 2. resolve_self_executable() in the HTTP server still read /proc/self/exe, which FreeBSD does not mount by default. Add a KERN_PROC_PATHNAME sysctl branch, mirroring cbm_detect_self_path(). Test updated: on FreeBSD ${LOCALBASE}/bin/cbm is now externally managed; a non-LOCALBASE prefix (/opt/local) is not. Signed-off-by: Olivier Cochard-Labbé <olivier@cochard.me>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
One scoping note. This PR implements exactly the two items from the #1331 closing comment (prefix detection + the sysctl After detection correctly skips the binary, agent configs still resolve through The FreeBSD port currently fixes this locally with two extra hunks: retarget Want me to fold that into this PR, or keep it as a separate follow-up? Happy to do either. |
|
Answering the two questions you raised in #1331 directly, since you asked me to own them rather than guess: "Does a platform-conditional "Is
CI note: the one red check ( |
|
Approved on merit. I have rebased it — the branch was 226 commits behind, so the red you are looking at is from 21 August and means nothing now. This is the follow-up we asked for when closing #1331, and it answers both gaps precisely. The externally-managed detection was the more important of the two. Three things I want to name because they are the difference between a patch and a correct patch: Honouring a configurable Anchoring with Leaving The One footgun worth documentingstrncmp(self_path, CBM_PKG_PREFIX "/bin/", sizeof(CBM_PKG_PREFIX "/bin/") - 1)relies on Minor: the description says " The honest limitWe have no FreeBSD CI leg, so nothing in our matrix exercises either branch. Your poudriere jail is the only verification this change will ever get from us, and I am taking it on that basis. What makes that acceptable rather than reckless is that every line is behind Add the |
|
Merged as I want to correct something I said. I described the For the record, so the next packager finds it: Two things about the change I want on the record because they are the reason it was easy to take: Honouring a configurable Anchoring with VerificationYour green was 34/34 but 31 commits behind, and #1802 had since touched The honest limit stands and is worth repeating on the record: we have no FreeBSD CI, so your poudriere-jail validation is the only platform evidence this will ever get from us. What makes that acceptable rather than reckless is that every line is behind |
Follow-up to the closure of #1331. That PR was superseded by the runtime rework of install/update (#1566) and the
KERN_PROC_PATHNAMEself-path fix that landed onmain. Two FreeBSD gaps remained, which @DeusData identified in the closing comment:1.
cli_external_manager_name()never recognised FreeBSD's package prefixIt matches only positive evidence (mise, Homebrew, nix, asdf, cargo).
${LOCALBASE}/bin(default/usr/local/bin) was not among them, so a ports/pkg install of cbm was still copied into~/.local/binand a shell rc was edited — exactly what externally-managed detection is meant to prevent.Now
${LOCALBASE}/binand/sbinare recognised on FreeBSD (#ifdef __FreeBSD__), soinstallskips the copy andupdaterefuses and points atpkg upgrade codebase-memory-mcp.LOCALBASEis configurable, so the FreeBSD port passes its realPREFIXvia-DCBM_PKG_PREFIXand we honour it, falling back to the documented/usr/localdefault for a plain upstream build.--force-binaryremains the escape hatch for anyone self-managing that prefix.2.
resolve_self_executable()in the HTTP server still read/proc/self/exeFreeBSD does not mount
/procby default. Added aKERN_PROC_PATHNAMEsysctl branch, mirroringcbm_detect_self_path()incli.c.Testing
cli_external_manager_detection_needs_positive_evidence_issue1566updated: on FreeBSD${LOCALBASE}/bin/cbmis now externally managed; a non-LOCALBASEprefix (/opt/local) is not.Built and validated through the FreeBSD port (
devel/codebase-memory-mcp) in a clean poudriere jail.