Skip to content

fix(modules): preserve selected matrix on loaded formulas #169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MeteorsLiu wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(modules): preserve selected matrix on loaded formulas #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d64bb22
fix(modules): preserve matrix on loaded formulas
MeteorsLiu Aug 21, 2026
e308d7a
refactor(modules): bind matrix to formula context
MeteorsLiu Aug 21, 2026
2761624
refactor(modules): initialize formula module matrix
MeteorsLiu Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions internal/modules/load.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (c *formulaContext) moduleOf(ctx context.Context, modPath string) (*formula
if err != nil {
return nil, err
}
fm := newFormulaModule(fs, modPath)
fm := newFormulaModule(fs, modPath, c.matrix)
actual, _ := c.moduleCache.LoadOrStore(modPath, fm)
return actual.(*formulaModule), nil
}
Expand All @@ -130,7 +130,7 @@ func (c *formulaContext) loadDeps(ctx context.Context, mod module.Version) (deps
if err != nil {
return nil, err
}
return resolveDeps(mod, thisMod.fsys.(fs.ReadFileFS), f, c.matrix)
return resolveDeps(mod, thisMod.fsys.(fs.ReadFileFS), f)
}

// convertToModules converts a list of module.Version into loaded Module structs.
Expand Down Expand Up @@ -192,7 +192,7 @@ func Load(ctx context.Context, main module.Version, opts Options) ([]*Module, er
if err != nil {
return nil, err
}
mainDeps, err := resolveDeps(main, mainMod.fsys.(fs.ReadFileFS), mainFormula, context.matrix)
mainDeps, err := resolveDeps(main, mainMod.fsys.(fs.ReadFileFS), mainFormula)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -287,14 +287,11 @@ func runFormulaHook(fn func()) (err error) {
// resolveDeps resolves the dependencies for a formula.
// It first tries to get dependencies from the OnRequire callback,
// then falls back to parsing versions.json if no dependencies are found.
func resolveDeps(mod module.Version, modFS fs.ReadFileFS, frla *formula.Formula, matrix classfile.Matrix) ([]module.Version, error) {
func resolveDeps(mod module.Version, modFS fs.ReadFileFS, frla *formula.Formula) ([]module.Version, error) {
if err := validateModulePath(mod.Path); err != nil {
return nil, err
}

// XGo formulas read the selected matrix through target.require/options.
// Inject before filter and onRequire so both hooks see the same target.
injectMatrix(frla, matrix)
if frla.Filter != nil {
var supported bool
if err := runFormulaHook(func() {
Expand Down
9 changes: 4 additions & 5 deletions internal/modules/load_coverage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"strings"
"testing"

classfile "github.com/goplus/llar/formula"
"github.com/goplus/llar/internal/formula"
"github.com/goplus/llar/internal/vcs"
"github.com/goplus/llar/mod/module"
Expand Down Expand Up @@ -123,7 +122,7 @@ func TestResolveDeps_InvalidModulePath(t *testing.T) {
mod := module.Version{Path: "", Version: "1.0.0"}
frla := &formula.Formula{ModPath: "", FromVer: "1.0.0"}

_, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
_, err := resolveDeps(mod, modFS, frla)
if err == nil {
t.Fatal("expected error for invalid module path")
}
Expand Down Expand Up @@ -215,7 +214,7 @@ func TestResolveDeps_InvalidDependencyPathFromVersions(t *testing.T) {
mod := module.Version{Path: "towner/main", Version: "1.0.0"}
frla := loadTestFormula(t, "testdata/load/towner/standalone", "towner/standalone", "1.0.0")

_, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
_, err := resolveDeps(mod, modFS, frla)
if err == nil {
t.Fatal("expected error for invalid dependency path")
}
Expand All @@ -229,7 +228,7 @@ func TestResolveDeps_MissingVersionsFile(t *testing.T) {
mod := module.Version{Path: "towner/badcmp", Version: "1.0.0"}
frla := loadTestFormula(t, "testdata/load/towner/standalone", "towner/standalone", "1.0.0")

_, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
_, err := resolveDeps(mod, modFS, frla)
if err == nil {
t.Fatal("expected error for missing versions.json")
}
Expand Down Expand Up @@ -278,7 +277,7 @@ func TestResolveDeps_OnRequireMkdirTempError(t *testing.T) {
modFS := os.DirFS("testdata/load/towner/withreq").(fs.ReadFileFS)
mod := module.Version{Path: "towner/withreq", Version: "1.0.0"}

_, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
_, err := resolveDeps(mod, modFS, frla)
if err == nil {
t.Fatal("expected MkdirTemp error")
}
Expand Down
81 changes: 73 additions & 8 deletions internal/modules/load_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/fs"
"os"
"reflect"
"slices"
"strings"
"testing"
Expand Down Expand Up @@ -53,7 +54,7 @@ func setupTestStore(t *testing.T, testdataDir string) repo.Store {
func loadTestFormula(t *testing.T, moduleDir, modPath, version string) *formula.Formula {
t.Helper()
fsys := os.DirFS(moduleDir)
mod := newFormulaModule(fsys, modPath)
mod := newFormulaModule(fsys, modPath, classfile.Matrix{})
f, err := mod.at(version)
if err != nil {
t.Fatalf("failed to load formula for %s@%s: %v", modPath, version, err)
Expand Down Expand Up @@ -90,7 +91,7 @@ func TestResolveDeps_NoOnRequire_DepsFromVersionsJson(t *testing.T) {
frla := loadTestFormula(t, "testdata/load/towner/mainmod", "towner/mainmod", "1.0.0")
mod := module.Version{Path: "towner/mainmod", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand All @@ -107,7 +108,7 @@ func TestResolveDeps_NoOnRequire_NoDeps(t *testing.T) {
frla := loadTestFormula(t, "testdata/load/towner/leafmod", "towner/leafmod", "1.0.0")
mod := module.Version{Path: "towner/leafmod", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand Down Expand Up @@ -160,7 +161,7 @@ func TestResolveDeps_VersionNotInDepsTable(t *testing.T) {
// Version 9.9.9 doesn't exist in versions.json deps table
mod := module.Version{Path: "towner/mainmod", Version: "9.9.9"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand All @@ -174,7 +175,7 @@ func TestResolveDeps_WithOnRequire_EchoOnly_FallbackToVersionsJson(t *testing.T)
modFS := os.DirFS("testdata/load/towner/withreq").(fs.ReadFileFS)
mod := module.Version{Path: "towner/withreq", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand All @@ -192,7 +193,7 @@ func TestResolveDeps_WithOnRequire_AddsDeps(t *testing.T) {
modFS := os.DirFS("testdata/load/towner/withdeps").(fs.ReadFileFS)
mod := module.Version{Path: "towner/withdeps", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand All @@ -211,7 +212,7 @@ func TestResolveDeps_WithOnRequire_EmptyVersionFallback(t *testing.T) {
modFS := os.DirFS("testdata/load/towner/reqnover").(fs.ReadFileFS)
mod := module.Version{Path: "towner/reqnover", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand All @@ -236,7 +237,7 @@ func TestResolveDeps_WithOnRequire_UnknownDepDropped(t *testing.T) {
modFS := os.DirFS("testdata/load/towner/reqdrop").(fs.ReadFileFS)
mod := module.Version{Path: "towner/reqdrop", Version: "1.0.0"}

deps, err := resolveDeps(mod, modFS, frla, classfile.Matrix{})
deps, err := resolveDeps(mod, modFS, frla)
if err != nil {
t.Fatalf("resolveDeps failed: %v", err)
}
Expand Down Expand Up @@ -455,6 +456,70 @@ func TestLoad_InjectsTargetBeforeFilterAndOnRequire(t *testing.T) {
if findModule(mods, "towner/depmod") == nil {
t.Fatalf("missing towner/depmod in build list")
}

mainMod := findModule(mods, main.Path)
if mainMod == nil || mainMod.Formula == nil {
t.Fatalf("missing formula for %s", main.Path)
}
if mainMod.Formula.OnBuild == nil {
t.Fatal("targetreq OnBuild is nil")
}
buildCtx := classfile.NewContext(&classfile.Project{}, "", "", "", nil)
mainMod.Formula.OnBuild(buildCtx)
if got := buildCtx.Out.Metadata(); got != "-ltargetreq-linux-openssl" {
t.Fatalf("OnBuild metadata = %q, want %q", got, "-ltargetreq-linux-openssl")
}
}

func TestFormulaContext_AtInjectsMatrix(t *testing.T) {
store := setupTestStore(t, "testdata/load")
formulaCtx := newFormulaContext(store.ModuleFS, classfile.Matrix{
Require: map[string][]string{"os": {"linux"}},
})

mod, err := formulaCtx.moduleOf(context.Background(), "towner/targetreq")
if err != nil {
t.Fatalf("moduleOf failed: %v", err)
}
f, err := mod.at("1.0.0")
if err != nil {
t.Fatalf("at failed: %v", err)
}

var deps classfile.ModuleDeps
f.OnRequire(&classfile.Project{}, &deps)
if got := len(deps.Deps()); got != 1 {
t.Fatalf("OnRequire deps = %d, want 1", got)
}
}

func TestConvertToModules_InjectsSelectedMatrix(t *testing.T) {
store := setupTestStore(t, "testdata/load")
matrix := classfile.Matrix{
Require: map[string][]string{"os": {"linux"}},
Options: map[string][]string{"ssl": {"securetransport"}},
}
formulaCtx := newFormulaContext(store.ModuleFS, matrix)

mods, err := formulaCtx.convertToModules(context.Background(), []module.Version{
{Path: "towner/targetreq", Version: "1.0.0"},
})
if err != nil {
t.Fatalf("convertToModules failed: %v", err)
}
if len(mods) != 1 {
t.Fatalf("modules len = %d, want 1", len(mods))
}

formulaElem := reflect.ValueOf(mods[0].Formula).Elem()
structElem := valueOf(formulaElem, "structElem").(reflect.Value)
target := valueOf(structElem, "target").(classfile.Matrix)
if got := target.Require["os"]; !slices.Equal(got, []string{"linux"}) {
t.Errorf("target.require[os] = %v, want [linux]", got)
}
if got := target.Options["ssl"]; !slices.Equal(got, []string{"securetransport"}) {
t.Errorf("target.options[ssl] = %v, want [securetransport]", got)
}
}

func TestLoad_FilterRejectsSelectedMatrix(t *testing.T) {
Expand Down
13 changes: 10 additions & 3 deletions internal/modules/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sync"

"github.com/goplus/ixgo/xgobuild"
classfile "github.com/goplus/llar/formula"
"github.com/goplus/llar/internal/formula"
"github.com/goplus/llar/mod/module"
"github.com/goplus/llar/x/gnu"
Expand All @@ -28,6 +29,7 @@ const defaultComparatorSuffix = "_cmp.gox"
type formulaModule struct {
fsys fs.FS
modPath string
matrix classfile.Matrix
comparator func() (func(v1, v2 module.Version) int, error)

mu sync.Mutex
Expand All @@ -37,10 +39,11 @@ type formulaModule struct {
// newFormulaModule creates a new formulaModule for the given module.
// The fsys should be rooted at the module's directory (already positioned by the caller).
// The modPath is used for constructing module.Version in version comparisons.
func newFormulaModule(fsys fs.FS, modPath string) *formulaModule {
func newFormulaModule(fsys fs.FS, modPath string, matrix classfile.Matrix) *formulaModule {
m := &formulaModule{
fsys: fsys,
modPath: modPath,
matrix: matrix,
formulas: make(map[string]*formula.Formula),
}
m.comparator = sync.OnceValues(func() (func(v1, v2 module.Version) int, error) {
Expand Down Expand Up @@ -82,15 +85,19 @@ func (m *formulaModule) at(version string) (*formula.Formula, error) {
defer m.mu.Unlock()

if f, ok := m.formulas[fromVer]; ok {
return formula.Clone(f), nil
clone := formula.Clone(f)
injectMatrix(clone, m.matrix)
return clone, nil
}
f, err := formula.LoadFS(m.fsys.(fs.ReadFileFS), formulaPath)

if err != nil {
return nil, err
}
m.formulas[fromVer] = f
return formula.Clone(f), nil
clone := formula.Clone(f)
injectMatrix(clone, m.matrix)
return clone, nil
}

// findMaxFromVer finds the formula file with the highest fromVer that is <= the target version.
Expand Down
24 changes: 12 additions & 12 deletions internal/modules/source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

func TestNewFormulaModule(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

if mod == nil {
t.Fatal("newFormulaModule returned nil")
Expand All @@ -38,7 +38,7 @@ func TestNewFormulaModule(t *testing.T) {

func TestFormulaModule_Comparator(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

// First call should load comparator
cmp, err := mod.comparator()
Expand Down Expand Up @@ -76,7 +76,7 @@ func TestFormulaModule_Comparator(t *testing.T) {
func TestFormulaModule_ComparatorDefaultFallback(t *testing.T) {
fsys := os.DirFS("testdata/madler/zlib")
// madler/zlib has no comparator file, should use default
mod := newFormulaModule(fsys, "madler/zlib")
mod := newFormulaModule(fsys, "madler/zlib", classfile.Matrix{})

cmp, err := mod.comparator()
if err != nil {
Expand All @@ -96,7 +96,7 @@ func TestFormulaModule_ComparatorDefaultFallback(t *testing.T) {

func TestFormulaModule_At(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

// Test getting formula for version v1.7.18 (should match fromVer v1.5.0)
f, err := mod.at("v1.7.18")
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestFormulaModule_At(t *testing.T) {
}

func TestFormulaModule_AtReturnsIsolatedClasses(t *testing.T) {
mod := newFormulaModule(os.DirFS("testdata/load/towner/targetreq"), "towner/targetreq")
mod := newFormulaModule(os.DirFS("testdata/load/towner/targetreq"), "towner/targetreq", classfile.Matrix{})

const workers = 32
errs := make(chan error, workers)
Expand Down Expand Up @@ -177,7 +177,7 @@ func TestFormulaModule_AtReturnsIsolatedClasses(t *testing.T) {

func TestFormulaModule_AtVersionMatching(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

tests := []struct {
version string
Expand Down Expand Up @@ -206,7 +206,7 @@ func TestFormulaModule_AtVersionMatching(t *testing.T) {

func TestFormulaModule_AtNoFormula(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

// Version lower than all fromVer should fail
_, err := mod.at("v0.5.0")
Expand All @@ -217,7 +217,7 @@ func TestFormulaModule_AtNoFormula(t *testing.T) {

func TestFormulaModule_AtNonexistentModule(t *testing.T) {
fsys := os.DirFS("testdata/nonexistent")
mod := newFormulaModule(fsys, "nonexistent/module")
mod := newFormulaModule(fsys, "nonexistent/module", classfile.Matrix{})

_, err := mod.at("1.0.0")
if err == nil {
Expand All @@ -227,7 +227,7 @@ func TestFormulaModule_AtNonexistentModule(t *testing.T) {

func TestFormulaModule_FindMaxFromVer(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

cmp, _ := mod.comparator()
target := module.Version{Path: "DaveGamble/cJSON", Version: "v1.7.18"}
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestParseCallArg_NonStringArg(t *testing.T) {

func TestIntegration_FormulaModuleToFormula(t *testing.T) {
fsys := os.DirFS("testdata/DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON")
mod := newFormulaModule(fsys, "DaveGamble/cJSON", classfile.Matrix{})

// Get formula
f, err := mod.at("v1.7.18")
Expand Down Expand Up @@ -477,7 +477,7 @@ func TestIntegration_MultipleModules(t *testing.T) {

for _, m := range modules {
fsys := os.DirFS(m.dir)
mod := newFormulaModule(fsys, m.path)
mod := newFormulaModule(fsys, m.path, classfile.Matrix{})

f, err := mod.at(m.version)
if err != nil {
Expand Down Expand Up @@ -513,7 +513,7 @@ func TestIntegration_RealRepo(t *testing.T) {
t.Fatalf("ModuleFS() failed: %v", err)
}

mod := newFormulaModule(fsys, "madler/zlib")
mod := newFormulaModule(fsys, "madler/zlib", classfile.Matrix{})

// Test comparator (should fall back to GNU version comparison)
cmp, err := mod.comparator()
Expand Down
Loading
Loading