Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions PhilipLudington/poshlib/v1.3.002/poshlib_llar.gox
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import (
"os"
"path/filepath"
"runtime"
"slices"
"strings"
)

const cmakeLists = `cmake_minimum_required(VERSION 3.12)
project(poshlib C)

if(WIN32 AND BUILD_SHARED_LIBS)
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()

file(GLOB SRCS_FILES $${POSH_SRC_DIR}/*.c)
file(GLOB HDRS_FILES $${POSH_SRC_DIR}/*.h)

add_library(posh $${SRCS_FILES})
target_include_directories(posh PUBLIC $${POSH_SRC_DIR})

include(GNUInstallDirs)
install(TARGETS posh
LIBRARY DESTINATION $${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION $${CMAKE_INSTALL_LIBDIR}
RUNTIME DESTINATION $${CMAKE_INSTALL_BINDIR})
install(FILES $${HDRS_FILES} DESTINATION $${CMAKE_INSTALL_INCLUDEDIR})
`

const consumerSource = `#include <stdio.h>

#include "posh.h"

int main(void)
{
printf("%s", POSH_GetArchString());
return 0;
}
`

id "PhilipLudington/poshlib"

fromVer "v1.3.002"

defaults {
"shared": "OFF",
"fPIC": "ON",
}

filter => {
for name, values in target.options {
if name != "shared" && name != "fPIC" {
return false
}
for value in values {
if value != "ON" && value != "OFF" {
return false
}
}
}
return true
}
Comment on lines +50 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] filter rejects targets that carry any option beyond shared/fPIC

The filter iterates every key in target.options and returns false for any name that isn't shared or fPIC. If the loader ever surfaces another standard key (e.g. os, arch, build type) in target.options, an otherwise valid selection is silently rejected as "no matching target" rather than failing with a clear cause.

The write-formula semantics guidance is explicit here: "Reject a selection only when the selected upstream revision proves it is unsupported" and "Defaults choose option values; they do not by themselves define every legal value." The sibling recp/cglm recipe follows the safer pattern — it validates only the values of the option(s) it cares about and never enumerates/rejects unknown keys.

Consider validating only the values of shared/fPIC instead of rejecting unrecognized option names.

Comment on lines +45 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Option/default semantics are undocumented vs. sibling recipes

The defaults (shared=OFF, fPIC=ON) and the fPIC-only-when-static decision have no explanatory comments. The exemplar recipes (recp/cglm, json-c) document why each option exists, that defaults mirror the CCI default_options (shared=False, fPIC=True), and why fPIC is meaningless for a shared build. Adding brief notes here — including that the onTest flags are rebuilt from installDir specifically so the test works on a cache hit — would match repo convention and help future maintainers.


onBuild ctx => {
installDir := ctx.outputDir

// The CCI recipe adds arm64 compiler spellings to the upstream ARM check.
headerPath := filepath.join(ctx.SourceDir, "posh.h")
header := string(os.readFile(headerPath)!)
header = strings.replace(header, "defined _ARM", "defined _ARM || defined __arm64 || defined __arm64__ || defined __aarch64__", 1)
os.writeFile(headerPath, []byte(header), 0644)!
Comment on lines +68 to +71

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Header patch is not idempotent on a reused source tree

The replacement anchor "defined _ARM" is a prefix of the replacement text ("defined _ARM || defined __arm64 ..."). This edit mutates the pristine upstream posh.h in place. If onBuild ever runs again against an already-patched source tree (a cache-miss rebuild reusing the tree, e.g. across option combinations), strings.replace(..., 1) re-matches the already-injected defined _ARM and expands it a second time.

A guard makes it safe and skips the write on re-runs, e.g. if !strings.contains(header, "__aarch64__") { ... }. As a bonus, strings.replace silently no-ops if upstream ever changes the anchor, so the arm64 fix would vanish without error — asserting the content actually changed would surface that loudly. Acceptable given the pinned tag, but worth hardening.


cmakeDir := filepath.join(ctx.SourceDir, "_llar_cmake")
os.mkdirAll(cmakeDir, 0755)!
os.writeFile(filepath.join(cmakeDir, "CMakeLists.txt"), []byte(cmakeLists), 0644)!

shared := slices.contains(target.options["shared"], "ON")
fPIC := slices.contains(target.options["fPIC"], "ON")
c := cmake.new(cmakeDir, filepath.join(ctx.SourceDir, "_build"), installDir)
c.define "POSH_SRC_DIR", ctx.SourceDir
c.define "CMAKE_INSTALL_LIBDIR", "lib"
c.defineBool "BUILD_SHARED_LIBS", shared
if !shared {
c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", fPIC
}
c.configure
c.build
c.install

licenseDir := filepath.join(installDir, "licenses")
os.mkdirAll(licenseDir, 0755)!
os.writeFile(filepath.join(licenseDir, "LICENSE"), os.readFile(filepath.join(ctx.SourceDir, "LICENSE"))!, 0644)!

osName := runtime.GOOS
osValues := target.require["os"]
if osValues.len > 0 {
osName = osValues[0]
}
metadata := "-I" + filepath.join(installDir, "include") + " -L" + filepath.join(installDir, "lib") + " -lposh"
if shared && osName == "windows" {
metadata = "-DPOSH_DLL " + metadata
}
Comment on lines +99 to +102

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Metadata is hand-built rather than derived from an installed interface

metadata reconstructs -I/-L/-lposh (plus -DPOSH_DLL) by hand. The semantics guidance prefers deriving metadata from a valid installed package interface (pkg-config/CMake package files) and cautions against copying a package manager's package_info without comparing it to the actual installed result. poshlib's CMake install here doesn't emit a .pc/config, so hand-built flags may be the only option — but a short comment noting that, and that -DPOSH_DLL mirrors the CCI package_info for Windows+shared, would document the choice the way the cglm/json-c recipes do.

ctx.setMetadata metadata
}

onTest ctx => {
installDir := ctx.outputDir
testDir := filepath.join(ctx.SourceDir, "_llar_consumer")
os.mkdirAll(testDir, 0755)!

sourcePath := filepath.join(testDir, "consumer.c")
os.writeFile(sourcePath, []byte(consumerSource), 0644)!

shared := slices.contains(target.options["shared"], "ON")
osName := runtime.GOOS
osValues := target.require["os"]
if osValues.len > 0 {
osName = osValues[0]
}
binary := filepath.join(testDir, "consumer")
args := []string{
"-I" + filepath.join(installDir, "include"),
sourcePath,
"-L" + filepath.join(installDir, "lib"),
"-lposh",
"-o", binary,
}
if shared && osName == "windows" {
args = append([]string{"-DPOSH_DLL"}, args...)
}
exec "cc", args...
lastErr!

if shared {
os.setenv("LD_LIBRARY_PATH", filepath.join(installDir, "lib"))!
os.setenv("DYLD_LIBRARY_PATH", filepath.join(installDir, "lib"))!
}
Comment on lines +134 to +137

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Shared-build test: overwrites loader env and misses Windows DLL resolution

Two issues in the shared-library test path:

  1. os.setenv("LD_LIBRARY_PATH", ...) / DYLD_LIBRARY_PATH overwrite any existing value rather than prepending, clobbering loader paths the test process may need. Prefer installDir/lib + ":" + os.getenv("LD_LIBRARY_PATH").

  2. On Windows — the one platform special-cased with -DPOSH_DLL — DLL resolution is governed by PATH, not LD_LIBRARY_PATH/DYLD_LIBRARY_PATH. So for the exact configuration that most needs runtime-path help (shared build on Windows), this block does nothing, and exec binary may fail to locate posh.dll at run time.

exec binary
lastErr!
}
4 changes: 4 additions & 0 deletions PhilipLudington/poshlib/versions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"path": "PhilipLudington/poshlib",
"deps": {}
}
Loading