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
143 changes: 143 additions & 0 deletions tjdistler/iqa/0559a8ff2acd9746864975834d35be6892cc9801/iqa_llar.gox
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import (
"os"
"path/filepath"
"runtime"
"slices"
"strings"
)

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

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

file(GLOB IQA_SOURCES $${IQA_SRC_DIR}/source/*.c)
file(GLOB IQA_HEADERS $${IQA_SRC_DIR}/include/*.h)

add_library(iqa $${IQA_SOURCES})
target_include_directories(iqa PUBLIC $${IQA_SRC_DIR}/include)

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

const consumerSource = `#include "math_utils.h"

int main(void) {
_max(0, 1);
return 0;
}
`

id "tjdistler/iqa"

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] Filename iqa_llar.gox differs from PascalCase siblings

Sibling formulas use a capitalized project prefix: Cglm_llar.gox, Jsonc_llar.gox, Zlib_llar.gox. This file is lowercase iqa_llar.gox. If the loader is case-sensitive about the prefix, rename to Iqa_llar.gox for consistency; otherwise confirm lowercase is accepted.


fromVer "0559a8ff2acd9746864975834d35be6892cc9801"

defaults {
"shared": "OFF",
"fPIC": "ON",
}
Comment on lines +42 to +45

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] Missing build-contract / options documentation vs sibling formulas

The sibling formulas (Cglm_llar.gox, Jsonc_llar.gox) open with a comment describing the build contract, options, and non-obvious choices. This formula has none. Worth documenting, since several things here are reviewer-relevant and non-obvious: (1) upstream ships no CMakeLists so this formula injects one and globs source/*.c / include/*.h; (2) the shared/fPIC options and why fPIC only applies to static builds (the if !shared guard at lines 72-74); (3) that the license is scraped from the iqa.h header comment. A short header comment block covering these would bring it in line with the repo convention.


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
}

onBuild ctx => {
installDir := ctx.outputDir
projectDir := filepath.join(ctx.SourceDir, "_llar_cmake")
os.mkdirAll(projectDir, 0o755)!
os.writeFile(filepath.join(projectDir, "CMakeLists.txt"), []byte(cmakeLists), 0o644)!

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

license := string(os.readFile(filepath.join(ctx.SourceDir, "include", "iqa.h"))!)
licenseEnd := strings.index(license, "*/")
license = strings.trimSpace(license[:licenseEnd])
Comment on lines +80 to +81

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] License parse can slice with a negative index when */ is absent

strings.index(license, "*/") returns -1 if iqa.h has no */ sequence, and license[:licenseEnd] then becomes license[:-1] — a negative-bound slice that panics (or silently corrupts the LICENSE). Unlike the sibling formulas (cglm/json-c), which derive license/flags from stable build artifacts, this parses raw header text without a guard. Add a check: if licenseEnd < 0, fail with a clear error rather than slicing.

Minor, in the same block: the * prefix strip on line 83 only matches the exact \n * sequence, so blank comment lines written as \n * (no trailing space) keep their prefix, and after removing /* the result begins with a leading blank line. A final strings.trimSpace would clean that up.

license = strings.replace(license, "/*", "", 1)
license = strings.replace(license, "\n * ", "\n", -1)
licenseDir := filepath.join(installDir, "licenses")
os.mkdirAll(licenseDir, 0o755)!
os.writeFile(filepath.join(licenseDir, "LICENSE"), []byte(license), 0o644)!

osName := runtime.GOOS
osValues := target.require["os"]
if osValues.len > 0 {
osName = osValues[0]
}
flags := []string{
"-I" + filepath.join(installDir, "include"),
"-L" + filepath.join(installDir, "lib"),
"-liqa",
}
if osName == "linux" {
flags <- "-lm"
}
Comment on lines +98 to +100

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] -lm gated on Linux only — omitted for macOS/other Unix

-lm is added to the published metadata only when osName == "linux". iqa uses <math.h>/math routines, so a static consumer on macOS or other Unix that needs libm will get metadata missing -lm. Compare cglm, which documents -lm "on Linux/FreeBSD". Consider broadening the condition to non-Windows (or the specific Unix targets you support) — and the same gate at line 125 in onTest should match. If Linux is genuinely the only supported target, a short comment saying so would make the choice clear.

ctx.setMetadata strings.join(flags, " ")
}

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

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

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

if shared {
if osName == "windows" {
path := os.getenv("PATH")
os.setenv("PATH", filepath.join(installDir, "bin")+";"+path)!
} else {
os.setenv("LD_LIBRARY_PATH", filepath.join(installDir, "lib"))!
os.setenv("DYLD_LIBRARY_PATH", filepath.join(installDir, "lib"))!
Comment on lines +137 to +138

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] LD_LIBRARY_PATH/DYLD_LIBRARY_PATH overwritten, not prepended

The Windows branch prepends to the existing PATH, but the non-Windows branch replaces LD_LIBRARY_PATH/DYLD_LIBRARY_PATH wholesale, discarding any loader paths already in the test environment. This is inconsistent with the Windows handling in the same block and can drop paths the runtime needs. Prepend instead, e.g. filepath.join(installDir, "lib") + ":" + os.getenv("LD_LIBRARY_PATH"), to match the Windows branch.

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