diff --git a/.claude/skills/dev-cmake/SKILL.md b/.claude/skills/dev-cmake/SKILL.md new file mode 100644 index 0000000..1d8c98b --- /dev/null +++ b/.claude/skills/dev-cmake/SKILL.md @@ -0,0 +1,180 @@ +--- +name: dev-cmake +description: Write CMake and Make files like a principal engineer with decades of experience. Use when creating, refactoring, reviewing, or debugging CMakeLists.txt, Makefiles, or build system configurations for C/C++ projects. Produces elegant, minimal, modern build files that reflect deep understanding of build systems. +--- + +# CMake & Make Expert + +Write build files that are elegant because the understanding is deep. Every line should have a reason. Simplicity comes from mastery, not shortcuts. + +## Core Philosophy + +**Targets are everything.** Modern CMake is about targets and properties, not variables and directories. Think of targets as objects with member functions and properties. + +**Explicit over implicit.** Always specify `PRIVATE`, `PUBLIC`, or `INTERFACE`. Never rely on inherited directory-level settings. + +**Minimal surface area.** Expose only what consumers need. Default to `PRIVATE`; use `PUBLIC` only when downstream targets genuinely require it. + +## CMake: The Principal Engineer Approach + +### Project Structure +```cmake +cmake_minimum_required(VERSION 3.16) +project(MyProject VERSION 1.0.0 LANGUAGES CXX) + +# Set standards at target level, not globally +# Use compile features, not flags +``` + +### Target Definition Pattern +```cmake +add_library(mylib) +add_library(MyProject::mylib ALIAS mylib) + +target_sources(mylib + PRIVATE + src/impl.cpp + PUBLIC + FILE_SET HEADERS + BASE_DIRS include + FILES include/mylib/api.h +) + +target_compile_features(mylib PUBLIC cxx_std_17) + +target_include_directories(mylib + PUBLIC + $ + $ +) +``` + +### What to Never Do +- `include_directories()` — use `target_include_directories()` +- `link_directories()` — use full paths or targets +- `add_definitions()` — use `target_compile_definitions()` +- `link_libraries()` — use `target_link_libraries()` +- `CMAKE_CXX_FLAGS` manipulation — use `target_compile_options()` or features +- `file(GLOB)` for sources — list files explicitly +- Bare library names in `target_link_libraries()` — use namespaced targets + +### Dependency Handling + +For find_package dependencies: +```cmake +find_package(Boost 1.70 REQUIRED COMPONENTS filesystem) +target_link_libraries(mylib PRIVATE Boost::filesystem) +``` + +For FetchContent (prefer over ExternalProject for CMake deps): +```cmake +include(FetchContent) +FetchContent_Declare(fmt + GIT_REPOSITORY https://github.com/fmtlib/fmt.git + GIT_TAG 10.1.0 +) +FetchContent_MakeAvailable(fmt) +target_link_libraries(mylib PRIVATE fmt::fmt) +``` + +### Generator Expressions + +Use for build/install path differences: +```cmake +target_include_directories(mylib PUBLIC + $ + $ +) +``` + +Use for conditional compilation: +```cmake +target_compile_definitions(mylib PRIVATE + $<$:DEBUG_MODE> +) +``` + +## Make: The Principal Engineer Approach + +### Essential Structure +```makefile +# Immediately expanded defaults +CC ?= gcc +CXX ?= g++ +CFLAGS ?= -Wall -Wextra -pedantic +LDFLAGS ?= + +# Preserve user-provided flags +CFLAGS += -MMD -MP + +# Automatic dependency tracking +SRCS := $(wildcard src/*.c) +OBJS := $(SRCS:src/%.c=build/%.o) +DEPS := $(OBJS:.o=.d) + +.PHONY: all clean + +all: bin/program + +bin/program: $(OBJS) | bin + $(CC) $(LDFLAGS) -o $@ $^ + +build/%.o: src/%.c | build + $(CC) $(CFLAGS) -c -o $@ $< + +bin build: + mkdir -p $@ + +clean: + rm -rf build bin + +-include $(DEPS) +``` + +### Pattern Rules — The Elegant Way +```makefile +# Single pattern rule replaces N explicit rules +%.o: %.c + $(CC) $(CFLAGS) $(CPPFLAGS) -c -o $@ $< +``` + +### Automatic Variables (memorize these) +- `$@` — target +- `$<` — first prerequisite +- `$^` — all prerequisites (no duplicates) +- `$+` — all prerequisites (with duplicates, for libs) +- `$*` — stem (matched by %) + +### What Makes Makefiles Elegant +1. Order-only prerequisites (`| dir`) for directory creation +2. `-include` for optional dependency files +3. `?=` for overridable defaults, `+=` to append +4. `.PHONY` for non-file targets +5. `.DELETE_ON_ERROR` to clean failed builds +6. Consistent variable naming (UPPERCASE for user-facing) + +## Reference Files + +- **references/cmake-patterns.md** — Complete modern CMake patterns (library export, install, presets, toolchains) +- **references/make-patterns.md** — Advanced Make patterns (multi-directory, cross-compilation, dependencies) +- **references/antipatterns.md** — Common mistakes and their fixes + +## Quality Checklist + +Before finalizing any build file: + +### CMake +- [ ] Every target has a namespaced alias +- [ ] All `target_*` calls specify scope (`PRIVATE`/`PUBLIC`/`INTERFACE`) +- [ ] No directory-level commands (`include_directories`, etc.) +- [ ] Generator expressions for build/install differences +- [ ] Version requirements on `find_package` +- [ ] `cmake_minimum_required` reflects actual features used + +### Make +- [ ] All variables use `?=` or `+=` appropriately +- [ ] Automatic dependency generation (`-MMD -MP`) +- [ ] Pattern rules instead of repeated explicit rules +- [ ] `.PHONY` declared for non-file targets +- [ ] Clean target removes all generated files +- [ ] Build artifacts in separate directory from source diff --git a/.claude/skills/dev-cmake/assets/CMakeLists-library.txt b/.claude/skills/dev-cmake/assets/CMakeLists-library.txt new file mode 100644 index 0000000..e37778e --- /dev/null +++ b/.claude/skills/dev-cmake/assets/CMakeLists-library.txt @@ -0,0 +1,101 @@ +cmake_minimum_required(VERSION 3.16) +project(MyLibrary + VERSION 1.0.0 + DESCRIPTION "A well-structured C++ library" + LANGUAGES CXX +) + +# ============================================================================ +# Options +# ============================================================================ +option(MYLIB_BUILD_TESTS "Build unit tests" OFF) +option(MYLIB_BUILD_EXAMPLES "Build examples" OFF) + +# ============================================================================ +# Library Target +# ============================================================================ +add_library(mylib) +add_library(${PROJECT_NAME}::mylib ALIAS mylib) + +target_sources(mylib + PRIVATE + src/mylib.cpp + # PUBLIC headers via FILE_SET (CMake 3.23+) or target_include_directories +) + +target_compile_features(mylib PUBLIC cxx_std_17) + +target_include_directories(mylib + PUBLIC + $ + $ + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src +) + +# Uncomment and modify for dependencies: +# find_package(fmt REQUIRED) +# target_link_libraries(mylib PUBLIC fmt::fmt) + +target_compile_options(mylib + PRIVATE + $<$:-Wall -Wextra -Wpedantic> + $<$:/W4> +) + +set_target_properties(mylib PROPERTIES + VERSION ${PROJECT_VERSION} + SOVERSION ${PROJECT_VERSION_MAJOR} + CXX_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON +) + +# ============================================================================ +# Installation +# ============================================================================ +include(GNUInstallDirs) +include(CMakePackageConfigHelpers) + +install(TARGETS mylib + EXPORT ${PROJECT_NAME}Targets + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} + INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} +) + +install(DIRECTORY include/ + DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} +) + +install(EXPORT ${PROJECT_NAME}Targets + FILE ${PROJECT_NAME}Targets.cmake + NAMESPACE ${PROJECT_NAME}:: + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME} +) + +configure_package_config_file( + ${CMAKE_CURRENT_SOURCE_DIR}/cmake/${PROJECT_NAME}Config.cmake.in + ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Config.cmake + INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME} +) + +write_basic_package_version_file( + ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake + VERSION ${PROJECT_VERSION} + COMPATIBILITY SameMajorVersion +) + +install(FILES + ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Config.cmake + ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME} +) + +# ============================================================================ +# Testing +# ============================================================================ +if(MYLIB_BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() diff --git a/.claude/skills/dev-cmake/assets/Makefile-template b/.claude/skills/dev-cmake/assets/Makefile-template new file mode 100644 index 0000000..03b312d --- /dev/null +++ b/.claude/skills/dev-cmake/assets/Makefile-template @@ -0,0 +1,122 @@ +# ============================================================================ +# Project Configuration +# ============================================================================ +PROJECT := myproject +VERSION := 1.0.0 + +# ============================================================================ +# Directories +# ============================================================================ +SRCDIR := src +INCDIR := include +BUILDDIR := build +BINDIR := bin + +# ============================================================================ +# Toolchain (overridable) +# ============================================================================ +CC ?= gcc +CXX ?= g++ +AR ?= ar +LD = $(CXX) + +# ============================================================================ +# Flags (overridable, then extended) +# ============================================================================ +CFLAGS ?= -Wall -Wextra -pedantic +CXXFLAGS ?= -Wall -Wextra -pedantic +CPPFLAGS ?= +LDFLAGS ?= +LDLIBS ?= + +# Project additions (always applied) +CPPFLAGS += -I$(INCDIR) +CXXFLAGS += -std=c++17 -MMD -MP + +# ============================================================================ +# Build Configuration +# ============================================================================ +BUILD ?= release + +ifeq ($(BUILD),debug) + CXXFLAGS += -g -O0 -DDEBUG + BUILDDIR := $(BUILDDIR)/debug +else + CXXFLAGS += -O2 -DNDEBUG + BUILDDIR := $(BUILDDIR)/release +endif + +# ============================================================================ +# Sources and Objects +# ============================================================================ +SRCS := $(wildcard $(SRCDIR)/*.cpp) +OBJS := $(SRCS:$(SRCDIR)/%.cpp=$(BUILDDIR)/%.o) +DEPS := $(OBJS:.o=.d) + +# ============================================================================ +# Targets +# ============================================================================ +.PHONY: all clean install uninstall help debug release + +all: $(BINDIR)/$(PROJECT) + +debug: + $(MAKE) BUILD=debug + +release: + $(MAKE) BUILD=release + +# Link executable +$(BINDIR)/$(PROJECT): $(OBJS) | $(BINDIR) + $(LD) $(LDFLAGS) -o $@ $^ $(LDLIBS) + +# Compile sources +$(BUILDDIR)/%.o: $(SRCDIR)/%.cpp | $(BUILDDIR) + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< + +# Create directories +$(BUILDDIR) $(BINDIR): + mkdir -p $@ + +# Include auto-generated dependencies +-include $(DEPS) + +# ============================================================================ +# Installation +# ============================================================================ +PREFIX ?= /usr/local + +install: $(BINDIR)/$(PROJECT) + install -d $(DESTDIR)$(PREFIX)/bin + install -m 755 $(BINDIR)/$(PROJECT) $(DESTDIR)$(PREFIX)/bin/ + +uninstall: + $(RM) $(DESTDIR)$(PREFIX)/bin/$(PROJECT) + +# ============================================================================ +# Maintenance +# ============================================================================ +clean: + $(RM) -r $(BUILDDIR) $(BINDIR) + +help: + @echo "$(PROJECT) v$(VERSION)" + @echo "" + @echo "Targets:" + @echo " all Build $(PROJECT) (default)" + @echo " debug Build with debug symbols" + @echo " release Build optimized" + @echo " clean Remove build artifacts" + @echo " install Install to PREFIX" + @echo " uninstall Remove installed files" + @echo "" + @echo "Variables:" + @echo " BUILD=$(BUILD) (debug|release)" + @echo " PREFIX=$(PREFIX)" + @echo " CXX=$(CXX)" + +# ============================================================================ +# Safety +# ============================================================================ +.DELETE_ON_ERROR: +.SECONDARY: diff --git a/.claude/skills/dev-cmake/references/antipatterns.md b/.claude/skills/dev-cmake/references/antipatterns.md new file mode 100644 index 0000000..547e09a --- /dev/null +++ b/.claude/skills/dev-cmake/references/antipatterns.md @@ -0,0 +1,365 @@ +# Build System Antipatterns + +Common mistakes and how to fix them. These are patterns that make build files fragile, hard to maintain, or just incorrect. + +--- + +## CMake Antipatterns + +### 1. Directory-Level Commands + +❌ **Bad:** +```cmake +include_directories(${CMAKE_SOURCE_DIR}/include) +add_definitions(-DFOO) +link_libraries(pthread) +``` + +✅ **Good:** +```cmake +target_include_directories(mylib PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) +target_compile_definitions(mylib PRIVATE FOO) +target_link_libraries(mylib PRIVATE pthread) +``` + +**Why:** Directory commands affect all targets in that scope, creating hidden dependencies and making it impossible to reason about individual target requirements. + +--- + +### 2. Manipulating CMAKE_CXX_FLAGS + +❌ **Bad:** +```cmake +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -Wall") +``` + +✅ **Good:** +```cmake +target_compile_features(mylib PUBLIC cxx_std_17) +target_compile_options(mylib PRIVATE -Wall) +``` + +**Why:** `CMAKE_CXX_FLAGS` is global, affects all targets, and doesn't work correctly across different compilers. `-std=c++17` is GCC/Clang syntax; MSVC uses `/std:c++17`. + +--- + +### 3. Using file(GLOB) for Sources + +❌ **Bad:** +```cmake +file(GLOB SOURCES "src/*.cpp") +add_library(mylib ${SOURCES}) +``` + +✅ **Good:** +```cmake +add_library(mylib + src/file1.cpp + src/file2.cpp + src/file3.cpp +) +``` + +**Why:** CMake evaluates GLOB at configure time. Adding a new file doesn't trigger reconfiguration, so the build system doesn't see it. This causes "works on my machine" bugs. + +--- + +### 4. Missing Scope Specifiers + +❌ **Bad:** +```cmake +target_link_libraries(mylib fmt) +target_include_directories(mylib include) +``` + +✅ **Good:** +```cmake +target_link_libraries(mylib PRIVATE fmt::fmt) +target_include_directories(mylib PUBLIC include) +``` + +**Why:** Without `PRIVATE`/`PUBLIC`/`INTERFACE`, CMake uses legacy behavior. Be explicit about what propagates to dependents. + +--- + +### 5. Bare Library Names + +❌ **Bad:** +```cmake +target_link_libraries(myapp boost_filesystem) +``` + +✅ **Good:** +```cmake +find_package(Boost REQUIRED COMPONENTS filesystem) +target_link_libraries(myapp PRIVATE Boost::filesystem) +``` + +**Why:** Bare names don't carry include directories or compile definitions. Namespaced targets (`Boost::filesystem`) propagate all usage requirements automatically. + +--- + +### 6. Overwriting Find Variables + +❌ **Bad:** +```cmake +set(CMAKE_CXX_FLAGS "-Wall -g") # Overwrites user settings! +``` + +✅ **Good:** +```cmake +# In CMakeLists.txt: use target commands +target_compile_options(mylib PRIVATE -Wall -g) + +# Or if you must set variables, append: +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") +``` + +**Why:** Users may pass `CMAKE_CXX_FLAGS` via command line or presets. Overwriting destroys their customization. + +--- + +### 7. Wrong Generator Expression Syntax + +❌ **Bad:** +```cmake +target_compile_definitions(mylib PRIVATE + DEBUG=$ +) +``` + +✅ **Good:** +```cmake +target_compile_definitions(mylib PRIVATE + $<$:DEBUG> +) +``` + +**Why:** `$` evaluates to `1` or `0`. You want conditional inclusion, not a value. + +--- + +### 8. Missing ALIAS Targets + +❌ **Bad:** +```cmake +add_library(mylib src/lib.cpp) +# Consumer uses: target_link_libraries(app mylib) +``` + +✅ **Good:** +```cmake +add_library(mylib src/lib.cpp) +add_library(MyProject::mylib ALIAS mylib) +# Consumer uses: target_link_libraries(app MyProject::mylib) +``` + +**Why:** Namespaced targets can't be accidentally modified. They also match the names used after `install(EXPORT)`. + +--- + +### 9. Installing Wrong Paths + +❌ **Bad:** +```cmake +target_include_directories(mylib PUBLIC include) +``` + +✅ **Good:** +```cmake +target_include_directories(mylib PUBLIC + $ + $ +) +``` + +**Why:** The build directory path makes no sense after installation. Generator expressions let you specify different paths for build vs. install. + +--- + +## Makefile Antipatterns + +### 1. Overwriting Variables + +❌ **Bad:** +```makefile +CFLAGS = -Wall -O2 +``` + +✅ **Good:** +```makefile +CFLAGS ?= -Wall +CFLAGS += -O2 +``` + +**Why:** `=` overwrites anything the user passed via `make CFLAGS=...`. Use `?=` for defaults and `+=` to append. + +--- + +### 2. Missing .PHONY + +❌ **Bad:** +```makefile +clean: + rm -rf build +``` + +✅ **Good:** +```makefile +.PHONY: clean +clean: + rm -rf build +``` + +**Why:** If a file named `clean` exists, Make thinks it's up-to-date and won't run the recipe. + +--- + +### 3. Hardcoded Compilers + +❌ **Bad:** +```makefile +CC = gcc +CXX = g++ +``` + +✅ **Good:** +```makefile +CC ?= cc +CXX ?= c++ +``` + +**Why:** Users on different systems (macOS, BSD) may use clang by default. Allow override. + +--- + +### 4. Missing Dependencies + +❌ **Bad:** +```makefile +main.o: main.c + $(CC) -c main.c -o main.o +# Forgot: main.c includes config.h +``` + +✅ **Good:** +```makefile +CFLAGS += -MMD -MP +-include $(DEPS) +``` + +**Why:** Without automatic dependency tracking, changing headers doesn't rebuild affected sources. + +--- + +### 5. Recipe Directory Creation Race + +❌ **Bad:** +```makefile +build/%.o: src/%.c + mkdir -p build + $(CC) -c $< -o $@ +``` + +✅ **Good:** +```makefile +build/%.o: src/%.c | build + $(CC) -c $< -o $@ + +build: + mkdir -p $@ +``` + +**Why:** With parallel make (`-j`), multiple jobs might race to create `build`. Order-only prerequisites (`|`) ensure directory exists before any compilation starts. + +--- + +### 6. Using `make` in Recipes + +❌ **Bad:** +```makefile +all: + make -C subdir +``` + +✅ **Good:** +```makefile +all: + $(MAKE) -C subdir +``` + +**Why:** `$(MAKE)` preserves flags like `-j`, `-k`, and jobserver communication. Raw `make` loses parallel build capability. + +--- + +### 7. Recursive Variable Loops + +❌ **Bad:** +```makefile +CFLAGS = $(CFLAGS) -Wall # Infinite recursion! +``` + +✅ **Good:** +```makefile +CFLAGS := $(CFLAGS) -Wall # Simple expansion +# Or +CFLAGS += -Wall # Append +``` + +**Why:** `=` creates recursive variables. Self-reference causes infinite expansion. + +--- + +### 8. Forgetting Automatic Variables + +❌ **Bad:** +```makefile +build/foo.o: src/foo.c + $(CC) $(CFLAGS) -c src/foo.c -o build/foo.o +``` + +✅ **Good:** +```makefile +build/%.o: src/%.c + $(CC) $(CFLAGS) -c $< -o $@ +``` + +**Why:** Explicit paths duplicate information and don't generalize. Pattern rules with automatic variables work for any file. + +--- + +### 9. Wrong Flag Variables + +❌ **Bad:** +```makefile +CFLAGS += -I./include # -I is a preprocessor flag +CFLAGS += -lpthread # -l is a library flag +``` + +✅ **Good:** +```makefile +CPPFLAGS += -I./include +LDLIBS += -lpthread +``` + +**Why:** Built-in rules use variables correctly: +- `CPPFLAGS`: Preprocessor (`-I`, `-D`) +- `CFLAGS`/`CXXFLAGS`: Compiler (`-Wall`, `-O2`, `-std=`) +- `LDFLAGS`: Linker flags (`-L`, `-Wl,`) +- `LDLIBS`: Libraries (`-l`) + +--- + +## Quick Fixes Checklist + +### CMake: Before You Submit +1. ✅ Run `cmake --warn-uninitialized` +2. ✅ Build with a different generator (Ninja if using Make, or vice versa) +3. ✅ Test `cmake --install` to an empty prefix +4. ✅ Try as a subdirectory of another project + +### Make: Before You Submit +1. ✅ Run `make -n` (dry run) to see what would execute +2. ✅ Run `make -j$(nproc)` to test parallel builds +3. ✅ Run `make CC=clang` to test compiler override +4. ✅ Clean and rebuild after changing headers diff --git a/.claude/skills/dev-cmake/references/cmake-patterns.md b/.claude/skills/dev-cmake/references/cmake-patterns.md new file mode 100644 index 0000000..d17b432 --- /dev/null +++ b/.claude/skills/dev-cmake/references/cmake-patterns.md @@ -0,0 +1,358 @@ +# CMake Patterns Reference + +## Table of Contents +1. [Modern Library Pattern](#modern-library-pattern) +2. [Executable Pattern](#executable-pattern) +3. [Interface Libraries](#interface-libraries) +4. [Export and Install](#export-and-install) +5. [FetchContent Patterns](#fetchcontent-patterns) +6. [Presets](#presets) +7. [Toolchain Files](#toolchain-files) +8. [Testing Integration](#testing-integration) + +--- + +## Modern Library Pattern + +Complete pattern for a reusable library: + +```cmake +cmake_minimum_required(VERSION 3.23) +project(MyLib VERSION 1.0.0 LANGUAGES CXX) + +# Options for this project +option(MYLIB_BUILD_TESTS "Build tests" OFF) +option(MYLIB_BUILD_EXAMPLES "Build examples" OFF) + +# Create library target +add_library(mylib) +add_library(MyLib::mylib ALIAS mylib) + +# Sources (list explicitly, never glob) +target_sources(mylib + PRIVATE + src/mylib.cpp + src/internal.cpp + PUBLIC + FILE_SET public_headers + TYPE HEADERS + BASE_DIRS include + FILES + include/mylib/mylib.hpp + include/mylib/types.hpp +) + +# Language standard via features +target_compile_features(mylib PUBLIC cxx_std_17) + +# Include directories with generator expressions +target_include_directories(mylib + PUBLIC + $ + $ + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src +) + +# Dependencies +find_package(fmt 9.0 REQUIRED) +target_link_libraries(mylib + PUBLIC + fmt::fmt # Public because it's in our headers +) + +# Compile options (warnings are PRIVATE) +target_compile_options(mylib + PRIVATE + $<$:-Wall -Wextra -Wpedantic> + $<$:/W4> +) + +# Symbol visibility +set_target_properties(mylib PROPERTIES + CXX_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON +) + +# Version info +set_target_properties(mylib PROPERTIES + VERSION ${PROJECT_VERSION} + SOVERSION ${PROJECT_VERSION_MAJOR} +) +``` + +--- + +## Executable Pattern + +```cmake +add_executable(myapp) + +target_sources(myapp + PRIVATE + src/main.cpp + src/app.cpp +) + +target_link_libraries(myapp + PRIVATE + MyLib::mylib +) + +# Runtime output directory +set_target_properties(myapp PROPERTIES + RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin +) +``` + +--- + +## Interface Libraries + +For header-only libraries or compile flag collections: + +```cmake +# Header-only library +add_library(myheaders INTERFACE) +add_library(MyProject::headers ALIAS myheaders) + +target_include_directories(myheaders + INTERFACE + $ + $ +) + +target_compile_features(myheaders INTERFACE cxx_std_20) + +# Compile flags collection (for project-wide settings) +add_library(project_options INTERFACE) +target_compile_options(project_options + INTERFACE + $<$:-Wall -Wextra> +) +target_compile_definitions(project_options + INTERFACE + $<$:DEBUG_BUILD> +) +``` + +--- + +## Export and Install + +Complete export pattern for library consumers: + +```cmake +include(GNUInstallDirs) +include(CMakePackageConfigHelpers) + +# Install targets +install(TARGETS mylib + EXPORT MyLibTargets + LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} + ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} + FILE_SET public_headers +) + +# Install export file +install(EXPORT MyLibTargets + FILE MyLibTargets.cmake + NAMESPACE MyLib:: + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/MyLib +) + +# Generate config file +configure_package_config_file( + ${CMAKE_CURRENT_SOURCE_DIR}/cmake/MyLibConfig.cmake.in + ${CMAKE_CURRENT_BINARY_DIR}/MyLibConfig.cmake + INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/MyLib +) + +# Generate version file +write_basic_package_version_file( + ${CMAKE_CURRENT_BINARY_DIR}/MyLibConfigVersion.cmake + VERSION ${PROJECT_VERSION} + COMPATIBILITY SameMajorVersion +) + +# Install config files +install(FILES + ${CMAKE_CURRENT_BINARY_DIR}/MyLibConfig.cmake + ${CMAKE_CURRENT_BINARY_DIR}/MyLibConfigVersion.cmake + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/MyLib +) +``` + +Config file template (`cmake/MyLibConfig.cmake.in`): +```cmake +@PACKAGE_INIT@ + +include(CMakeFindDependencyMacro) +find_dependency(fmt 9.0) + +include("${CMAKE_CURRENT_LIST_DIR}/MyLibTargets.cmake") + +check_required_components(MyLib) +``` + +--- + +## FetchContent Patterns + +### Basic FetchContent +```cmake +include(FetchContent) + +FetchContent_Declare(googletest + GIT_REPOSITORY https://github.com/google/googletest.git + GIT_TAG v1.14.0 + FIND_PACKAGE_ARGS NAMES GTest +) + +# Try find_package first, fetch if not found +FetchContent_MakeAvailable(googletest) +``` + +### With options override +```cmake +FetchContent_Declare(spdlog + GIT_REPOSITORY https://github.com/gabime/spdlog.git + GIT_TAG v1.12.0 +) + +# Set options before MakeAvailable +set(SPDLOG_BUILD_EXAMPLE OFF CACHE BOOL "" FORCE) +set(SPDLOG_BUILD_TESTS OFF CACHE BOOL "" FORCE) + +FetchContent_MakeAvailable(spdlog) +``` + +### Prefer system packages +```cmake +FetchContent_Declare(fmt + GIT_REPOSITORY https://github.com/fmtlib/fmt.git + GIT_TAG 10.1.0 + FIND_PACKAGE_ARGS # Try find_package(fmt) first +) +FetchContent_MakeAvailable(fmt) +``` + +--- + +## Presets + +`CMakePresets.json` example: +```json +{ + "version": 6, + "cmakeMinimumRequired": {"major": 3, "minor": 23, "patch": 0}, + "configurePresets": [ + { + "name": "base", + "hidden": true, + "binaryDir": "${sourceDir}/build/${presetName}", + "installDir": "${sourceDir}/install/${presetName}" + }, + { + "name": "debug", + "inherits": "base", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug" + } + }, + { + "name": "release", + "inherits": "base", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release" + } + }, + { + "name": "ci", + "inherits": "release", + "cacheVariables": { + "BUILD_TESTING": "ON" + } + } + ], + "buildPresets": [ + {"name": "debug", "configurePreset": "debug"}, + {"name": "release", "configurePreset": "release"} + ], + "testPresets": [ + { + "name": "ci", + "configurePreset": "ci", + "output": {"outputOnFailure": true} + } + ] +} +``` + +Usage: +```bash +cmake --preset debug +cmake --build --preset debug +ctest --preset ci +``` + +--- + +## Toolchain Files + +Cross-compilation toolchain (`arm-toolchain.cmake`): +```cmake +set(CMAKE_SYSTEM_NAME Linux) +set(CMAKE_SYSTEM_PROCESSOR arm) + +set(CMAKE_SYSROOT /path/to/sysroot) + +set(CMAKE_C_COMPILER arm-linux-gnueabihf-gcc) +set(CMAKE_CXX_COMPILER arm-linux-gnueabihf-g++) + +set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) +set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) +set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) +set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) +``` + +--- + +## Testing Integration + +```cmake +include(CTest) + +if(BUILD_TESTING) + find_package(GTest REQUIRED) + + add_executable(mylib_tests) + target_sources(mylib_tests + PRIVATE + tests/test_main.cpp + tests/test_feature.cpp + ) + + target_link_libraries(mylib_tests + PRIVATE + MyLib::mylib + GTest::gtest_main + ) + + include(GoogleTest) + gtest_discover_tests(mylib_tests) +endif() +``` + +--- + +## Scope Reference + +| Scope | Build Requirement | Interface Requirement | Use When | +|-------|------------------|----------------------|----------| +| PRIVATE | ✓ | ✗ | Internal implementation details | +| INTERFACE | ✗ | ✓ | Header-only, or things only consumers need | +| PUBLIC | ✓ | ✓ | Headers that include dependencies | + +**Rule of thumb:** Start with `PRIVATE`. Only promote to `PUBLIC` if it appears in your public headers. diff --git a/.claude/skills/dev-cmake/references/make-patterns.md b/.claude/skills/dev-cmake/references/make-patterns.md new file mode 100644 index 0000000..d33eee1 --- /dev/null +++ b/.claude/skills/dev-cmake/references/make-patterns.md @@ -0,0 +1,368 @@ +# Make Patterns Reference + +## Table of Contents +1. [Production-Ready Template](#production-ready-template) +2. [Multi-Directory Projects](#multi-directory-projects) +3. [Library Builds](#library-builds) +4. [Automatic Dependencies](#automatic-dependencies) +5. [Cross-Compilation](#cross-compilation) +6. [Parallel Builds](#parallel-builds) +7. [Debug vs Release](#debug-vs-release) + +--- + +## Production-Ready Template + +Complete Makefile for a C/C++ project: + +```makefile +# Project configuration +PROJECT := myproject +VERSION := 1.0.0 + +# Directories +SRCDIR := src +INCDIR := include +BUILDDIR := build +BINDIR := bin + +# Tools (overridable) +CC ?= gcc +CXX ?= g++ +AR ?= ar +LD := $(CXX) + +# Flags (overridable, then extended) +CFLAGS ?= -Wall -Wextra -pedantic +CXXFLAGS ?= -Wall -Wextra -pedantic +LDFLAGS ?= +LDLIBS ?= + +# Project-specific additions +CPPFLAGS += -I$(INCDIR) +CFLAGS += -std=c11 -MMD -MP +CXXFLAGS += -std=c++17 -MMD -MP + +# Source files (explicit listing preferred, wildcard acceptable for simple projects) +SRCS := $(wildcard $(SRCDIR)/*.cpp) +OBJS := $(SRCS:$(SRCDIR)/%.cpp=$(BUILDDIR)/%.o) +DEPS := $(OBJS:.o=.d) + +# Default target +.PHONY: all +all: $(BINDIR)/$(PROJECT) + +# Link executable +$(BINDIR)/$(PROJECT): $(OBJS) | $(BINDIR) + $(LD) $(LDFLAGS) -o $@ $^ $(LDLIBS) + +# Compile sources +$(BUILDDIR)/%.o: $(SRCDIR)/%.cpp | $(BUILDDIR) + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< + +# Create directories +$(BUILDDIR) $(BINDIR): + mkdir -p $@ + +# Include dependencies +-include $(DEPS) + +# Clean +.PHONY: clean +clean: + $(RM) -r $(BUILDDIR) $(BINDIR) + +# Install +PREFIX ?= /usr/local +.PHONY: install +install: $(BINDIR)/$(PROJECT) + install -d $(DESTDIR)$(PREFIX)/bin + install -m 755 $(BINDIR)/$(PROJECT) $(DESTDIR)$(PREFIX)/bin/ + +.PHONY: uninstall +uninstall: + $(RM) $(DESTDIR)$(PREFIX)/bin/$(PROJECT) + +# Help +.PHONY: help +help: + @echo "Targets:" + @echo " all - Build $(PROJECT)" + @echo " clean - Remove build artifacts" + @echo " install - Install to PREFIX (default: /usr/local)" + @echo " help - Show this message" + @echo "" + @echo "Variables:" + @echo " CC=$(CC) CXX=$(CXX)" + @echo " CFLAGS=$(CFLAGS)" + @echo " PREFIX=$(PREFIX)" + +# Prevent deletion of intermediate files +.SECONDARY: + +# Delete targets on recipe failure +.DELETE_ON_ERROR: +``` + +--- + +## Multi-Directory Projects + +### Recursive Make (traditional, but has issues) +```makefile +SUBDIRS := lib app tests + +.PHONY: all clean $(SUBDIRS) + +all: $(SUBDIRS) + +$(SUBDIRS): + $(MAKE) -C $@ + +app: lib +tests: lib app + +clean: + for dir in $(SUBDIRS); do $(MAKE) -C $$dir clean; done +``` + +### Non-recursive Make (preferred for complex projects) +```makefile +# Top-level Makefile +BUILDDIR := build + +# Include all module makefiles +include src/lib/module.mk +include src/app/module.mk +include tests/module.mk + +.PHONY: all clean + +all: $(ALL_TARGETS) + +clean: + $(RM) -r $(BUILDDIR) +``` + +Module makefile (`src/lib/module.mk`): +```makefile +LIB_SRC := src/lib +LIB_SRCS := $(wildcard $(LIB_SRC)/*.cpp) +LIB_OBJS := $(LIB_SRCS:%.cpp=$(BUILDDIR)/%.o) + +$(BUILDDIR)/libcore.a: $(LIB_OBJS) + $(AR) rcs $@ $^ + +$(BUILDDIR)/$(LIB_SRC)/%.o: $(LIB_SRC)/%.cpp | $(BUILDDIR)/$(LIB_SRC) + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< + +$(BUILDDIR)/$(LIB_SRC): + mkdir -p $@ + +ALL_TARGETS += $(BUILDDIR)/libcore.a +``` + +--- + +## Library Builds + +### Static Library +```makefile +LIB_OBJS := $(patsubst %.c,%.o,$(wildcard src/*.c)) + +libfoo.a: $(LIB_OBJS) + $(AR) rcs $@ $^ +``` + +### Shared Library +```makefile +LIB_OBJS := $(patsubst %.c,%.o,$(wildcard src/*.c)) + +CFLAGS += -fPIC + +libfoo.so: $(LIB_OBJS) + $(CC) -shared -o $@ $^ $(LDFLAGS) + +# With versioning +LIBNAME := libfoo.so +LIBVER := 1.0.0 +LIBSONAME := $(LIBNAME).1 + +$(LIBNAME).$(LIBVER): $(LIB_OBJS) + $(CC) -shared -Wl,-soname,$(LIBSONAME) -o $@ $^ $(LDFLAGS) + ln -sf $@ $(LIBSONAME) + ln -sf $@ $(LIBNAME) +``` + +--- + +## Automatic Dependencies + +### GCC/Clang Method (recommended) +```makefile +CFLAGS += -MMD -MP + +DEPS := $(OBJS:.o=.d) +-include $(DEPS) +``` + +Flags explained: +- `-MMD`: Generate `.d` files with dependencies (excludes system headers) +- `-MP`: Add phony targets for headers (prevents errors when headers are deleted) + +### Manual Method (for non-GCC compilers) +```makefile +depend: $(SRCS) + $(CC) -MM $(CPPFLAGS) $^ > .depend + +-include .depend +``` + +--- + +## Cross-Compilation + +```makefile +# Cross-compile prefix +CROSS_COMPILE ?= + +CC := $(CROSS_COMPILE)gcc +CXX := $(CROSS_COMPILE)g++ +AR := $(CROSS_COMPILE)ar +LD := $(CROSS_COMPILE)ld + +# Usage: make CROSS_COMPILE=arm-linux-gnueabihf- +``` + +### Platform detection +```makefile +UNAME := $(shell uname -s) + +ifeq ($(UNAME),Linux) + LDLIBS += -lrt +endif +ifeq ($(UNAME),Darwin) + CFLAGS += -mmacosx-version-min=10.15 +endif +``` + +--- + +## Parallel Builds + +Make supports parallel builds with `-j`: +```bash +make -j$(nproc) +``` + +Ensure correct dependencies so parallel builds work: +```makefile +# Order-only prerequisite for directory creation +$(BUILDDIR)/%.o: %.c | $(BUILDDIR) + $(CC) $(CFLAGS) -c -o $@ $< + +# Bad: race condition +$(BUILDDIR)/%.o: %.c + mkdir -p $(BUILDDIR) # Multiple jobs might race + $(CC) $(CFLAGS) -c -o $@ $< +``` + +--- + +## Debug vs Release + +### Using separate targets +```makefile +CFLAGS_DEBUG := -g -O0 -DDEBUG +CFLAGS_RELEASE := -O2 -DNDEBUG + +.PHONY: debug release + +debug: CFLAGS += $(CFLAGS_DEBUG) +debug: all + +release: CFLAGS += $(CFLAGS_RELEASE) +release: all +``` + +### Using build directories +```makefile +BUILD ?= release + +ifeq ($(BUILD),debug) + CFLAGS += -g -O0 -DDEBUG + BUILDDIR := build/debug +else + CFLAGS += -O2 -DNDEBUG + BUILDDIR := build/release +endif +``` + +--- + +## Variable Reference + +| Variable | Purpose | Set By | +|----------|---------|--------| +| `CC` | C compiler | Make/User | +| `CXX` | C++ compiler | Make/User | +| `CFLAGS` | C compiler flags | User | +| `CXXFLAGS` | C++ compiler flags | User | +| `CPPFLAGS` | Preprocessor flags (-I, -D) | User | +| `LDFLAGS` | Linker flags (-L, -Wl,) | User | +| `LDLIBS` | Libraries (-l) | User | +| `AR` | Archive tool | Make/User | + +### Variable Assignment Types +```makefile +VAR = value # Recursive (re-evaluated on use) +VAR := value # Simple (evaluated once) +VAR ?= value # Conditional (set if unset) +VAR += value # Append +``` + +**Best practice:** Use `?=` for tool definitions (allow user override), `+=` for flags (preserve user additions). + +--- + +## Automatic Variables Quick Reference + +| Variable | Meaning | +|----------|---------| +| `$@` | Target filename | +| `$<` | First prerequisite | +| `$^` | All prerequisites (deduplicated) | +| `$+` | All prerequisites (with duplicates) | +| `$*` | Stem (what % matched) | +| `$(@D)` | Directory part of target | +| `$(@F)` | File part of target | +| `$(=16.0.0", + "npm": ">=8.0.0" + } +} +``` + +--- + +## 8. Document Platform Requirements + +```markdown +## Installation + +Requires: +- Node.js 16+ +- Python 3.6+ +- C++ build tools (platform-specific) + +See [node-gyp installation](https://github.com/nodejs/node-gyp#installation) for details. +``` + +--- + +## 9. Cache Build Artifacts in CI + +```yaml +# GitHub Actions +- uses: actions/cache@v3 + with: + path: ~/.node-gyp + key: ${{ runner.os }}-node-gyp-${{ hashFiles('binding.gyp') }} +``` + +--- + +## 10. Handle Missing Dependencies Gracefully + +```javascript +let addon; +try { + addon = require('./build/Release/addon.node'); +} catch (err) { + console.error('Native addon failed to load. Falling back to pure JS.'); + addon = require('./lib/fallback.js'); +} +module.exports = addon; +``` + +--- + +## Common Flags Reference + +```bash +# Performance +-j max # Use all CPU cores for parallel builds + +# Build type +--debug # Debug build (symbols, no optimization) +--release # Release build (default) + +# Target configuration +--target=VERSION # Target Node.js version (e.g., 18.0.0) +--arch=ARCH # Target architecture (x64, arm64, ia32) +--dist-url=URL # Headers download URL + +# Platform-specific +--python=PATH # Specify Python executable +--msvs_version=VERSION # Visual Studio version (2017, 2019, 2022) + +# Debugging +--verbose # Detailed build output +--silly # Maximum verbosity + +# Advanced +--devdir=DIR # Headers cache directory (default: ~/.node-gyp) +--nodedir=DIR # Node source directory (for building from source) +``` + +--- + +## Related Resources + +- [node-gyp Official Docs](https://github.com/nodejs/node-gyp) +- [Node-API Documentation](https://nodejs.org/api/n-api.html) +- [node-addon-api Package](https://github.com/nodejs/node-addon-api) +- [GYP Input Format Reference](https://gyp.gsrc.io/docs/InputFormatReference.md) +- [prebuild Package](https://github.com/prebuild/prebuild) +- [electron-rebuild](https://github.com/electron/electron-rebuild) +- [Native Abstractions for Node.js (nan)](https://github.com/nodejs/nan) - Legacy, use N-API instead diff --git a/.claude/skills/node-gyp/references/troubleshooting.md b/.claude/skills/node-gyp/references/troubleshooting.md new file mode 100644 index 0000000..3e19e19 --- /dev/null +++ b/.claude/skills/node-gyp/references/troubleshooting.md @@ -0,0 +1,242 @@ +# node-gyp Troubleshooting Guide + +Comprehensive troubleshooting for common node-gyp build errors. + +## Python Not Found + +**Symptoms:** +``` +gyp ERR! find Python +gyp ERR! Could not find Python +``` + +**Solutions:** +```bash +# Check Python version (needs 3.6+) +python3 --version + +# Option 1: Set globally +npm config set python /usr/bin/python3 + +# Option 2: Set per-project (.npmrc) +echo "python=/usr/bin/python3" >> .npmrc + +# Option 3: Environment variable +export npm_config_python=/usr/bin/python3 +``` + +**Why:** node-gyp requires Python to generate build files but doesn't always detect it correctly. + +--- + +## Visual Studio Not Found (Windows) + +**Symptoms:** +``` +gyp ERR! find VS +gyp ERR! Could not find Visual Studio installation +``` + +**Solutions:** +```bash +# Check installed versions +where msbuild + +# Specify version explicitly +node-gyp rebuild --msvs_version=2022 + +# Set globally +npm config set msvs_version 2022 + +# Alternative: Use VS Build Tools (lighter than full VS) +# Download from: https://visualstudio.microsoft.com/downloads/#build-tools-for-visual-studio-2022 +``` + +**Why:** Windows requires Visual Studio's C++ compiler. Auto-detection sometimes fails, especially with VS 2022. + +--- + +## NODE_MODULE_VERSION Mismatch + +**Symptoms:** +``` +Error: The module was compiled against a different Node.js version using +NODE_MODULE_VERSION 93. This version of Node.js requires NODE_MODULE_VERSION 108. +``` + +**Solutions:** +```bash +# Rebuild all native modules +npm rebuild + +# Rebuild specific package +npm rebuild bcrypt + +# Or full reinstall +rm -rf node_modules +npm install + +# For global modules +npm rebuild -g +``` + +**Why:** Native modules are compiled for specific Node.js versions. When you upgrade Node, they must be recompiled. + +--- + +## Architecture Mismatch (M1/M2 Macs) + +**Symptoms:** +``` +Error: dlopen(...): mach-o, but wrong architecture +``` + +**Solutions:** +```bash +# Clean and rebuild for current architecture +rm -rf node_modules +npm install + +# Or explicitly specify +node-gyp rebuild --arch=arm64 # for M1/M2 +node-gyp rebuild --arch=x64 # for Intel/Rosetta + +# Check your Node architecture +node -p "process.arch" # should match your system +``` + +**Why:** Running x64 Node on ARM Mac (or vice versa) requires matching native modules. + +--- + +## Missing Node.js Headers + +**Symptoms:** +``` +gyp ERR! Could not find common.gypi +gyp ERR! Could not find node.h +``` + +**Solutions:** +```bash +# Download headers for current Node version +node-gyp install + +# Force re-download +rm -rf ~/.node-gyp +node-gyp rebuild + +# For specific Node version +node-gyp install --target=18.0.0 +``` + +**Why:** Headers are cached in `~/.node-gyp/`. Corruption or version mismatches cause this error. + +--- + +## Make Not Found (Linux/macOS) + +**Symptoms:** +``` +gyp ERR! build error +gyp ERR! stack Error: `make` failed with exit code: 127 +``` + +**Solutions:** +```bash +# macOS +xcode-select --install + +# Ubuntu/Debian +sudo apt-get install build-essential + +# CentOS/RHEL +sudo yum groupinstall "Development Tools" + +# Verify +make --version +gcc --version +``` + +**Why:** Build tools aren't installed. Rare on developer machines but common in CI environments. + +--- + +## Permission Denied Errors + +**Symptoms:** +``` +EACCES: permission denied +gyp ERR! stack Error: EACCES: permission denied, mkdir '...' +``` + +**Solutions:** +```bash +# DO NOT use sudo npm install - use nvm or fix permissions instead + +# Option 1: Use nvm (recommended) +curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.0/install.sh | bash +nvm install node + +# Option 2: Fix npm permissions +mkdir ~/.npm-global +npm config set prefix '~/.npm-global' +echo 'export PATH=~/.npm-global/bin:$PATH' >> ~/.bashrc +source ~/.bashrc + +# Option 3: Fix ownership (Linux) +sudo chown -R $(whoami) ~/.npm /usr/local/lib/node_modules +``` + +**Why:** Installing with sudo causes permission issues. Always use a user-owned Node installation. + +--- + +## Undefined Reference / Symbol Not Found + +**Symptoms:** +``` +undefined reference to `pthread_create' +ld: symbol(s) not found for architecture arm64 +``` + +**Solutions:** + +Add missing libraries to binding.gyp: +```json +{ + "targets": [{ + "conditions": [ + ["OS=='linux'", { + "libraries": ["-lpthread", "-ldl"] + }], + ["OS=='mac'", { + "libraries": ["-framework CoreFoundation"] + }] + ] + }] +} +``` + +**Why:** Missing system libraries. Platform-specific linking is required. + +--- + +## Verbose Debugging + +When errors are unclear, enable detailed logging: + +```bash +# Enable detailed logging +node-gyp rebuild --verbose + +# Even more detail +node-gyp rebuild --verbose --silly + +# Check environment +node-gyp list # Show installed Node versions +node -p "process.versions" # Node/V8 versions +npm config list # npm configuration +``` + +**Why:** Default output hides crucial details. Verbose mode shows exact compiler commands and errors. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..f40bb82 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,235 @@ +# CI workflow for node-webcodecs +# Runs linting, building, and testing on all supported platforms +# +# Platforms (matching @pproenca/webcodecs-ffmpeg packages): +# - darwin-arm64 (macos-14 runner) +# - darwin-x64 (macos-13 runner) +# - linux-x64-glibc (Rocky Linux 9 container) +# - linux-arm64 (QEMU + ARM64 container) +# - linux-x64-musl (Alpine 3.20 container) +# +# Variants: +# - free (LGPL FFmpeg) +# - non-free (GPL FFmpeg with x264/x265) +# +# Node.js version: 20 + +name: CI + +on: + push: + branches: [master] + pull_request: + branches: [master] + +permissions: + contents: read + +jobs: + lint: + name: Lint + runs-on: ubuntu-24.04 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: 20 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install cpplint + run: pip install cpplint + + - name: Install dependencies + run: npm install + + - name: Build TypeScript + run: npm run build:ts + + - name: Lint C++ + run: cpplint --quiet src/*.h src/*.cc + + - name: Lint TypeScript + run: npm run lint:ts + + - name: Lint types + run: npm run lint:types + + - name: Lint markdown + run: npm run lint:md + + build: + name: ${{ matrix.platform }}-${{ matrix.variant }} + runs-on: ${{ matrix.runner }} + container: ${{ matrix.container || '' }} + strategy: + fail-fast: false + matrix: + platform: [darwin-arm64, darwin-x64, linux-x64-glibc, linux-arm64, linux-x64-musl] + variant: [free, non-free] + include: + - platform: darwin-arm64 + runner: macos-14 + os: darwin + ffmpeg_pkg: darwin-arm64 + - platform: darwin-x64 + runner: macos-13 + os: darwin + ffmpeg_pkg: darwin-x64 + - platform: linux-x64-glibc + runner: ubuntu-24.04 + container: rockylinux:9 + os: linux + ffmpeg_pkg: linux-x64 + - platform: linux-arm64 + runner: ubuntu-24.04 + os: linux + ffmpeg_pkg: linux-arm64 + qemu: true + - platform: linux-x64-musl + runner: ubuntu-24.04 + container: alpine:3.20 + os: linux + ffmpeg_pkg: linux-x64-musl + + steps: + # QEMU setup for ARM64 cross-compilation + - name: Set up QEMU + if: matrix.qemu == true + uses: docker/setup-qemu-action@v3 + with: + platforms: arm64 + + - name: Set up Docker Buildx + if: matrix.qemu == true + uses: docker/setup-buildx-action@v3 + + # System dependencies for Rocky Linux container + - name: Install system dependencies (Rocky Linux) + if: contains(matrix.container, 'rockylinux') + run: | + dnf install -y epel-release + dnf config-manager --set-enabled crb + dnf install -y https://mirrors.rpmfusion.org/free/el/rpmfusion-free-release-9.noarch.rpm + dnf install -y gcc-c++ make python3 git pkgconfig zlib-devel bzip2-devel libatomic + + # System dependencies for Alpine container + - name: Install system dependencies (Alpine) + if: contains(matrix.container, 'alpine') + run: | + apk add --no-cache build-base python3 pkgconf git nodejs npm zlib-dev bzip2-dev + + - name: Checkout + uses: actions/checkout@v4 + + # Node.js setup for non-container jobs (macOS) + - name: Setup Node.js + if: matrix.os == 'darwin' + uses: actions/setup-node@v4 + with: + node-version: 20 + + # Node.js setup for Rocky Linux container + - name: Install Node.js (Rocky Linux) + if: contains(matrix.container, 'rockylinux') + uses: actions/setup-node@v4 + with: + node-version: 20 + + # Cache node-gyp for macOS + - name: Cache node-gyp + if: matrix.os == 'darwin' + uses: actions/cache@v4 + with: + path: | + ~/.cache/node-gyp + ~/Library/Caches/node-gyp + key: node-gyp-${{ matrix.platform }}-node20-${{ hashFiles('binding.gyp') }} + restore-keys: | + node-gyp-${{ matrix.platform }}-node20- + + # FFmpeg installation for macOS + # - free variant: npm LGPL package + # - non-free variant: Homebrew (includes x264/x265) + - name: Install FFmpeg (macOS npm - free) + if: matrix.os == 'darwin' && matrix.variant == 'free' + run: | + npm cache clean --force + PKG="@pproenca/webcodecs-ffmpeg-${{ matrix.ffmpeg_pkg }}" + echo "Installing FFmpeg package: $PKG" + npm install --no-save "$PKG" + + - name: Install FFmpeg (macOS Homebrew - non-free) + if: matrix.os == 'darwin' && matrix.variant == 'non-free' + run: brew install ffmpeg + + # Install dependencies (macOS - omit optional to avoid platform mismatch) + - name: Install dependencies (macOS) + if: matrix.os == 'darwin' + run: npm install --omit=optional + + # Install FFmpeg npm package based on variant (Linux non-ARM64) + - name: Install FFmpeg (Linux npm) + if: matrix.os == 'linux' && matrix.qemu != true + run: | + npm cache clean --force + PKG="@pproenca/webcodecs-ffmpeg-${{ matrix.ffmpeg_pkg }}" + if [ "${{ matrix.variant }}" = "non-free" ]; then + PKG="${PKG}-non-free" + fi + echo "Installing FFmpeg package: $PKG" + npm install --no-save "$PKG" + + # Install dependencies (Linux containers) + - name: Install dependencies (Linux) + if: matrix.os == 'linux' && matrix.qemu != true + run: npm install + + # Linux ARM64 builds via Docker with QEMU + - name: Build and Test (linux-arm64) + if: matrix.qemu == true + env: + VARIANT: ${{ matrix.variant }} + run: | + # Determine FFmpeg package based on variant + if [ "$VARIANT" = "non-free" ]; then + FFMPEG_PKG="@pproenca/webcodecs-ffmpeg-linux-arm64-non-free" + else + FFMPEG_PKG="@pproenca/webcodecs-ffmpeg-linux-arm64" + fi + + docker run --rm --platform linux/arm64 \ + -v "${{ github.workspace }}:/workspace" \ + -w /workspace \ + -e FFMPEG_VARIANT="$VARIANT" \ + arm64v8/rockylinux:9 \ + bash -c " + set -e + dnf install -y epel-release + dnf config-manager --set-enabled crb + dnf install -y https://mirrors.rpmfusion.org/free/el/rpmfusion-free-release-9.noarch.rpm + dnf install -y gcc-c++ make python3 git pkgconfig zlib-devel bzip2-devel libatomic + curl -fsSL https://rpm.nodesource.com/setup_20.x | bash - + dnf install -y nodejs + npm cache clean --force + npm install --no-save \"$FFMPEG_PKG\" + npm install + npm run build + npm test + " + + # Build for non-ARM64 platforms + - name: Build + if: matrix.qemu != true + run: npm run build + + # Test for non-ARM64 platforms + - name: Test + if: matrix.qemu != true + run: npm test diff --git a/binding.gyp b/binding.gyp index 705ced5..642ef8e 100644 --- a/binding.gyp +++ b/binding.gyp @@ -2,6 +2,27 @@ "variables": { "enable_sanitizers%": 0 }, + "target_defaults": { + "default_configuration": "Release", + "configurations": { + "Debug": { + "defines": ["DEBUG", "_DEBUG"], + "cflags_cc": ["-g", "-O0"], + "xcode_settings": { + "GCC_OPTIMIZATION_LEVEL": "0", + "GCC_GENERATE_DEBUGGING_SYMBOLS": "YES" + } + }, + "Release": { + "defines": ["NDEBUG"], + "cflags_cc": ["-O3"], + "xcode_settings": { + "GCC_OPTIMIZATION_LEVEL": "3", + "GCC_GENERATE_DEBUGGING_SYMBOLS": "NO" + } + } + } + }, "targets": [ { "target_name": "node_webcodecs", @@ -44,10 +65,10 @@ "conditions": [ ["OS=='mac'", { "include_dirs": [ - "/dev/null || pkg-config --cflags-only-I libavcodec libavutil libswscale libswresample libavfilter 2>/dev/null | sed s/-I//g || echo '/opt/homebrew/include /usr/local/include')" + "/dev/null || pkg-config --cflags-only-I libavcodec libavutil libswscale libswresample libavfilter 2>/dev/null | sed s/-I//g || echo '/opt/homebrew/include /usr/local/include /opt/local/include')" ], "libraries": [ - "/dev/null || (pkg-config --libs libavcodec libavformat libavutil libswscale libswresample libavfilter 2>/dev/null | sed 's/-framework [^ ]*//g') || echo '-L/opt/homebrew/lib -L/usr/local/lib -lavcodec -lavformat -lavutil -lswscale -lswresample -lavfilter')", + "/dev/null || (pkg-config --libs libavcodec libavformat libavutil libswscale libswresample libavfilter 2>/dev/null | sed 's/-framework [^ ]*//g') || echo '-L/opt/homebrew/lib -L/usr/local/lib -L/opt/local/lib -lavcodec -lavformat -lavutil -lswscale -lswresample -lavfilter')", "-framework VideoToolbox", "-framework AudioToolbox", "-framework CoreMedia", @@ -71,34 +92,42 @@ "-fexceptions", "-Wall", "-Wextra", + "-Wpedantic", + "-Wshadow", "-Wno-unused-parameter" ], "OTHER_LDFLAGS": [ - "-mmacosx-version-min=11.0" + "-mmacosx-version-min=11.0", + "-Wl,-rpath,@loader_path/../lib" ] } }], ["OS=='linux'", { "include_dirs": [ - "/dev/null || pkg-config --cflags-only-I libavcodec libavutil libswscale libswresample libavfilter | sed s/-I//g)" + "/dev/null || pkg-config --cflags-only-I libavcodec libavutil libswscale libswresample libavfilter 2>/dev/null | sed s/-I//g || echo '/usr/include /usr/local/include')" ], "libraries": [ - "/dev/null || pkg-config --libs --static libavcodec libavformat libavutil libswscale libswresample libavfilter)", + "/dev/null || pkg-config --libs --static libavcodec libavformat libavutil libswscale libswresample libavfilter 2>/dev/null || pkg-config --libs libavcodec libavformat libavutil libswscale libswresample libavfilter 2>/dev/null || echo '-L/usr/lib -L/usr/local/lib -lavcodec -lavformat -lavutil -lswscale -lswresample -lavfilter')", "-lpthread", "-lm", "-ldl", "-lz" ], "ldflags": [ - "-Wl,-Bsymbolic" + "-Wl,-Bsymbolic", + "-Wl,-rpath,$ORIGIN/../lib", + "-fno-lto" ], "cflags_cc": [ "-std=c++20", "-fexceptions", "-Wall", "-Wextra", + "-Wpedantic", + "-Wshadow", "-Wno-unused-parameter", - "-fPIC" + "-fPIC", + "-fno-lto" ] }], ["enable_sanitizers==1", { diff --git a/gyp/ffmpeg-paths-lib.js b/gyp/ffmpeg-paths-lib.js index f314cb9..0d827e3 100644 --- a/gyp/ffmpeg-paths-lib.js +++ b/gyp/ffmpeg-paths-lib.js @@ -4,10 +4,19 @@ // // Resolve FFmpeg paths for node-gyp binding. // -// Resolution order: -// 1. FFMPEG_ROOT env var (set by CI from deps-v* release artifacts) -// 2. ./ffmpeg-install directory (local development) -// 3. System pkg-config (fallback) +// Include path resolution order: +// 1. @pproenca/webcodecs-ffmpeg-dev npm package (cross-platform headers) +// 2. FFMPEG_ROOT env var + pkg-config +// 3. Platform-specific npm package + pkg-config +// 4. ./ffmpeg-install + pkg-config +// 5. System pkg-config (fallback) +// +// Library flags resolution order: +// 1. Platform-specific npm package ./link-flags export (static paths, no pkg-config) +// 2. FFMPEG_ROOT env var + pkg-config +// 3. Platform-specific npm package + pkg-config +// 4. ./ffmpeg-install + pkg-config +// 5. System pkg-config (fallback) // // The FFmpeg static libraries are built from: // - Linux: docker/Dockerfile.linux-x64 (Alpine musl, fully static) @@ -24,16 +33,40 @@ // node-gyp's splits output by whitespace, breaking "-framework Metal" into // two tokens. binding.gyp already explicitly adds required frameworks. Object.defineProperty(exports, "__esModule", { value: true }); +exports.isPkgConfigAvailable = isPkgConfigAvailable; exports.filterFrameworkFlags = filterFrameworkFlags; exports.getFfmpegRoot = getFfmpegRoot; exports.runPkgConfig = runPkgConfig; exports.resolveLibFlags = resolveLibFlags; exports.resolveIncludeFlags = resolveIncludeFlags; +exports.resolveRpath = resolveRpath; exports.resolveProjectRoot = resolveProjectRoot; const node_fs_1 = require("node:fs"); const node_child_process_1 = require("node:child_process"); const node_path_1 = require("node:path"); +const node_os_1 = require("node:os"); const FFMPEG_LIBS = 'libavcodec libavformat libavutil libswscale libswresample libavfilter'; +const LOG_PREFIX = '[node-webcodecs]'; +function logError(message) { + console.error(`${LOG_PREFIX} ${message}`); +} +function logDebug(message, env) { + if (env.DEBUG) { + console.error(`${LOG_PREFIX} [DEBUG] ${message}`); + } +} +function isPkgConfigAvailable() { + try { + (0, node_child_process_1.execSync)('pkg-config --version', { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return true; + } + catch { + return false; + } +} function filterFrameworkFlags(flags) { const tokens = flags.split(/\s+/); const result = []; @@ -46,7 +79,87 @@ function filterFrameworkFlags(flags) { } return result.join(' '); } +function isMuslLibc() { + // Check if we're running on musl libc (Alpine Linux, etc.) + if ((0, node_os_1.platform)() !== 'linux') { + return false; + } + try { + // ldd --version outputs "musl libc" on musl systems + const result = (0, node_child_process_1.execSync)('ldd --version 2>&1 || true', { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return result.toLowerCase().includes('musl'); + } + catch { + return false; + } +} +function tryResolveIncludeFromDevPackage() { + // Try to resolve headers from the cross-platform dev package + // This package contains only headers, no platform-specific libraries + try { + const includeIndex = require.resolve('@pproenca/webcodecs-ffmpeg-dev/include'); + const includeDir = (0, node_path_1.dirname)(includeIndex); + if ((0, node_fs_1.existsSync)(includeDir)) { + return includeDir; + } + } + catch { + // Package not installed + } + return null; +} +function tryResolveLinkFlagsFromNpmPackage() { + // Resolve link flags directly from platform-specific npm package + // This avoids pkg-config which has hardcoded paths in .pc files + const basePlatform = `${(0, node_os_1.platform)()}-${(0, node_os_1.arch)()}`; + const pkgNames = isMuslLibc() + ? [`@pproenca/webcodecs-ffmpeg-${basePlatform}-musl`, `@pproenca/webcodecs-ffmpeg-${basePlatform}`] + : [`@pproenca/webcodecs-ffmpeg-${basePlatform}`]; + for (const pkgName of pkgNames) { + try { + // Try to resolve the link-flags export from the platform package + // eslint-disable-next-line @typescript-eslint/no-require-imports + const linkFlags = require(`${pkgName}/link-flags`); + if (linkFlags?.flags) { + return linkFlags.flags; + } + } + catch { + // Package not installed or doesn't have link-flags export + } + } + return null; +} +function tryResolveFromNpmPackage() { + // Build platform-specific package name (e.g., @pproenca/webcodecs-ffmpeg-darwin-arm64) + // On musl systems, try the musl-specific package first + const basePlatform = `${(0, node_os_1.platform)()}-${(0, node_os_1.arch)()}`; + const pkgNames = isMuslLibc() + ? [`@pproenca/webcodecs-ffmpeg-${basePlatform}-musl`, `@pproenca/webcodecs-ffmpeg-${basePlatform}`] + : [`@pproenca/webcodecs-ffmpeg-${basePlatform}`]; + for (const pkgName of pkgNames) { + try { + // Resolve the pkgconfig export from the platform package + // The package exports "./pkgconfig" pointing to "./lib/pkgconfig/index.js" + const pkgconfigIndex = require.resolve(`${pkgName}/pkgconfig`); + const pkgconfig = (0, node_path_1.dirname)(pkgconfigIndex); + if ((0, node_fs_1.existsSync)(pkgconfig)) { + // The root is two levels up from lib/pkgconfig + const root = (0, node_path_1.dirname)((0, node_path_1.dirname)(pkgconfig)); + return { root, pkgconfig }; + } + } + catch { + // Package not installed - try next one + } + } + return null; +} function getFfmpegRoot(projectRoot, env) { + // 1. FFMPEG_ROOT env var (explicit override) if (env.FFMPEG_ROOT) { const root = env.FFMPEG_ROOT; const pkgconfig = (0, node_path_1.join)(root, 'lib', 'pkgconfig'); @@ -54,11 +167,18 @@ function getFfmpegRoot(projectRoot, env) { return { root, pkgconfig }; } } + // 2. @pproenca/webcodecs-ffmpeg npm package (if installed) + const npmPackage = tryResolveFromNpmPackage(); + if (npmPackage) { + return npmPackage; + } + // 3. ./ffmpeg-install directory (local development) const ffmpegInstall = (0, node_path_1.join)(projectRoot, 'ffmpeg-install'); const pkgconfig = (0, node_path_1.join)(ffmpegInstall, 'lib', 'pkgconfig'); if ((0, node_fs_1.existsSync)(pkgconfig)) { return { root: ffmpegInstall, pkgconfig }; } + // 4. System pkg-config will be used as fallback by the caller return null; } function runPkgConfig(args, ffmpegRoot, pkgConfigPath, env) { @@ -70,17 +190,31 @@ function runPkgConfig(args, ffmpegRoot, pkgConfigPath, env) { env: mergedEnv, stdio: ['pipe', 'pipe', 'pipe'], }); - return result.trim(); + const trimmed = result.trim(); + if (!trimmed) { + logError(`pkg-config returned empty output for: ${args}`); + logError('Ensure FFmpeg 5.0+ development files are installed.'); + return null; + } + logDebug(`pkg-config ${args} -> ${trimmed}`, env); + return trimmed; } catch (error) { - if (env.DEBUG) { - const message = error instanceof Error ? error.message : String(error); - console.error(`pkg-config failed: ${message}`); - } + const message = error instanceof Error ? error.message : String(error); + logError(`pkg-config failed: ${message}`); + logError('Ensure FFmpeg 5.0+ development files are installed.'); + logError('Install with: brew install ffmpeg (macOS), apt install libavcodec-dev libavformat-dev libavutil-dev libswscale-dev libswresample-dev libavfilter-dev (Debian/Ubuntu)'); return null; } } -function resolveLibFlags(projectRoot, env, platform) { +function resolveLibFlags(projectRoot, env, currentPlatform) { + // 1. Try direct link flags from npm package (avoids pkg-config path issues) + const directFlags = tryResolveLinkFlagsFromNpmPackage(); + if (directFlags) { + logDebug(`lib (npm link-flags) -> ${directFlags}`, env); + return currentPlatform === 'darwin' ? filterFrameworkFlags(directFlags) : directFlags; + } + // 2. Fall back to pkg-config const ffmpeg = getFfmpegRoot(projectRoot, env); if (!ffmpeg) { return null; @@ -89,9 +223,16 @@ function resolveLibFlags(projectRoot, env, platform) { if (!result) { return null; } - return platform === 'darwin' ? filterFrameworkFlags(result) : result; + return currentPlatform === 'darwin' ? filterFrameworkFlags(result) : result; } function resolveIncludeFlags(projectRoot, env) { + // 1. Try the cross-platform dev package first (has headers only) + const devInclude = tryResolveIncludeFromDevPackage(); + if (devInclude) { + logDebug(`include (dev package) -> ${devInclude}`, env); + return devInclude; + } + // 2. Fall back to pkg-config from FFmpeg root const ffmpeg = getFfmpegRoot(projectRoot, env); if (!ffmpeg) { return null; @@ -102,6 +243,19 @@ function resolveIncludeFlags(projectRoot, env) { } return result.replace(/-I/g, '').trim(); } +function resolveRpath(projectRoot, env) { + const ffmpeg = getFfmpegRoot(projectRoot, env); + if (!ffmpeg) { + return null; + } + // Return the lib directory path for RPATH configuration + const libDir = (0, node_path_1.join)(ffmpeg.root, 'lib'); + if ((0, node_fs_1.existsSync)(libDir)) { + logDebug(`rpath -> ${libDir}`, env); + return libDir; + } + return null; +} function resolveProjectRoot() { return (0, node_path_1.resolve)(__dirname, '..'); } diff --git a/gyp/ffmpeg-paths-lib.ts b/gyp/ffmpeg-paths-lib.ts index ac79b04..d02c317 100644 --- a/gyp/ffmpeg-paths-lib.ts +++ b/gyp/ffmpeg-paths-lib.ts @@ -3,10 +3,19 @@ // // Resolve FFmpeg paths for node-gyp binding. // -// Resolution order: -// 1. FFMPEG_ROOT env var (set by CI from deps-v* release artifacts) -// 2. ./ffmpeg-install directory (local development) -// 3. System pkg-config (fallback) +// Include path resolution order: +// 1. @pproenca/webcodecs-ffmpeg-dev npm package (cross-platform headers) +// 2. FFMPEG_ROOT env var + pkg-config +// 3. Platform-specific npm package + pkg-config +// 4. ./ffmpeg-install + pkg-config +// 5. System pkg-config (fallback) +// +// Library flags resolution order: +// 1. Platform-specific npm package ./link-flags export (static paths, no pkg-config) +// 2. FFMPEG_ROOT env var + pkg-config +// 3. Platform-specific npm package + pkg-config +// 4. ./ffmpeg-install + pkg-config +// 5. System pkg-config (fallback) // // The FFmpeg static libraries are built from: // - Linux: docker/Dockerfile.linux-x64 (Alpine musl, fully static) @@ -25,10 +34,35 @@ import {existsSync} from 'node:fs'; import {execSync} from 'node:child_process'; -import {join, resolve} from 'node:path'; +import {join, resolve, dirname} from 'node:path'; +import {platform, arch} from 'node:os'; const FFMPEG_LIBS = 'libavcodec libavformat libavutil libswscale libswresample libavfilter'; +const LOG_PREFIX = '[node-webcodecs]'; + +function logError(message: string): void { + console.error(`${LOG_PREFIX} ${message}`); +} + +function logDebug(message: string, env: NodeJS.ProcessEnv): void { + if (env.DEBUG) { + console.error(`${LOG_PREFIX} [DEBUG] ${message}`); + } +} + +export function isPkgConfigAvailable(): boolean { + try { + execSync('pkg-config --version', { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return true; + } catch { + return false; + } +} + export interface FfmpegRoot { readonly root: string; readonly pkgconfig: string; @@ -47,7 +81,90 @@ export function filterFrameworkFlags(flags: string): string { return result.join(' '); } +function isMuslLibc(): boolean { + // Check if we're running on musl libc (Alpine Linux, etc.) + if (platform() !== 'linux') { + return false; + } + try { + // ldd --version outputs "musl libc" on musl systems + const result = execSync('ldd --version 2>&1 || true', { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return result.toLowerCase().includes('musl'); + } catch { + return false; + } +} + +function tryResolveIncludeFromDevPackage(): string | null { + // Try to resolve headers from the cross-platform dev package + // This package contains only headers, no platform-specific libraries + try { + const includeIndex = require.resolve('@pproenca/webcodecs-ffmpeg-dev/include'); + const includeDir = dirname(includeIndex); + if (existsSync(includeDir)) { + return includeDir; + } + } catch { + // Package not installed + } + return null; +} + +function tryResolveLinkFlagsFromNpmPackage(): string | null { + // Resolve link flags directly from platform-specific npm package + // This avoids pkg-config which has hardcoded paths in .pc files + const basePlatform = `${platform()}-${arch()}`; + const pkgNames = isMuslLibc() + ? [`@pproenca/webcodecs-ffmpeg-${basePlatform}-musl`, `@pproenca/webcodecs-ffmpeg-${basePlatform}`] + : [`@pproenca/webcodecs-ffmpeg-${basePlatform}`]; + + for (const pkgName of pkgNames) { + try { + // Try to resolve the link-flags export from the platform package + // eslint-disable-next-line @typescript-eslint/no-require-imports + const linkFlags = require(`${pkgName}/link-flags`); + if (linkFlags?.flags) { + return linkFlags.flags; + } + } catch { + // Package not installed or doesn't have link-flags export + } + } + return null; +} + +function tryResolveFromNpmPackage(): FfmpegRoot | null { + // Build platform-specific package name (e.g., @pproenca/webcodecs-ffmpeg-darwin-arm64) + // On musl systems, try the musl-specific package first + const basePlatform = `${platform()}-${arch()}`; + const pkgNames = isMuslLibc() + ? [`@pproenca/webcodecs-ffmpeg-${basePlatform}-musl`, `@pproenca/webcodecs-ffmpeg-${basePlatform}`] + : [`@pproenca/webcodecs-ffmpeg-${basePlatform}`]; + + for (const pkgName of pkgNames) { + try { + // Resolve the pkgconfig export from the platform package + // The package exports "./pkgconfig" pointing to "./lib/pkgconfig/index.js" + const pkgconfigIndex = require.resolve(`${pkgName}/pkgconfig`); + const pkgconfig = dirname(pkgconfigIndex); + + if (existsSync(pkgconfig)) { + // The root is two levels up from lib/pkgconfig + const root = dirname(dirname(pkgconfig)); + return {root, pkgconfig}; + } + } catch { + // Package not installed - try next one + } + } + return null; +} + export function getFfmpegRoot(projectRoot: string, env: NodeJS.ProcessEnv): FfmpegRoot | null { + // 1. FFMPEG_ROOT env var (explicit override) if (env.FFMPEG_ROOT) { const root = env.FFMPEG_ROOT; const pkgconfig = join(root, 'lib', 'pkgconfig'); @@ -56,12 +173,20 @@ export function getFfmpegRoot(projectRoot: string, env: NodeJS.ProcessEnv): Ffmp } } + // 2. @pproenca/webcodecs-ffmpeg npm package (if installed) + const npmPackage = tryResolveFromNpmPackage(); + if (npmPackage) { + return npmPackage; + } + + // 3. ./ffmpeg-install directory (local development) const ffmpegInstall = join(projectRoot, 'ffmpeg-install'); const pkgconfig = join(ffmpegInstall, 'lib', 'pkgconfig'); if (existsSync(pkgconfig)) { return {root: ffmpegInstall, pkgconfig}; } + // 4. System pkg-config will be used as fallback by the caller return null; } @@ -80,12 +205,19 @@ export function runPkgConfig( env: mergedEnv, stdio: ['pipe', 'pipe', 'pipe'], }); - return result.trim(); - } catch (error) { - if (env.DEBUG) { - const message = error instanceof Error ? error.message : String(error); - console.error(`pkg-config failed: ${message}`); + const trimmed = result.trim(); + if (!trimmed) { + logError(`pkg-config returned empty output for: ${args}`); + logError('Ensure FFmpeg 5.0+ development files are installed.'); + return null; } + logDebug(`pkg-config ${args} -> ${trimmed}`, env); + return trimmed; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logError(`pkg-config failed: ${message}`); + logError('Ensure FFmpeg 5.0+ development files are installed.'); + logError('Install with: brew install ffmpeg (macOS), apt install libavcodec-dev libavformat-dev libavutil-dev libswscale-dev libswresample-dev libavfilter-dev (Debian/Ubuntu)'); return null; } } @@ -93,8 +225,16 @@ export function runPkgConfig( export function resolveLibFlags( projectRoot: string, env: NodeJS.ProcessEnv, - platform: NodeJS.Platform, + currentPlatform: NodeJS.Platform, ): string | null { + // 1. Try direct link flags from npm package (avoids pkg-config path issues) + const directFlags = tryResolveLinkFlagsFromNpmPackage(); + if (directFlags) { + logDebug(`lib (npm link-flags) -> ${directFlags}`, env); + return currentPlatform === 'darwin' ? filterFrameworkFlags(directFlags) : directFlags; + } + + // 2. Fall back to pkg-config const ffmpeg = getFfmpegRoot(projectRoot, env); if (!ffmpeg) { return null; @@ -103,13 +243,21 @@ export function resolveLibFlags( if (!result) { return null; } - return platform === 'darwin' ? filterFrameworkFlags(result) : result; + return currentPlatform === 'darwin' ? filterFrameworkFlags(result) : result; } export function resolveIncludeFlags( projectRoot: string, env: NodeJS.ProcessEnv, ): string | null { + // 1. Try the cross-platform dev package first (has headers only) + const devInclude = tryResolveIncludeFromDevPackage(); + if (devInclude) { + logDebug(`include (dev package) -> ${devInclude}`, env); + return devInclude; + } + + // 2. Fall back to pkg-config from FFmpeg root const ffmpeg = getFfmpegRoot(projectRoot, env); if (!ffmpeg) { return null; @@ -121,6 +269,23 @@ export function resolveIncludeFlags( return result.replace(/-I/g, '').trim(); } +export function resolveRpath( + projectRoot: string, + env: NodeJS.ProcessEnv, +): string | null { + const ffmpeg = getFfmpegRoot(projectRoot, env); + if (!ffmpeg) { + return null; + } + // Return the lib directory path for RPATH configuration + const libDir = join(ffmpeg.root, 'lib'); + if (existsSync(libDir)) { + logDebug(`rpath -> ${libDir}`, env); + return libDir; + } + return null; +} + export function resolveProjectRoot(): string { return resolve(__dirname, '..'); } diff --git a/gyp/ffmpeg-paths.js b/gyp/ffmpeg-paths.js index a0160a3..540c3c1 100644 --- a/gyp/ffmpeg-paths.js +++ b/gyp/ffmpeg-paths.js @@ -21,7 +21,12 @@ if (mode === 'include') { process.exit(1); } if (mode === 'rpath') { - process.exit(0); + const result = (0, ffmpeg_paths_lib_1.resolveRpath)(projectRoot, process.env); + if (result) { + console.log(result); + process.exit(0); + } + process.exit(1); } console.error(`Unknown mode: ${mode}`); process.exit(1); diff --git a/gyp/ffmpeg-paths.ts b/gyp/ffmpeg-paths.ts index 6bc43d4..642ffd2 100644 --- a/gyp/ffmpeg-paths.ts +++ b/gyp/ffmpeg-paths.ts @@ -4,6 +4,7 @@ import { resolveProjectRoot, resolveLibFlags, resolveIncludeFlags, + resolveRpath, } from './ffmpeg-paths-lib'; const mode = process.argv[2] ?? 'lib'; @@ -28,7 +29,12 @@ if (mode === 'include') { } if (mode === 'rpath') { - process.exit(0); + const result = resolveRpath(projectRoot, process.env); + if (result) { + console.log(result); + process.exit(0); + } + process.exit(1); } console.error(`Unknown mode: ${mode}`); diff --git a/openspec/changes/harden-native-build-config/design.md b/openspec/changes/harden-native-build-config/design.md new file mode 100644 index 0000000..a2641db --- /dev/null +++ b/openspec/changes/harden-native-build-config/design.md @@ -0,0 +1,161 @@ +## Context + +This project builds a Node.js native addon using node-gyp with FFmpeg as the codec engine. The build configuration must support: + +- **Platforms:** macOS (arm64, x64), Linux (glibc, musl) +- **FFmpeg resolution:** 4-tier fallback (env var → npm package → local dir → pkg-config) +- **Static linking:** FFmpeg libraries statically linked for distribution +- **Framework linking:** macOS requires system frameworks (VideoToolbox, CoreMedia, etc.) + +**Current Architecture:** +``` +binding.gyp + ├── Sources: 21 C++ files + ├── N-API: v8 with exceptions enabled + ├── macOS: xcode_settings + frameworks + fallback paths + └── Linux: cflags_cc + static linking + -Wl,-Bsymbolic + +gyp/ffmpeg-paths.js → ffmpeg-paths-lib.ts + ├── FFMPEG_ROOT env var (priority 1) + ├── @pproenca/webcodecs-ffmpeg npm package (priority 2) + ├── ./ffmpeg-install directory (priority 3) + └── System pkg-config (fallback) +``` + +**Problem Statement:** + +The audit identified 16 issues in the current configuration. The most critical are: + +1. **Linux has no hardcoded FFmpeg fallback** — macOS has `/opt/homebrew`, `/usr/local`; Linux has nothing +2. **No RPATH configuration** — Prebuilt binaries can't find FFmpeg libs at runtime +3. **Silent error handling** — `ffmpeg-paths.js` only logs with `DEBUG=1` +4. **No output validation** — Empty pkg-config output passes through silently + +## Goals / Non-Goals + +**Goals:** +- Fix all 4 critical issues (P0) that cause build/runtime failures +- Fix 4 high-priority issues (P1) affecting portability +- Add missing best practices (P2) for warnings and build configuration +- Improve CI efficiency (P3) with caching + +**Non-Goals:** +- Windows support (intentionally dropped per project decision) +- Electron support (not currently needed) +- Cross-compilation (handled by CI matrix) +- prebuild/prebuildify integration (using platform packages instead) + +## Decisions + +### 1. Linux Fallback Paths + +**Decision:** Add hardcoded fallback paths for Linux: `/usr/include`, `/usr/local/include`, `/usr/lib`, `/usr/local/lib`. + +**Rationale:** Mirrors macOS behavior. Most Linux distributions with FFmpeg development packages install to these paths. Provides graceful degradation when pkg-config is missing. + +**Alternatives considered:** +- No fallback (current): Requires pkg-config, confusing errors when missing +- Only `/usr/include`: Misses custom installations in `/usr/local` + +### 2. RPATH Configuration + +**Decision:** Add runtime library search paths: +- macOS: `-Wl,-rpath,@loader_path/../lib` +- Linux: `-Wl,-rpath,$ORIGIN/../lib` + +**Rationale:** Prebuilt FFmpeg libraries from npm packages install to `node_modules/@pproenca/webcodecs-ffmpeg-*/lib/`. Without RPATH, the native addon can't find them at runtime. + +**Alternatives considered:** +- Absolute paths only: Breaks when modules move +- LD_LIBRARY_PATH environment variable: Fragile, requires user configuration + +### 3. Explicit Error Logging + +**Decision:** Always log errors to stderr in `ffmpeg-paths-lib.ts`, not just when `DEBUG=1`. + +**Rationale:** Silent failures cause confusing linker errors. Explicit messages ("FFmpeg pkg-config failed: ...") guide users to the solution. + +**Implementation:** Log to stderr (not stdout) to avoid polluting gyp output which expects paths on stdout. + +**Alternatives considered:** +- Keep silent by default: Poor developer experience +- Throw exceptions: Would break fallback chain + +### 4. Static/Dynamic Linking Fallback + +**Decision:** Linux tries `pkg-config --libs --static` first, falls back to `pkg-config --libs` (dynamic). + +**Rationale:** Static linking is preferred for distribution, but dynamic linking allows using system FFmpeg when static libs unavailable. + +**Alternatives considered:** +- Static only (current): Fails on systems with only dynamic FFmpeg +- Dynamic only: Larger binaries, runtime dependency on system FFmpeg version + +### 5. Debug/Release Configurations + +**Decision:** Add explicit `configurations` block with distinct optimization and debug symbol settings. + +**Rationale:** node-gyp has defaults, but explicit configuration ensures: +- Debug builds have `-g -O0` for debugging +- Release builds have `-O3` for performance +- Consistent behavior across node-gyp versions + +### 6. Parallel Compilation + +**Decision:** Add `-j max` flag to all `node-gyp rebuild` invocations. + +**Rationale:** Significantly faster builds on multi-core systems. No downside on single-core. + +### 7. Additional Compiler Warnings + +**Decision:** Add `-Wpedantic` and `-Wshadow` to both platforms. + +**Rationale:** +- `-Wpedantic`: Enforces ISO C++ compliance +- `-Wshadow`: Catches variable shadowing bugs + +**Not adding:** `-Wconversion` — Too noisy with FFmpeg types, would require extensive casts. + +## Risks / Trade-offs + +**[Risk] RPATH breaks existing deployments** +→ Mitigation: RPATH is additive; existing absolute path resolution still works. Test on all CI platforms. + +**[Risk] New warnings surface existing code issues** +→ Mitigation: Review warnings before enabling. May require code fixes. + +**[Risk] Dynamic FFmpeg fallback links wrong version** +→ Mitigation: Document that static is preferred, dynamic is fallback. pkg-config returns compatible version. + +**[Risk] Stderr logging confuses users who expect silence** +→ Mitigation: Clear message prefix `[node-webcodecs]`. Only on actual errors. + +## Migration Plan + +1. **Phase 1 (P0):** Critical fixes — Must be deployed together + - Linux fallback paths + - RPATH configuration + - Explicit error logging + - Output validation + +2. **Phase 2 (P1):** Reliability improvements — Can be incremental + - Dynamic linking fallback + - MacPorts paths + - pkg-config existence check + +3. **Phase 3 (P2):** Best practices — Low risk + - Compiler warnings + - Configurations block + - Parallel compilation + +4. **Phase 4 (P3):** Enhancements — Optional + - CI caching + - rpath mode implementation + +**Rollback:** All changes are in configuration files. Revert commits to restore previous behavior. + +## Open Questions + +1. Should we add architecture-specific compiler flags (NEON for ARM, SSE/AVX for x86)? +2. Should we support cross-compilation scenarios (e.g., building Linux x64 on arm64)? +3. Should we add prebuildify integration for automated binary distribution? diff --git a/openspec/changes/harden-native-build-config/proposal.md b/openspec/changes/harden-native-build-config/proposal.md new file mode 100644 index 0000000..fbb5162 --- /dev/null +++ b/openspec/changes/harden-native-build-config/proposal.md @@ -0,0 +1,56 @@ +# Change: Harden Native Build Configuration + +## Why + +The `binding.gyp` and FFmpeg path resolution system has 16 identified issues ranging from critical build failures to best-practice gaps. A comprehensive audit revealed: + +- **4 critical issues** causing silent build failures or runtime crashes +- **4 high-priority issues** affecting reliability and portability +- **4 medium-priority issues** missing best practices +- **4 low-priority enhancements** for CI efficiency + +Key problems: +1. Linux builds fail silently if FFmpeg is missing (no fallback paths like macOS) +2. Runtime library resolution fails without RPATH configuration +3. Errors are suppressed by default in `ffmpeg-paths.js` +4. No validation that pkg-config returns non-empty output + +## What Changes + +### Critical Fixes (P0) +- Add Linux FFmpeg include/library fallback paths +- Make `ffmpeg-paths.js` errors explicit (always log to stderr) +- Add RPATH configuration for macOS (`@loader_path`) and Linux (`$ORIGIN`) +- Validate pkg-config output is non-empty before processing + +### High Priority (P1) +- Add dynamic linking fallback for Linux (when `--static` fails) +- Add MacPorts support in macOS fallback paths (`/opt/local`) +- Add pkg-config existence check before using it + +### Medium Priority (P2) +- Add missing compiler warnings (`-Wpedantic`, `-Wshadow`) +- Add explicit Debug/Release `configurations` block +- Enable parallel compilation (`-j max`) by default + +### Low Priority (P3) +- Add node-gyp header caching in CI +- Implement `rpath` mode in `ffmpeg-paths.js` + +## Impact + +- **Affected specs:** Creates new `native-build` capability spec (none exists) +- **Affected code:** + - `binding.gyp` — Platform conditions, RPATH, warnings, configurations + - `gyp/ffmpeg-paths-lib.ts` — Error logging, validation, pkg-config check + - `package.json` — Build script flags + - `.github/workflows/ci.yml` — Caching configuration + +## Risk Assessment + +| Change | Risk | Mitigation | +|--------|------|------------| +| RPATH addition | May affect existing builds | Test on all CI platforms | +| Explicit error logging | May break silent fallback behavior | Log to stderr only, not stdout | +| Static→dynamic fallback | May link wrong FFmpeg version | Document precedence clearly | +| Additional warnings | May surface new warnings | Review before enabling | diff --git a/openspec/changes/harden-native-build-config/specs/native-build/spec.md b/openspec/changes/harden-native-build-config/specs/native-build/spec.md new file mode 100644 index 0000000..93e68c6 --- /dev/null +++ b/openspec/changes/harden-native-build-config/specs/native-build/spec.md @@ -0,0 +1,129 @@ +## ADDED Requirements + +### Requirement: FFmpeg Path Resolution + +The build system SHALL resolve FFmpeg include and library paths using a prioritized fallback chain. + +#### Scenario: Resolution via FFMPEG_ROOT environment variable +- **WHEN** the `FFMPEG_ROOT` environment variable is set +- **AND** `$FFMPEG_ROOT/lib/pkgconfig` exists +- **THEN** FFmpeg paths SHALL be resolved from that location + +#### Scenario: Resolution via npm package +- **WHEN** `FFMPEG_ROOT` is not set +- **AND** `@pproenca/webcodecs-ffmpeg-{platform}-{arch}` package is installed +- **THEN** FFmpeg paths SHALL be resolved from the npm package location + +#### Scenario: Resolution via local directory +- **WHEN** neither env var nor npm package is available +- **AND** `./ffmpeg-install/lib/pkgconfig` exists +- **THEN** FFmpeg paths SHALL be resolved from the local directory + +#### Scenario: Resolution via system pkg-config +- **WHEN** none of the above sources are available +- **AND** `pkg-config` is installed and FFmpeg .pc files are discoverable +- **THEN** FFmpeg paths SHALL be resolved from system pkg-config + +#### Scenario: Fallback to hardcoded paths +- **WHEN** all resolution methods fail +- **THEN** the build system SHALL use platform-specific hardcoded paths: + - macOS: `/opt/homebrew/include`, `/usr/local/include`, `/opt/local/include` + - Linux: `/usr/include`, `/usr/local/include` + +### Requirement: Error Reporting + +The build system SHALL report FFmpeg resolution errors explicitly to stderr. + +#### Scenario: pkg-config failure +- **WHEN** pkg-config execution fails +- **THEN** an error message SHALL be logged to stderr with prefix `[node-webcodecs]` +- **AND** the message SHALL include the failure reason +- **AND** the message SHALL suggest ensuring FFmpeg 5.0+ is installed + +#### Scenario: Empty pkg-config output +- **WHEN** pkg-config returns empty output +- **THEN** an error message SHALL be logged to stderr +- **AND** resolution SHALL fall back to the next method in the chain + +### Requirement: Runtime Library Path Configuration + +The build system SHALL configure runtime library search paths (RPATH) for the native addon. + +#### Scenario: macOS RPATH configuration +- **WHEN** building on macOS +- **THEN** the native addon SHALL be linked with `-Wl,-rpath,@loader_path/../lib` +- **AND** the native addon SHALL be linked with `-Wl,-rpath,@loader_path/../../ffmpeg-install/lib` + +#### Scenario: Linux RPATH configuration +- **WHEN** building on Linux +- **THEN** the native addon SHALL be linked with `-Wl,-rpath,$ORIGIN/../lib` +- **AND** the native addon SHALL be linked with `-Wl,-rpath,$ORIGIN/../../ffmpeg-install/lib` + +### Requirement: Linux Library Linking Fallback + +The Linux build SHALL support both static and dynamic FFmpeg linking. + +#### Scenario: Static linking preferred +- **WHEN** building on Linux +- **THEN** the build system SHALL first attempt `pkg-config --libs --static` + +#### Scenario: Dynamic linking fallback +- **WHEN** static linking fails on Linux +- **THEN** the build system SHALL fall back to `pkg-config --libs` (dynamic) + +### Requirement: Build Configuration + +The build system SHALL support explicit Debug and Release configurations. + +#### Scenario: Debug configuration +- **WHEN** building with `--debug` flag +- **THEN** the build SHALL use `-g -O0` compiler flags +- **AND** the build SHALL define `DEBUG` and `_DEBUG` preprocessor macros + +#### Scenario: Release configuration +- **WHEN** building without `--debug` flag (default) +- **THEN** the build SHALL use `-O3` optimization +- **AND** the build SHALL define `NDEBUG` preprocessor macro + +### Requirement: Parallel Compilation + +The build system SHALL enable parallel compilation by default. + +#### Scenario: Default parallel build +- **WHEN** running `npm run build:native` +- **THEN** node-gyp SHALL be invoked with `-j max` flag +- **AND** all available CPU cores SHALL be utilized + +### Requirement: Compiler Warnings + +The build system SHALL enable comprehensive compiler warnings. + +#### Scenario: Warning flags on macOS +- **WHEN** building on macOS +- **THEN** the compiler SHALL use flags: `-Wall -Wextra -Wpedantic -Wshadow -Wno-unused-parameter` + +#### Scenario: Warning flags on Linux +- **WHEN** building on Linux +- **THEN** the compiler SHALL use flags: `-Wall -Wextra -Wpedantic -Wshadow -Wno-unused-parameter` + +### Requirement: pkg-config Availability Check + +The build system SHALL verify pkg-config availability before using it. + +#### Scenario: pkg-config available +- **WHEN** `pkg-config --version` succeeds +- **THEN** pkg-config-based resolution SHALL proceed normally + +#### Scenario: pkg-config unavailable +- **WHEN** `pkg-config --version` fails +- **THEN** pkg-config-based resolution SHALL be skipped +- **AND** the build SHALL fall back to hardcoded paths + +### Requirement: CI Build Caching + +The CI workflow SHALL cache node-gyp headers for faster builds. + +#### Scenario: Cache node-gyp headers +- **WHEN** running CI builds +- **THEN** the `~/.node-gyp` directory SHALL be cached +- **AND** the cache key SHALL include `binding.gyp` hash diff --git a/openspec/changes/harden-native-build-config/tasks.md b/openspec/changes/harden-native-build-config/tasks.md new file mode 100644 index 0000000..ee112c8 --- /dev/null +++ b/openspec/changes/harden-native-build-config/tasks.md @@ -0,0 +1,89 @@ +## 1. Critical Fixes (P0) + +### 1.1 Linux FFmpeg Fallback +- [x] 1.1.1 Add fallback include paths to `binding.gyp:83-84` (`/usr/include /usr/local/include`) +- [x] 1.1.2 Add fallback library paths to `binding.gyp:86` (`-L/usr/lib -L/usr/local/lib`) +- [x] 1.1.3 Test build without pkg-config available + +### 1.2 Explicit Error Logging +- [x] 1.2.1 Update `gyp/ffmpeg-paths-lib.ts:116-122` to always log errors to stderr +- [x] 1.2.2 Add `[node-webcodecs]` prefix to error messages +- [x] 1.2.3 Add helpful message suggesting FFmpeg 5.0+ installation +- [x] 1.2.4 Rebuild gyp scripts (`npm run build:scripts`) + +### 1.3 RPATH Configuration +- [x] 1.3.1 Add macOS RPATH to `binding.gyp` xcode_settings OTHER_LDFLAGS +- [x] 1.3.2 Add Linux RPATH to `binding.gyp` ldflags array +- [x] 1.3.3 Verify RPATH with `otool -l` (macOS) / `readelf -d` (Linux) + +### 1.4 Output Validation +- [x] 1.4.1 Add empty output check in `runPkgConfig()` function +- [x] 1.4.2 Log warning when pkg-config returns empty string +- [x] 1.4.3 Rebuild and test with mocked empty output + +## 2. High Priority (P1) + +### 2.1 Dynamic Linking Fallback +- [x] 2.1.1 Update Linux library resolution to try `--static` first, then without +- [x] 2.1.2 Test with FFmpeg dynamic-only installation + +### 2.2 MacPorts Support +- [x] 2.2.1 Add `/opt/local/include` to macOS include fallback paths +- [x] 2.2.2 Add `/opt/local/lib` to macOS library fallback paths + +### 2.3 pkg-config Existence Check +- [x] 2.3.1 Add `isPkgConfigAvailable()` helper function to `ffmpeg-paths-lib.ts` +- [x] 2.3.2 Check availability before attempting pkg-config resolution +- [x] 2.3.3 Log clear message when pkg-config is unavailable + +## 3. Medium Priority (P2) + +### 3.1 Compiler Warnings +- [x] 3.1.1 Add `-Wpedantic` and `-Wshadow` to macOS OTHER_CPLUSPLUSFLAGS +- [x] 3.1.2 Add `-Wpedantic` and `-Wshadow` to Linux cflags_cc +- [x] 3.1.3 Fix any new warnings that surface +- [x] 3.1.4 Verify clean build on both platforms + +### 3.2 Configurations Block +- [x] 3.2.1 Add `configurations` block to binding.gyp with Debug settings +- [x] 3.2.2 Add Release settings to configurations block +- [x] 3.2.3 Test `npm run build:debug` produces debug symbols +- [x] 3.2.4 Test default build produces optimized output + +### 3.3 Parallel Compilation +- [x] 3.3.1 Update `package.json` build:native script with `-j max` +- [x] 3.3.2 Update build script with `-j max` +- [x] 3.3.3 Verify parallel compilation works + +## 4. Low Priority (P3) + +### 4.1 CI Caching +- [x] 4.1.1 Add node-gyp cache configuration to `.github/workflows/ci.yml` +- [x] 4.1.2 Use cache key based on `binding.gyp` hash +- [x] 4.1.3 Test CI with and without cache + +### 4.2 rpath Mode Implementation +- [x] 4.2.1 Implement `rpath` mode in `gyp/ffmpeg-paths-lib.ts` +- [x] 4.2.2 Return FFmpeg lib directory path +- [x] 4.2.3 Update compiled `ffmpeg-paths.js` + +## 5. Verification + +### 5.1 Build Verification +- [x] 5.1.1 Run `npm run build` (clean rebuild) +- [x] 5.1.2 Run `npm run build:debug` (debug build) +- [ ] 5.1.3 Run `npm run build -- --enable_sanitizers=1` (sanitizers) + +### 5.2 Test Verification +- [x] 5.2.1 Run `npm run check` (lint + test) +- [ ] 5.2.2 Run `npm run test:native` (C++ tests) + +### 5.3 Platform Verification +- [x] 5.3.1 Verify linked libraries with `otool -L` (macOS) or `ldd` (Linux) +- [x] 5.3.2 Verify RPATH with `otool -l | grep -A2 LC_RPATH` (macOS) +- [ ] 5.3.3 Verify RPATH with `readelf -d | grep RUNPATH` (Linux) + +### 5.4 FFmpeg Resolution Verification +- [x] 5.4.1 Test with `DEBUG=1 node gyp/ffmpeg-paths.js lib` +- [ ] 5.4.2 Test with `FFMPEG_ROOT=/path/to/ffmpeg node gyp/ffmpeg-paths.js lib` +- [x] 5.4.3 Test with pkg-config unavailable (verify fallback) diff --git a/openspec/changes/introduce-ci-workflow/.openspec.yaml b/openspec/changes/introduce-ci-workflow/.openspec.yaml new file mode 100644 index 0000000..c094fba --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-01-11 diff --git a/openspec/changes/introduce-ci-workflow/design.md b/openspec/changes/introduce-ci-workflow/design.md new file mode 100644 index 0000000..6898439 --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/design.md @@ -0,0 +1,168 @@ +## Context + +This project is a Node.js native addon providing W3C WebCodecs API using FFmpeg. It requires: + +- C++17 compilation with node-gyp +- FFmpeg 5.0+ libraries (libavcodec, libavformat, libavutil, libswscale, libswresample, libavfilter) +- Platform-specific frameworks (VideoToolbox on macOS) +- Multiple libc variants on Linux (glibc, musl) + +**FFmpeg Dependency Resolution (`gyp/ffmpeg-paths-lib.ts`) - Current:** +1. `FFMPEG_ROOT` env var → Explicit override +2. `./ffmpeg-install` directory → Local development +3. System pkg-config → Fallback to system FFmpeg + +**FFmpeg Dependency Resolution - After This Change:** +1. `FFMPEG_ROOT` env var → Explicit override +2. `@pproenca/webcodecs-ffmpeg` npm package → **NEW: Auto-installed via optionalDeps** +3. `./ffmpeg-install` directory → Local development +4. System pkg-config → Fallback to system FFmpeg + +**Related Repository: `pproenca/webcodecs-ffmpeg`** + +This separate repository builds and publishes **FFmpeg static libraries** to NPM: +- `@pproenca/webcodecs-ffmpeg` - Main package (LGPL: VP8/9, AV1, Opus, Vorbis, MP3) +- `@pproenca/webcodecs-ffmpeg-non-free` - GPL variant (adds H.264, H.265) +- Platform packages: `*-darwin-arm64`, `*-darwin-x64`, `*-linux-arm64`, `*-linux-x64` + +The build workflow (`_build.yml`) produces 8 artifacts (4 platforms × 2 licenses) that are published via `release.yml` on git tags. + +**This Project's Prebuild Packages:** + +`node-webcodecs` publishes its own native addon prebuilds as optionalDependencies: +- `@pproenca/node-webcodecs-darwin-arm64` +- `@pproenca/node-webcodecs-darwin-x64` +- `@pproenca/node-webcodecs-linux-x64-glibc` +- `@pproenca/node-webcodecs-linux-x64-musl` + +Currently, there is no CI. Code quality is enforced manually via `npm run check`. This CI focuses on validation (lint, build, test) - prebuild publishing is a separate concern. + +## Goals / Non-Goals + +**Goals:** + +- Automate lint checks (C++, TypeScript, types, markdown) on every push/PR +- Build and test native addon on all supported platforms +- Fail fast on code quality issues before merge +- Provide clear feedback on build/test failures + +**Non-Goals:** + +- Native addon prebuild publishing (separate concern, possibly via `prebuildify`) +- Windows support (not currently in optionalDependencies) +- ARM64 Linux builds (not currently in optionalDependencies) +- Release automation or npm publishing +- Building FFmpeg from source (use existing packages) + +## Decisions + +### 1. Workflow Structure: Separate Lint and Build Jobs + +**Decision:** Use two separate jobs - `lint` and `build-native`. + +**Rationale:** Lint checks are fast and platform-independent. Running them in a dedicated job provides quick feedback without waiting for slow native builds. Build failures don't block lint feedback. + +**Alternatives considered:** +- Single job with all steps: Slower feedback, wastes resources if lint fails +- Matrix including lint: Redundant lint runs across platforms + +### 2. Linux Build Strategy: Container-based with Rocky and Alpine + +**Decision:** Use official `rockylinux:8` container for glibc builds and `alpine:3.20` for musl builds. + +**Rationale:** +- Rocky Linux 8 provides glibc 2.28, compatible with most Linux distributions +- Alpine provides musl libc for lightweight container deployments +- Containers ensure reproducible builds independent of runner updates + +**Alternatives considered:** +- Ubuntu runner directly: Higher glibc version limits compatibility +- Custom Docker images: Maintenance burden, slower cold starts + +### 3. macOS Strategy: Native Runners with Homebrew FFmpeg + +**Decision:** Use `macos-14` (arm64) and `macos-13` (x64) runners with Homebrew-installed FFmpeg. + +**Rationale:** +- GitHub provides both architectures natively +- Homebrew FFmpeg is well-maintained and includes hardware acceleration support +- No container overhead on macOS + +**Alternatives considered:** +- Pre-built FFmpeg binaries: Version management complexity +- Building FFmpeg from source: Slow, maintenance burden + +### 4. Node.js Version Strategy: Test Against Supported Engines + +**Decision:** Test on Node.js 20 and 22 (matching `engines` field: `^20.17.0 || ^22.9.0 || >=24`). + +**Rationale:** These are the LTS versions currently supported. Node 24 is too new for stable CI. + +**Alternatives considered:** +- Test all three versions: Node 24 may have unstable N-API, adds matrix complexity +- Single version only: Misses version-specific issues + +### 5. FFmpeg Installation: System Package Managers + +**Decision:** Use system package managers - `dnf` on Rocky, `apk` on Alpine, `brew` on macOS. + +**Rationale:** +- Simpler setup, no artifact management +- Consistent with how end users install FFmpeg +- Package managers handle dependencies automatically +- Matches the fallback path in `gyp/ffmpeg-paths-lib.ts` (system pkg-config) + +**Alternatives considered:** +- Use `@pproenca/webcodecs-ffmpeg` npm packages: More consistent with production builds, but adds complexity (npm install, extract, set `FFMPEG_ROOT`). Better suited for prebuild CI, not validation CI. +- Download artifacts from webcodecs-ffmpeg releases: Requires GitHub token, artifact management. Overkill for validation. +- Build FFmpeg from source: Too slow, maintenance burden + +### 6. Test Scope: Full Test Suite Excluding Stress Tests + +**Decision:** Run `npm run test:fast` and `npm run test:guardrails` but not `npm run test:stress`. + +**Rationale:** +- Fast tests and guardrails catch regressions without excessive CI time +- Stress tests are resource-intensive and may timeout on shared runners + +### 7. Optional FFmpeg Dependencies (Sharp-style) + +**Decision:** Add `@pproenca/webcodecs-ffmpeg-*` platform packages as `optionalDependencies` in package.json, and update `gyp/ffmpeg-paths-lib.ts` to resolve from npm packages before falling back to system FFmpeg. + +**Rationale:** +- Developers get FFmpeg automatically on `npm install` (zero-config) +- Developers can opt-out with `--omit=optional` to use system FFmpeg +- Matches Sharp's pattern for optional native dependencies +- CI uses `--omit=optional` to validate the system fallback works + +**Implementation:** +1. Add to package.json `optionalDependencies`: + - `@pproenca/webcodecs-ffmpeg-darwin-arm64` + - `@pproenca/webcodecs-ffmpeg-darwin-x64` + - `@pproenca/webcodecs-ffmpeg-linux-x64` +2. Update `gyp/ffmpeg-paths-lib.ts` to check for `@pproenca/webcodecs-ffmpeg/resolve` +3. CI runs `npm ci --omit=optional` to ensure system fallback is tested + +**Alternatives considered:** +- Required dependency: Forces all users to download FFmpeg even if system version available +- No npm FFmpeg: Requires all users to install FFmpeg manually (worse DX) + +## Risks / Trade-offs + +**[Risk] FFmpeg version drift across platforms** → Pin FFmpeg version where possible (e.g., `ffmpeg-7` on Alpine). Document minimum version in workflow comments. + +**[Risk] macOS runner availability/cost** → GitHub provides free macOS minutes for public repos. For private repos, consider running macOS tests only on PRs to main. + +**[Risk] Container startup time** → Rocky and Alpine images are small. First-run caches layers. Acceptable trade-off for reproducibility. + +**[Risk] Flaky tests on shared runners** → Use `--test-concurrency=1` (already in npm scripts). Consider retry strategy if issues emerge. + +**[Risk] webcodecs-ffmpeg version mismatch** → Keep optionalDependencies versions in sync with tested FFmpeg version. Document minimum compatible version. + +**[Risk] npm package resolution complexity** → The resolve module must handle missing packages gracefully. Test both paths (with and without optional deps). + +## Open Questions + +1. Should we add a code coverage reporting step? (e.g., upload to Codecov) +2. Should we cache node_modules and FFmpeg installations for faster runs? +3. Should we add a step to verify the prebuild packages install correctly? diff --git a/openspec/changes/introduce-ci-workflow/proposal.md b/openspec/changes/introduce-ci-workflow/proposal.md new file mode 100644 index 0000000..cbc0f16 --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/proposal.md @@ -0,0 +1,43 @@ +## Why + +This project currently has no CI workflow. Without automated testing, lint failures and regressions can slip into the codebase. A CI workflow ensures code quality gates are enforced on every push and pull request, and enables automated cross-platform native addon builds. + +The project relies on FFmpeg libraries, which are available via: +1. System package managers (for local dev and CI) +2. `@pproenca/webcodecs-ffmpeg` npm packages (pre-built static FFmpeg libraries) + +This CI workflow focuses on testing and validation. Prebuild publishing for `@pproenca/node-webcodecs-*` packages is a separate concern. + +## What Changes + +**CI Workflow:** +- Add `.github/workflows/ci.yml` with automated checks on push and pull request +- Lint job: C++ linting (`cpplint`), TypeScript linting (`biome`), type checking (`tsd`), markdown formatting (`prettier`) +- Build and test job: Multi-platform matrix (Linux x64 glibc/musl, macOS x64/arm64) +- Native addon compilation using `node-gyp` with FFmpeg dependencies +- Test execution for unit tests, golden tests, and guardrails +- Container-based Linux builds for consistent glibc/musl environments + +**Optional FFmpeg Dependencies (Sharp-style):** +- Add `@pproenca/webcodecs-ffmpeg` platform packages as `optionalDependencies` +- Update `gyp/ffmpeg-paths-lib.ts` to resolve FFmpeg from npm packages first +- Developers can use `--ignore-optional` to skip and use system FFmpeg instead + +## Capabilities + +### New Capabilities + +- `ci-workflow`: GitHub Actions workflow for automated linting, building, and testing across platforms +- `optional-ffmpeg-deps`: Optional FFmpeg dependencies allowing developers to use system FFmpeg or npm packages + +### Modified Capabilities + +(none - no existing specs are modified) + +## Impact + +- **Code**: New `.github/workflows/ci.yml` file, updated `gyp/ffmpeg-paths-lib.ts` +- **Dependencies**: New `optionalDependencies` for `@pproenca/webcodecs-ffmpeg-*` packages +- **External**: GitHub Actions integration; prebuild publishing remains in `pproenca/webcodecs-ffmpeg` repo +- **Platforms**: macOS (x64, arm64), Linux (x64 glibc via Rocky, x64 musl via Alpine) +- **Developer Experience**: `npm install` gets FFmpeg automatically; `npm install --ignore-optional` uses system FFmpeg diff --git a/openspec/changes/introduce-ci-workflow/specs/ci-workflow/spec.md b/openspec/changes/introduce-ci-workflow/specs/ci-workflow/spec.md new file mode 100644 index 0000000..84b4400 --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/specs/ci-workflow/spec.md @@ -0,0 +1,133 @@ +## ADDED Requirements + +### Requirement: CI workflow triggers on push and pull request + +The CI workflow SHALL execute on every push to `master` branch and on every pull request targeting `master`. + +#### Scenario: Push to master triggers CI + +- **WHEN** a commit is pushed to the `master` branch +- **THEN** the CI workflow executes all jobs + +#### Scenario: Pull request triggers CI + +- **WHEN** a pull request is opened or updated targeting `master` +- **THEN** the CI workflow executes all jobs + +### Requirement: Lint job validates code quality + +The CI workflow SHALL include a lint job that runs C++ linting, TypeScript linting, type checking, and markdown formatting checks. + +#### Scenario: Lint job runs all checks + +- **WHEN** the lint job executes +- **THEN** the following checks run in sequence: + - `npm run lint:cpp` (cpplint) + - `npm run lint:ts` (biome) + - `npm run lint:types` (tsd) + - `npm run lint:md` (prettier) + +#### Scenario: Lint failure blocks merge + +- **WHEN** any lint check fails +- **THEN** the workflow reports failure and the PR cannot be merged + +### Requirement: Build job compiles native addon on Linux glibc + +The CI workflow SHALL build the native addon on Linux x64 with glibc using a Rocky Linux 8 container. + +#### Scenario: Rocky Linux build with FFmpeg + +- **WHEN** the build job runs on Linux glibc matrix entry +- **THEN** the job: + - Uses `rockylinux:8` container + - Installs FFmpeg development packages via `dnf` + - Runs `npm run build` + - Runs `npm test` + +### Requirement: Build job compiles native addon on Linux musl + +The CI workflow SHALL build the native addon on Linux x64 with musl libc using an Alpine container. + +#### Scenario: Alpine Linux build with FFmpeg + +- **WHEN** the build job runs on Linux musl matrix entry +- **THEN** the job: + - Uses `alpine:3.20` container + - Installs FFmpeg development packages via `apk` + - Runs `npm run build` + - Runs `npm test` + +### Requirement: Build job compiles native addon on macOS ARM64 + +The CI workflow SHALL build the native addon on macOS ARM64 (Apple Silicon) using a native runner. + +#### Scenario: macOS ARM64 build with Homebrew FFmpeg + +- **WHEN** the build job runs on macOS ARM64 matrix entry +- **THEN** the job: + - Uses `macos-14` runner + - Installs FFmpeg via Homebrew + - Runs `npm run build` + - Runs `npm test` + +### Requirement: Build job compiles native addon on macOS x64 + +The CI workflow SHALL build the native addon on macOS x64 (Intel) using a native runner. + +#### Scenario: macOS x64 build with Homebrew FFmpeg + +- **WHEN** the build job runs on macOS x64 matrix entry +- **THEN** the job: + - Uses `macos-13` runner + - Installs FFmpeg via Homebrew + - Runs `npm run build` + - Runs `npm test` + +### Requirement: Build job tests multiple Node.js versions + +The CI workflow SHALL test against Node.js 20 and 22 on each platform. + +#### Scenario: Matrix includes Node.js versions + +- **WHEN** the build matrix is configured +- **THEN** each platform entry includes Node.js versions `20` and `22` + +### Requirement: Test execution includes fast tests and guardrails + +The CI workflow SHALL run the standard test suite excluding stress tests. + +#### Scenario: Test commands execute + +- **WHEN** tests run in the build job +- **THEN** the following commands execute: + - `npm run test:fast` (unit and golden tests) + - `npm run test:guardrails` (fuzzer and event loop lag tests) + +#### Scenario: Stress tests are excluded + +- **WHEN** tests run in the build job +- **THEN** `npm run test:stress` is NOT executed + +### Requirement: Workflow uses minimal permissions + +The CI workflow SHALL request only the minimum required GitHub permissions. + +#### Scenario: Default permissions are restricted + +- **WHEN** the workflow file is parsed +- **THEN** the top-level `permissions` block sets `contents: read` only + +### Requirement: Lint job provides fast feedback + +The lint job SHALL run independently and complete before build jobs finish, providing quick feedback on code quality issues. + +#### Scenario: Lint job runs in parallel with builds + +- **WHEN** the workflow executes +- **THEN** the lint job runs concurrently with build jobs (no `needs` dependency) + +#### Scenario: Lint job completes quickly + +- **WHEN** the lint job executes +- **THEN** it completes in under 2 minutes on a standard runner diff --git a/openspec/changes/introduce-ci-workflow/specs/optional-ffmpeg-deps/spec.md b/openspec/changes/introduce-ci-workflow/specs/optional-ffmpeg-deps/spec.md new file mode 100644 index 0000000..fdc7d19 --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/specs/optional-ffmpeg-deps/spec.md @@ -0,0 +1,79 @@ +## ADDED Requirements + +### Requirement: FFmpeg packages are optional dependencies + +The package.json SHALL declare `@pproenca/webcodecs-ffmpeg` platform packages as `optionalDependencies`, allowing developers to skip them with `--omit=optional`. + +#### Scenario: Standard install includes FFmpeg packages + +- **WHEN** a developer runs `npm install` +- **THEN** the appropriate platform-specific FFmpeg package is installed (e.g., `@pproenca/webcodecs-ffmpeg-darwin-arm64` on Apple Silicon) + +#### Scenario: Install with ignore-optional skips FFmpeg + +- **WHEN** a developer runs `npm install --omit=optional` +- **THEN** no `@pproenca/webcodecs-ffmpeg-*` packages are installed +- **AND** the build falls back to system FFmpeg via pkg-config + +### Requirement: FFmpeg resolution prefers npm packages + +The `gyp/ffmpeg-paths-lib.ts` SHALL resolve FFmpeg in this order: +1. `FFMPEG_ROOT` environment variable (explicit override) +2. `@pproenca/webcodecs-ffmpeg` npm package (if installed) +3. `./ffmpeg-install` directory (local development) +4. System pkg-config (fallback) + +#### Scenario: Resolve from npm package when installed + +- **WHEN** `@pproenca/webcodecs-ffmpeg` is installed +- **AND** no `FFMPEG_ROOT` env var is set +- **THEN** FFmpeg paths resolve from the npm package's lib/pkgconfig directory + +#### Scenario: Fallback to system when npm package missing + +- **WHEN** `@pproenca/webcodecs-ffmpeg` is NOT installed +- **AND** no `FFMPEG_ROOT` env var is set +- **AND** no `./ffmpeg-install` directory exists +- **THEN** FFmpeg paths resolve from system pkg-config + +### Requirement: Platform packages follow naming convention + +The optionalDependencies SHALL include platform-specific packages matching the pattern `@pproenca/webcodecs-ffmpeg-{os}-{arch}`. + +#### Scenario: Supported platforms are declared + +- **WHEN** package.json is parsed +- **THEN** optionalDependencies includes: + - `@pproenca/webcodecs-ffmpeg-darwin-arm64` + - `@pproenca/webcodecs-ffmpeg-darwin-x64` + - `@pproenca/webcodecs-ffmpeg-linux-x64` + +### Requirement: Resolution uses webcodecs-ffmpeg resolve module + +The `gyp/ffmpeg-paths-lib.ts` SHALL use `@pproenca/webcodecs-ffmpeg/resolve` to locate the FFmpeg installation when the npm package is available. + +#### Scenario: Resolve module provides paths + +- **WHEN** resolving FFmpeg from npm package +- **THEN** the code requires `@pproenca/webcodecs-ffmpeg/resolve` +- **AND** uses its `pkgconfig` property to get the pkg-config directory + +#### Scenario: Graceful fallback when resolve fails + +- **WHEN** `require('@pproenca/webcodecs-ffmpeg/resolve')` throws +- **THEN** resolution continues to the next fallback (./ffmpeg-install or system) + +### Requirement: CI uses system FFmpeg not npm packages + +The CI workflow SHALL NOT install FFmpeg via npm optionalDependencies. It SHALL use system package managers to ensure the build works without the npm packages. + +#### Scenario: CI installs with ignore-optional + +- **WHEN** the CI workflow runs `npm install` or `npm ci` +- **THEN** it uses `--omit=optional` flag +- **AND** FFmpeg is installed via system package manager (dnf, apk, brew) + +#### Scenario: CI validates system FFmpeg fallback works + +- **WHEN** CI builds complete successfully +- **THEN** this proves the system FFmpeg fallback path works correctly diff --git a/openspec/changes/introduce-ci-workflow/tasks.md b/openspec/changes/introduce-ci-workflow/tasks.md new file mode 100644 index 0000000..75899ea --- /dev/null +++ b/openspec/changes/introduce-ci-workflow/tasks.md @@ -0,0 +1,77 @@ +## 1. Optional FFmpeg Dependencies + +- [x] 1.1 Add `@pproenca/webcodecs-ffmpeg-darwin-arm64` to optionalDependencies in package.json +- [x] 1.2 Add `@pproenca/webcodecs-ffmpeg-darwin-x64` to optionalDependencies in package.json +- [x] 1.3 Add `@pproenca/webcodecs-ffmpeg-linux-x64` to optionalDependencies in package.json +- [x] 1.4 Update `gyp/ffmpeg-paths-lib.ts` to check for `@pproenca/webcodecs-ffmpeg/resolve` before other fallbacks +- [x] 1.5 Add try/catch in resolution to gracefully handle missing npm package +- [x] 1.6 Compile `gyp/ffmpeg-paths-lib.ts` to verify no TypeScript errors + +## 2. CI Workflow Setup + +- [x] 2.1 Create `.github/workflows/` directory if it doesn't exist +- [x] 2.2 Create `.github/workflows/ci.yml` with workflow name and triggers (push to master, PR to master) +- [x] 2.3 Set minimal permissions (`contents: read`) + +## 3. Lint Job + +- [x] 3.1 Add `lint` job running on `ubuntu-24.04` +- [x] 3.2 Add checkout step +- [x] 3.3 Add Node.js setup step (version 22) +- [x] 3.4 Add `npm ci --ignore-optional` step +- [x] 3.5 Add `npm run lint:cpp` step +- [x] 3.6 Add `npm run lint:ts` step +- [x] 3.7 Add `npm run lint:types` step +- [x] 3.8 Add `npm run lint:md` step + +## 4. Build Matrix Configuration + +- [x] 4.1 Define matrix strategy with platform entries (linux-glibc, linux-musl, macos-arm64, macos-x64) +- [x] 4.2 Add Node.js version matrix (20, 22) +- [x] 4.3 Configure container images for Linux entries (rockylinux:8, alpine:3.20) +- [x] 4.4 Configure native runners for macOS entries (macos-14, macos-13) + +## 5. Linux glibc Build (Rocky Linux 8) + +- [x] 5.1 Add FFmpeg installation via `dnf install -y ffmpeg-free-devel` +- [x] 5.2 Add build dependencies (gcc-c++, make, python3, pkg-config) +- [x] 5.3 Add Node.js installation step appropriate for Rocky Linux container +- [x] 5.4 Add `npm ci --ignore-optional` step +- [x] 5.5 Add `npm run build` step +- [x] 5.6 Add `npm test` step + +## 6. Linux musl Build (Alpine 3.20) + +- [x] 6.1 Add FFmpeg installation via `apk add ffmpeg-dev` +- [x] 6.2 Add build dependencies (build-base, python3, pkgconfig) +- [x] 6.3 Add Node.js installation step appropriate for Alpine container +- [x] 6.4 Add `npm ci --ignore-optional` step +- [x] 6.5 Add `npm run build` step +- [x] 6.6 Add `npm test` step + +## 7. macOS ARM64 Build + +- [x] 7.1 Configure job to run on `macos-14` runner +- [x] 7.2 Add FFmpeg installation via `brew install ffmpeg` +- [x] 7.3 Add Node.js setup action +- [x] 7.4 Add `npm ci --ignore-optional` step +- [x] 7.5 Add `npm run build` step +- [x] 7.6 Add `npm test` step + +## 8. macOS x64 Build + +- [x] 8.1 Configure job to run on `macos-13` runner +- [x] 8.2 Add FFmpeg installation via `brew install ffmpeg` +- [x] 8.3 Add Node.js setup action +- [x] 8.4 Add `npm ci --ignore-optional` step +- [x] 8.5 Add `npm run build` step +- [x] 8.6 Add `npm test` step + +## 9. Validation + +- [x] 9.1 Run workflow locally with `act` to validate syntax (if available) +- [x] 9.2 Push to branch and verify workflow triggers on PR (https://github.com/pproenca/node-webcodecs/pull/10) +- [ ] 9.3 Verify lint job completes in under 2 minutes +- [ ] 9.4 Verify all matrix entries build and test successfully +- [x] 9.5 Test `npm install` locally includes FFmpeg optional dependency +- [x] 9.6 Test `npm install --omit=optional` locally falls back to system FFmpeg diff --git a/package.json b/package.json index 4362544..61e44b7 100644 --- a/package.json +++ b/package.json @@ -12,11 +12,11 @@ "url": "https://github.com/pproenca/node-webcodecs/issues" }, "scripts": { - "build": "npm run build:scripts && node-gyp rebuild && tsc", - "build:native": "npm run build:scripts && node-gyp rebuild", + "build": "npm run build:scripts && node-gyp rebuild -j max && tsc", + "build:native": "npm run build:scripts && node-gyp rebuild -j max", "build:scripts": "tsc -p tsconfig.gyp.json", "build:ts": "tsc", - "build:debug": "npm run build:scripts && node-gyp rebuild --debug && tsc", + "build:debug": "npm run build:scripts && node-gyp rebuild -j max --debug && tsc", "rebuild": "npm run clean && npm run build", "clean": "rm -rf src/build/ .nyc_output/ coverage/ test/fixtures/output.*", "check": "npm run lint && npm test", @@ -79,7 +79,12 @@ "@pproenca/node-webcodecs-darwin-arm64": "0.1.1-alpha.8", "@pproenca/node-webcodecs-darwin-x64": "0.1.1-alpha.8", "@pproenca/node-webcodecs-linux-x64-glibc": "0.1.1-alpha.8", - "@pproenca/node-webcodecs-linux-x64-musl": "0.1.1-alpha.8" + "@pproenca/node-webcodecs-linux-x64-musl": "0.1.1-alpha.8", + "@pproenca/webcodecs-ffmpeg-darwin-arm64": "^0.1.7", + "@pproenca/webcodecs-ffmpeg-darwin-x64": "^0.1.7", + "@pproenca/webcodecs-ffmpeg-linux-arm64": "^0.1.7", + "@pproenca/webcodecs-ffmpeg-linux-x64": "^0.1.7", + "@pproenca/webcodecs-ffmpeg-linux-x64-musl": "^0.1.7" }, "dependencies": { "detect-libc": "^2.1.2", @@ -88,6 +93,7 @@ "devDependencies": { "@biomejs/biome": "^2.3.10", "@cpplint/cli": "^0.1.0", + "@pproenca/webcodecs-ffmpeg-dev": "^0.1.7", "@types/node": "^25.0.3", "@types/turndown": "^5.0.6", "c8": "^10.1.3", diff --git a/src/common.h b/src/common.h index a7c2fbc..1fb774d 100644 --- a/src/common.h +++ b/src/common.h @@ -26,6 +26,16 @@ extern "C" { #error "FFmpeg 5.0+ (libavcodec 59+) is required" #endif +// AVFrame::duration was added in FFmpeg 5.1 (libavutil 57.28.100) +// Before that, use pkt_duration (deprecated in 5.1+) +#if LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(57, 28, 100) +#define AV_FRAME_DURATION(frame) ((frame)->duration) +#define AV_FRAME_SET_DURATION(frame, val) ((frame)->duration = (val)) +#else +#define AV_FRAME_DURATION(frame) ((frame)->pkt_duration) +#define AV_FRAME_SET_DURATION(frame, val) ((frame)->pkt_duration = (val)) +#endif + namespace webcodecs { //============================================================================== diff --git a/src/image_decoder.cc b/src/image_decoder.cc index be91774..20277b7 100644 --- a/src/image_decoder.cc +++ b/src/image_decoder.cc @@ -546,8 +546,8 @@ bool ImageDecoder::ParseAnimatedImageMetadata() { // Duration from packet or frame int64_t duration = pkt->duration; - if (duration <= 0 && frm->duration > 0) { - duration = frm->duration; + if (duration <= 0 && AV_FRAME_DURATION(frm.get()) > 0) { + duration = AV_FRAME_DURATION(frm.get()); } if (duration > 0) { decoded_frame.duration = diff --git a/src/video_encoder.cc b/src/video_encoder.cc index 2d029d4..fbc2e83 100644 --- a/src/video_encoder.cc +++ b/src/video_encoder.cc @@ -526,7 +526,7 @@ Napi::Value VideoEncoder::Encode(const Napi::CallbackInfo& info) { frame->height = video_frame->GetHeight(); frame->format = AV_PIX_FMT_RGBA; frame->pts = video_frame->GetTimestampValue(); - frame->duration = video_frame->GetDurationValue(); + AV_FRAME_SET_DURATION(frame.get(), video_frame->GetDurationValue()); // Copy RGBA data size_t data_size = frame->width * frame->height * kBytesPerPixelRgba; diff --git a/src/video_encoder_worker.cc b/src/video_encoder_worker.cc index 09da099..dd4dbb9 100644 --- a/src/video_encoder_worker.cc +++ b/src/video_encoder_worker.cc @@ -341,7 +341,7 @@ void VideoEncoderWorker::OnEncode(const EncodeMessage& msg) { int src_width = src_frame->width; int src_height = src_frame->height; int64_t timestamp = src_frame->pts; - int64_t duration = src_frame->duration; + int64_t duration = AV_FRAME_DURATION(src_frame); // For RGBA input, convert to YUV420P // The source frame contains RGBA data packed in data[0] diff --git a/test/unit/scripts.test.ts b/test/unit/scripts.test.ts index 4f88e4f..c656e42 100644 --- a/test/unit/scripts.test.ts +++ b/test/unit/scripts.test.ts @@ -77,14 +77,19 @@ test('filterFrameworkFlags removes framework pairs', () => { assert.strictEqual(filterFrameworkFlags(input), '-L/foo -lavcodec'); }); -test('getFfmpegRoot resolves ffmpeg-install when present', () => { +test('getFfmpegRoot resolves npm package or ffmpeg-install', () => { const root = createTempRoot(); const pkgconfig = join(root, 'ffmpeg-install', 'lib', 'pkgconfig'); mkdirSync(pkgconfig, {recursive: true}); const result = getFfmpegRoot(root, {}); assert.ok(result); - assert.strictEqual(result?.root, join(root, 'ffmpeg-install')); + // npm package takes precedence over ffmpeg-install when installed + // Either npm package or ffmpeg-install is acceptable + assert.ok( + result?.root.includes('ffmpeg-install') || result?.root.includes('webcodecs-ffmpeg'), + `Expected ffmpeg-install or npm package, got: ${result?.root}` + ); }); test('platform-package main returns error for unknown mode', () => {