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
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
cmake_minimum_required(VERSION 3.15)
project(mikktspace LANGUAGES C)

add_library(mikktspace ${MIKKTSPACE_SRC_DIR}/mikktspace.c)
target_include_directories(mikktspace PUBLIC ${MIKKTSPACE_SRC_DIR})
set_target_properties(mikktspace PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON)

include(CheckFunctionExists)
check_function_exists(pow HAVE_MATH_SYSTEM)
if(NOT HAVE_MATH_SYSTEM)
target_link_libraries(mikktspace PRIVATE m)
endif()

include(GNUInstallDirs)
install(TARGETS mikktspace
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
)
install(FILES ${MIKKTSPACE_SRC_DIR}/mikktspace.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import (
"os"
"path/filepath"
"slices"
"strings"
)

const consumerSource = `#include <mikktspace.h>
#include <stdlib.h>

static int GetNumFaces(const SMikkTSpaceContext *pContext)
{
return 0;
}

int main()
{
SMikkTSpaceInterface sInterface = {NULL};
sInterface.m_getNumFaces = GetNumFaces;

SMikkTSpaceContext sContext = {NULL};
sContext.m_pInterface = &sInterface;

genTangSpaceDefault(&sContext);

return 0;
}
`

id "mmikk/MikkTSpace"

fromVer "3e895b49d05ea07e4c2133156cfa94369e19e409"

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
}

onBuild ctx => {
installDir := ctx.outputDir
cmakeLists := ctx.Proj.readFile("3e895b49d05ea07e4c2133156cfa94369e19e409/CMakeLists.txt")!
os.writeFile(filepath.join(ctx.SourceDir, "CMakeLists.txt"), cmakeLists, 0o644)!

shared := target.options["shared"][0] == "ON"
fPIC := target.options["fPIC"][0] == "ON"
Comment on lines +58 to +59

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] Unchecked [0] index on target.options may crash

target.options["shared"][0] / target.options["fPIC"][0] index element 0 of a slice that may be empty. filter only rejects unknown option names and non-ON/OFF values — it never guarantees shared/fPIC are present, so a target with those options absent passes the filter and then triggers an index-out-of-range here (and again at line 91 in onTest).

The recp/cglm formula avoids this by using slices.contains, which is safe on an empty/absent slice and honors the default:

shared := slices.contains(target.options["shared"], "ON")
fPIC := slices.contains(target.options["fPIC"], "ON")

c := cmake.new(ctx.SourceDir, filepath.join(ctx.SourceDir, "_build"), installDir)
c.define "MIKKTSPACE_SRC_DIR", ctx.SourceDir
c.defineBool "BUILD_SHARED_LIBS", shared
c.defineBool "CMAKE_POSITION_INDEPENDENT_CODE", fPIC
c.configure
c.build
c.install

licenseDir := filepath.join(installDir, "licenses")
os.mkdirAll(licenseDir, 0o755)!
os.writeFile(filepath.join(licenseDir, "LICENSE"), os.readFile(filepath.join(ctx.SourceDir, "mikktspace.h"))!, 0o644)!

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] LICENSE file is the full mikktspace.h header

This writes the entire mikktspace.h (C source included) as licenses/LICENSE. This is accurate — upstream MikkTSpace ships no standalone LICENSE file and the full zlib license text lives in the header's top comment — but a LICENSE file containing C source reads like a mistake to anyone auditing licenses. Either extract just the license comment block, or add a one-line comment noting "upstream ships no LICENSE file; the zlib license text lives in mikktspace.h." (Note the other three formulas don't capture a license at all.)


flags := []string{
"-I" + filepath.join(installDir, "include"),
"-L" + filepath.join(installDir, "lib"),
"-lmikktspace",
}
if !shared && (slices.contains(target.require["os"], "linux") || slices.contains(target.require["os"], "freebsd")) {
flags <- "-lm"
}
ctx.setMetadata strings.join(flags, " ")
}

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

sourcePath := filepath.join(testDir, "consumer.c")
os.writeFile(sourcePath, []byte(consumerSource), 0o644)!
binary := filepath.join(testDir, "consumer")
shared := target.options["shared"][0] == "ON"

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] onTest re-derives -lm from options; duplicates onBuild logic

onTest recomputes shared and the -lm linkage from target.options/target.require (lines 91, 99-101), duplicating the identical logic in onBuild (lines 77-79) and the CMake check_function_exists(pow ...) probe. There are now three places that must agree on whether libm is needed. onBuild already encodes the correct consumer flags via ctx.setMetadata; consider having onTest consume that instead of re-deriving it (this also matches how cglm/json-c derive test flags from the installed output). Same [0] indexing concern as above applies at line 91.

args := []string{
"-I" + filepath.join(installDir, "include"),
sourcePath,
"-L" + filepath.join(installDir, "lib"),
"-lmikktspace",
"-o", binary,
}
if !shared && (slices.contains(target.require["os"], "linux") || slices.contains(target.require["os"], "freebsd")) {
args <- "-lm"
}
exec "cc", args...
lastErr!

if shared {
os.setenv("LD_LIBRARY_PATH", filepath.join(installDir, "lib"))!

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] setenv overwrites LD_LIBRARY_PATH instead of prepending

os.setenv replaces LD_LIBRARY_PATH/DYLD_LIBRARY_PATH with a single path rather than prepending the install lib dir to the existing value. If the shared consumer ever needs a library reachable only via a pre-existing LD_LIBRARY_PATH, the test could fail to locate it. Minor/functional (not a security issue — the written path is trusted); worth prepending installDir/lib + ":" + os.getenv("LD_LIBRARY_PATH") to be safe.

os.setenv("DYLD_LIBRARY_PATH", filepath.join(installDir, "lib"))!
}
exec binary
lastErr!
}
4 changes: 4 additions & 0 deletions mmikk/MikkTSpace/versions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"path": "mmikk/MikkTSpace",

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] versions.json uses 2-space indent; siblings use tabs

recp/cglm/versions.json, json-c/json-c/versions.json, and madler/zlib/versions.json are all tab-indented; this file uses 2 spaces. Re-indent with tabs for consistency.

"deps": {}
}
Loading