Skip to content

feat(cmd): include output directories in JSON results #173

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 1 commit into
base: main
Choose a base branch
from
Open

feat(cmd): include output directories in JSON results #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions cmd/llar/internal/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func install(ctx context.Context, progress io.Writer, serviceURL, arg string, ma
matrixStr := matrix.Combinations()[0]

var rootResult moduleOutputResult
deps := make([]module.Version, 0, len(messages)-1)
deps := make([]moduleOutputDep, 0, len(messages)-1)
for _, message := range messages {
parsed, err := url.Parse(message.ID)
if err != nil {
Expand Down Expand Up @@ -184,7 +184,7 @@ func install(ctx context.Context, progress io.Writer, serviceURL, arg string, ma
OutputDir: installDir,
}
} else {
deps = append(deps, mod)
deps = append(deps, moduleOutputDep{Module: mod, OutputDir: installDir})
}
}
rootResult.Deps = deps
Expand Down
21 changes: 15 additions & 6 deletions cmd/llar/internal/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ func TestInstallCommand(t *testing.T) {
if result.Path != "test/root" || result.Version != "v1.0.0" {
t.Fatalf("result = %+v, want test/root@v1.0.0", result)
}
installDir := filepath.Join(workspaceDir, "test/root@v1.0.0-testarch-testos|ON")
if result.Dir != installDir {
t.Fatalf("result dir = %q, want %q", result.Dir, installDir)
}
if _, err := os.Stat(output); err != nil {
t.Fatalf("output artifact: %v", err)
}
Expand All @@ -156,7 +160,6 @@ func TestInstallCommand(t *testing.T) {
if archiveInfo.Metadata != "-I"+filepath.Join(extracted, "include")+" -lroot" {
t.Fatalf("archive metadata = %+v", archiveInfo)
}
installDir := filepath.Join(workspaceDir, "test/root@v1.0.0-testarch-testos|ON")
assertInstallFile(t, filepath.Join(installDir, "include", "root.h"), "root")

directoryOutput := filepath.Join(t.TempDir(), "root-out")
Expand Down Expand Up @@ -312,7 +315,12 @@ func TestInstallDownloadsRootAndDependencies(t *testing.T) {
if result.Module != (module.Version{Path: "test/root", Version: "v1.0.0"}) {
t.Fatalf("result module = %+v", result.Module)
}
if len(result.Deps) != 1 || result.Deps[0] != (module.Version{Path: "test/dep", Version: "v1.2.3"}) {
depDir := filepath.Join(workspaceDir, fmt.Sprintf("test/dep@v1.2.3-%s", matrixStr))
rootDir := filepath.Join(workspaceDir, fmt.Sprintf("test/root@v1.0.0-%s", matrixStr))
if result.OutputDir != rootDir {
t.Fatalf("result output dir = %q, want %q", result.OutputDir, rootDir)
}
if len(result.Deps) != 1 || result.Deps[0].Module != (module.Version{Path: "test/dep", Version: "v1.2.3"}) || result.Deps[0].OutputDir != depDir {
t.Fatalf("result deps = %+v", result.Deps)
}
if result.Metadata != "-I"+filepath.Join(result.OutputDir, "include")+" -lroot" {
Expand All @@ -325,8 +333,6 @@ func TestInstallDownloadsRootAndDependencies(t *testing.T) {
t.Fatalf("llard artifact requests = %d, want one", artifactRequests)
}

depDir := filepath.Join(workspaceDir, fmt.Sprintf("test/dep@v1.2.3-%s", matrixStr))
rootDir := filepath.Join(workspaceDir, fmt.Sprintf("test/root@v1.0.0-%s", matrixStr))
assertInstallFile(t, filepath.Join(depDir, "lib", "libdep.a"), "dep")
assertInstallFile(t, filepath.Join(rootDir, "include", "root.h"), "root")
assertInstallCache(t, workspaceDir, "test/dep", "v1.2.3", matrixStr, "-L"+filepath.Join(depDir, "lib")+" -ldep")
Expand Down Expand Up @@ -355,9 +361,12 @@ func TestInstallDownloadsRootAndDependencies(t *testing.T) {
if err := json.Unmarshal(jsonOutput.Bytes(), &jsonResult); err != nil {
t.Fatal(err)
}
if jsonResult.Path != "test/root" || jsonResult.Version != "v1.0.0" || len(jsonResult.Deps) != 1 {
if jsonResult.Path != "test/root" || jsonResult.Version != "v1.0.0" || jsonResult.Dir != rootDir || len(jsonResult.Deps) != 1 {
t.Fatalf("JSON result = %+v", jsonResult)
}
if jsonResult.Deps[0].Path != "test/dep" || jsonResult.Deps[0].Version != "v1.2.3" || jsonResult.Deps[0].Dir != depDir {
t.Fatalf("JSON dependency = %+v", jsonResult.Deps[0])
}

}

Expand Down Expand Up @@ -420,7 +429,7 @@ func TestInstallSkipsCachedArtifacts(t *testing.T) {
if result.Module != (module.Version{Path: "test/root", Version: "v1.0.0"}) {
t.Fatalf("result module = %+v", result.Module)
}
if len(result.Deps) != 1 || result.Deps[0] != (module.Version{Path: "test/dep", Version: "v1.2.3"}) {
if len(result.Deps) != 1 || result.Deps[0].Module != (module.Version{Path: "test/dep", Version: "v1.2.3"}) || result.Deps[0].OutputDir != depDir {
t.Fatalf("result deps = %+v", result.Deps)
}
if result.Metadata != "-I"+filepath.Join(result.OutputDir, "include")+" -lroot" {
Expand Down
28 changes: 27 additions & 1 deletion cmd/llar/internal/make.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,35 @@ func buildModule(ctx context.Context, store repo.Store, modPath, version string,

if len(results) > 0 {
main := results[len(results)-1]
deps := artifactDeps(mods)
mainPath, err := module.EscapePath(mods[0].Path)
if err != nil {
return err
}
mainSuffix := fmt.Sprintf("%s@%s-%s", mainPath, mods[0].Version, matrixStr)
mainDir := filepath.Clean(main.OutputDir)
if !strings.HasSuffix(mainDir, mainSuffix) {
return fmt.Errorf("unexpected build output directory %q for %s@%s", mainDir, mods[0].Path, mods[0].Version)
}
workspaceDir := strings.TrimSuffix(mainDir, mainSuffix)
workspaceDir = strings.TrimSuffix(workspaceDir, string(filepath.Separator))
outputDeps := make([]moduleOutputDep, 0, len(deps))
for _, dep := range deps {
depPath, err := module.EscapePath(dep.Path)
if err != nil {
return err
}
outputDeps = append(outputDeps, moduleOutputDep{

@fennoai fennoai Bot Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make.go recomputes each dependency's output dir with filepath.FromSlash(dep.Path), but the builder's canonical install dir (internal/build/cache.go installDir) and the parallel code in install.go:161 both use module.EscapePath (i.e. filepath.Localize).

FromSlash only swaps / for the OS separator, whereas Localize additionally escapes/rejects path elements invalid on the target OS. For ordinary lowercase module paths the two agree, but they diverge for any path requiring localization — in which case the dir reported for a dependency won't match the directory the builder actually created. Note the root module's dir (line 200) correctly comes from main.OutputDir (EscapePath-based), so root and deps use inconsistent schemes within the same command.

Since builder.Build already returns an authoritative OutputDir for every module (root and each dependency), the most robust fix is to source dep dirs from the returned results rather than recomputing them. At minimum, use module.EscapePath(dep.Path) (handling its error) to match install.go and the builder.

Both new tests use lowercase module paths, so they wouldn't catch this divergence.

Module: dep,
OutputDir: filepath.Join(
workspaceDir,
fmt.Sprintf("%s@%s-%s", depPath, dep.Version, matrixStr),
),
})
}
result := moduleOutputResult{
Module: module.Version{Path: mods[0].Path, Version: mods[0].Version},
Deps: artifactDeps(mods),
Deps: outputDeps,
Metadata: main.Metadata,
OutputDir: main.OutputDir,
}
Expand Down
10 changes: 10 additions & 0 deletions cmd/llar/internal/make_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,9 +688,11 @@ func TestMakeLocal_JSONOutput(t *testing.T) {
var got struct {
Path string `json:"path"`
Version string `json:"version"`
Dir string `json:"dir"`
Deps []struct {
Path string `json:"path"`
Version string `json:"version"`
Dir string `json:"dir"`
} `json:"deps"`
Metadata string `json:"metadata"`
}
Expand All @@ -703,6 +705,10 @@ func TestMakeLocal_JSONOutput(t *testing.T) {
if got.Version != "1.0.0" {
t.Fatalf("version = %q, want %q", got.Version, "1.0.0")
}
rootDir := filepath.Join(workspaceDir, fmt.Sprintf("test/jsonroot@1.0.0-%s", matrixStr))
if got.Dir != rootDir {
t.Fatalf("dir = %q, want %q", got.Dir, rootDir)
}
if got.Metadata != "-ljsonroot" {
t.Fatalf("metadata = %q, want %q", got.Metadata, "-ljsonroot")
}
Expand All @@ -712,6 +718,10 @@ func TestMakeLocal_JSONOutput(t *testing.T) {
if got.Deps[0].Path != "test/jsondep" || got.Deps[0].Version != "1.2.3" {
t.Fatalf("deps[0] = %+v, want test/jsondep@1.2.3", got.Deps[0])
}
depDir := filepath.Join(workspaceDir, fmt.Sprintf("test/jsondep@1.2.3-%s", matrixStr))
if got.Deps[0].Dir != depDir {
t.Fatalf("deps[0].dir = %q, want %q", got.Deps[0].Dir, depDir)
}
}

func TestMakeLocal_VerboseWritesBuildOutputToStderr(t *testing.T) {
Expand Down
22 changes: 19 additions & 3 deletions cmd/llar/internal/module_output.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,25 @@ import (
type moduleJSONDep struct {
Path string `json:"path"`
Version string `json:"version"`
Dir string `json:"dir"`
}

type moduleJSONResult struct {
Path string `json:"path"`
Version string `json:"version"`
Dir string `json:"dir"`
Deps []moduleJSONDep `json:"deps,omitempty"`
Metadata string `json:"metadata"`
}

type moduleOutputDep struct {
Module module.Version
OutputDir string
}

type moduleOutputResult struct {
Module module.Version
Deps []module.Version
Deps []moduleOutputDep
Metadata string
OutputDir string
}
Expand All @@ -42,11 +49,16 @@ func writeModuleResult(output io.Writer, result moduleOutputResult, jsonOutput b

deps := make([]moduleJSONDep, 0, len(result.Deps))
for _, dep := range result.Deps {
deps = append(deps, moduleJSONDep{Path: dep.Path, Version: dep.Version})
deps = append(deps, moduleJSONDep{
Path: dep.Module.Path,
Version: dep.Module.Version,
Dir: dep.OutputDir,
})
}
return json.NewEncoder(output).Encode(moduleJSONResult{
Path: result.Module.Path,
Version: result.Module.Version,
Dir: result.OutputDir,
Deps: deps,
Metadata: result.Metadata,
})
Expand All @@ -71,7 +83,11 @@ func writeModuleOutput(result moduleOutputResult, dest string) error {
if !strings.HasSuffix(dest, ".zip") && !strings.HasSuffix(dest, ".tar.gz") {
return os.CopyFS(dest, os.DirFS(result.OutputDir))
}
body, err := metadata.Encode(metadata.Info{Metadata: result.Metadata, Deps: result.Deps}, result.OutputDir)
deps := make([]module.Version, 0, len(result.Deps))
for _, dep := range result.Deps {
deps = append(deps, dep.Module)
}
body, err := metadata.Encode(metadata.Info{Metadata: result.Metadata, Deps: deps}, result.OutputDir)
if err != nil {
return err
}
Expand Down
Loading