Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scanner/cargofallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func buildFileGraphFromOutcomeWithCargoMetadataAndFilters(ctx context.Context, r
return nil, err
}
applyPrecomputedFileEdges(fg, outcome.precomputedEdges)
fg.sortEdges()
return fg, nil
}

Expand Down
53 changes: 53 additions & 0 deletions scanner/deterministic_edges_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package scanner

import (
"context"
"reflect"
"sort"
"testing"
)

// Edges are appended in analysis order, which the scanner does not fix, so the
// same repository scanned twice produced the same edges in a different
// sequence. That made --importers output shift between identical runs and made
// an exact-list assertion impossible for any caller.
func TestFileGraphEdgeOrderIsDeterministic(t *testing.T) {
const runs = 8
var first *FileGraph
for run := 0; run < runs; run++ {
graph, err := BuildFileGraph(context.Background(), "../testdata/deterministic-edges", Filters{})
if err != nil {
t.Fatalf("run %d: build graph: %v", run, err)
}
if first == nil {
first = graph
continue
}
if !reflect.DeepEqual(graph.Importers, first.Importers) {
t.Fatalf("run %d importers = %v, want the same order as run 0 %v", run, graph.Importers, first.Importers)
}
if !reflect.DeepEqual(graph.Imports, first.Imports) {
t.Fatalf("run %d imports = %v, want the same order as run 0 %v", run, graph.Imports, first.Imports)
}
// Imports keep resolution order, so they must stay stable without
// being sorted — the CUE resolver's selected-package-first ordering
// depends on it.
}

// Sorted, not merely stable: a caller asserting an exact list needs to
// know which order it will get.
got := first.Importers["lib/shared.ts"]
want := append([]string(nil), got...)
sort.Strings(want)
if !reflect.DeepEqual(got, want) {
t.Fatalf("importers = %v, want them sorted %v", got, want)
}
if len(got) != 4 {
t.Fatalf("importers = %v, want all four importers of lib/shared.ts", got)
}
}

func TestSortEdgesHandlesNilGraph(t *testing.T) {
var graph *FileGraph
graph.sortEdges() // must not panic
}
37 changes: 36 additions & 1 deletion scanner/filegraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"os"
"path/filepath"
"sort"
"strings"

"codemap/analysis"
Expand Down Expand Up @@ -248,9 +249,30 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context,
if err := ctx.Err(); err != nil {
return nil, err
}
fg.sortEdges()
return fg, nil
}

// sortEdges orders the reverse edge lists. Importers are appended while
// iterating analyses, whose order the scanner does not fix, so the same
// repository scanned twice produced the same importers in a different
// sequence: --importers output shifted between identical runs, diffs of
// codemap output showed changes that were not changes, and no caller could
// assert an exact list.
//
// Imports are deliberately left alone. They are appended per file in
// resolution order, which is already stable and which callers rely on: the
// CUE resolver returns a selected package before the package it falls back
// to, and DepsProject sorts its own copy for JSON output anyway.
func (fg *FileGraph) sortEdges() {
if fg == nil {
return
}
for file := range fg.Importers {
sort.Strings(fg.Importers[file])
}
}

func applyPrecomputedFileEdges(fg *FileGraph, edges []fileEdge) {
for _, edge := range edges {
if edge.from == "" || edge.to == "" || edge.from == edge.to {
Expand Down Expand Up @@ -713,14 +735,27 @@ func (fg *FileGraph) IsHub(path string) bool {
return CountHubImporters(fg.Importers[path]) >= HubThreshold
}

// HubFiles returns all files that qualify as hubs under IsHub.
// HubFiles returns all files that qualify as hubs under IsHub, ordered by
// non-test importer count descending and then by path. Map iteration order is
// random, so callers that truncate or display the head of this list would
// otherwise show a different set of hubs on every run over the same graph.
func (fg *FileGraph) HubFiles() []string {
var hubs []string
for path := range fg.Importers {
if fg.IsHub(path) {
hubs = append(hubs, path)
}
}
counts := make(map[string]int, len(hubs))
for _, path := range hubs {
counts[path] = CountHubImporters(fg.Importers[path])
}
sort.Slice(hubs, func(i, j int) bool {
if counts[hubs[i]] != counts[hubs[j]] {
return counts[hubs[i]] > counts[hubs[j]]
}
return hubs[i] < hubs[j]
})
return hubs
}

Expand Down
25 changes: 25 additions & 0 deletions scanner/filegraph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,31 @@ func TestDetectModule(t *testing.T) {
}
}

func TestHubFilesOrderIsDeterministic(t *testing.T) {
fg := &FileGraph{
Importers: map[string][]string{
"core.go": {"a.go", "b.go", "c.go", "d.go", "e.go"},
"util.go": {"a.go", "b.go", "c.go", "d.go"},
"alpha.go": {"a.go", "b.go", "c.go"},
"beta.go": {"a.go", "b.go", "c.go"},
"gamma.go": {"a.go", "b.go", "c.go"},
"delta.go": {"a.go", "b.go", "c.go"},
"lonely.go": {"a.go"},
// Test importers never count toward hub status, so this file must
// not appear no matter how many test files import it.
"testonly.go": {"a_test.go", "b_test.go", "c_test.go", "d_test.go"},
},
}

want := []string{"core.go", "util.go", "alpha.go", "beta.go", "delta.go", "gamma.go"}
for i := 0; i < 12; i++ {
got := fg.HubFiles()
if !reflect.DeepEqual(got, want) {
t.Fatalf("HubFiles() call %d = %v, want %v", i+1, got, want)
}
}
}

func TestFileGraphHubAndConnectedFiles(t *testing.T) {
fg := &FileGraph{
Imports: map[string][]string{
Expand Down
43 changes: 43 additions & 0 deletions scanner/rustcargo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,49 @@ func TestCargoMetadataSharesOneScanDeadline(t *testing.T) {
}
}

func TestCargoMetadataDeadlinePreservesFallbackTopology(t *testing.T) {
root := t.TempDir()
writeRustCargoFixture(t, root, map[string]string{
"Cargo.toml": "[workspace]\nmembers = [\"one\", \"two\"]\n",
"one/Cargo.toml": cargoTestManifest("one"),
"one/src/lib.rs": "mod local;\n",
"one/src/local.rs": "",
"two/Cargo.toml": cargoTestManifest("two"),
"two/src/lib.rs": "",
})
analyses := []FileAnalysis{
{Path: "one/src/lib.rs", Language: "rust", Imports: []string{"local"}},
{Path: "one/src/local.rs", Language: "rust"},
{Path: "two/src/lib.rs", Language: "rust"},
}
files := []FileInfo{
{Path: "one/src/lib.rs"},
{Path: "one/src/local.rs"},
{Path: "two/src/lib.rs"},
}

index, outcome, err := buildRustWorkspaceIndexWithTimeout(
context.Background(), root, analyses, files,
func(ctx context.Context, _ string) ([]byte, error) {
<-ctx.Done()
return nil, ctx.Err()
},
time.Millisecond,
)
if err != nil {
t.Fatalf("buildRustWorkspaceIndexWithTimeout() error: %v", err)
}
if outcome == nil || outcome.Status != ScanSourceFallback {
t.Fatalf("metadata outcome = %#v, want fallback", outcome)
}
if pkg, ok := index.packageForFile("one/src/local.rs"); !ok || pkg.root != "one" {
t.Fatalf("fallback package = %#v, ok %v, want one", pkg, ok)
}
if pkg, ok := index.packageForFile("two/src/lib.rs"); !ok || pkg.root != "two" {
t.Fatalf("fallback package = %#v, ok %v, want two", pkg, ok)
}
}

func TestCargoMetadataRecoversLocalCargoTopology(t *testing.T) {
tests := []struct {
name string
Expand Down
3 changes: 3 additions & 0 deletions testdata/deterministic-edges/app/alpha.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { shared } from "../lib/shared";

export function alpha(): string { return shared(); }
3 changes: 3 additions & 0 deletions testdata/deterministic-edges/app/bravo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { shared } from "../lib/shared";

export function bravo(): string { return shared(); }
3 changes: 3 additions & 0 deletions testdata/deterministic-edges/app/charlie.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { shared } from "../lib/shared";

export function charlie(): string { return shared(); }
3 changes: 3 additions & 0 deletions testdata/deterministic-edges/app/delta.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { shared } from "../lib/shared";

export function delta(): string { return shared(); }
1 change: 1 addition & 0 deletions testdata/deterministic-edges/lib/shared.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export function shared(): string { return "s"; }
2 changes: 1 addition & 1 deletion watch/graph_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const (
graphLifecycleSkippedSize = GraphLifecycleSkippedSize
graphLifecycleFailed = GraphLifecycleFailed

graphBuilderRevision = "filegraph-v1"
graphBuilderRevision = "filegraph-v2"
graphCacheLegacy = "legacy"
graphCacheRootMismatch = "root_mismatch"
graphCacheFilterMismatch = "filter_mismatch"
Expand Down
42 changes: 42 additions & 0 deletions watch/graph_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package watch

import (
"context"
"encoding/json"
"errors"
"os"
"path/filepath"
Expand All @@ -16,6 +17,47 @@ import (
"github.com/fsnotify/fsnotify"
)

// A state file written by a binary from before importers were sorted carries
// builder revision "filegraph-v1". Its edge lists are in map-iteration order,
// so reusing it would keep serving non-deterministic answers; the revision bump
// must force a rebuild.
func TestPreSortStateFileIsNotReused(t *testing.T) {
if graphBuilderRevision == "filegraph-v1" {
t.Fatalf("graphBuilderRevision is still %q; bump it so pre-sort caches are rebuilt", graphBuilderRevision)
}

root := t.TempDir()
cfg := config.ProjectConfig{}
current := newGraphState(root, cfg, graphLifecycleAvailable, time.Unix(10, 0), []string{"dep.go", "main.go"})
fresh := State{
Graph: &current,
Imports: map[string][]string{"main.go": {"dep.go"}},
Importers: map[string][]string{"dep.go": {"main.go"}},
}
configuredCount := 2
fresh.ConfiguredFileCount = &configuredCount
fresh.Coverage.Status = analysis.CoverageComplete

if graph, reason := ValidateCachedGraph(&fresh, root, cfg); graph == nil || reason != "" {
t.Fatalf("current-revision cache = %#v, %q; want it reused", graph, reason)
}

payload, err := json.Marshal(fresh)
if err != nil {
t.Fatal(err)
}
var onDisk State
if err := json.Unmarshal(payload, &onDisk); err != nil {
t.Fatal(err)
}
onDisk.Graph.BuilderRevision = "filegraph-v1"

graph, reason := ValidateCachedGraph(&onDisk, root, cfg)
if graph != nil || reason != graphCacheRevisionMismatch {
t.Fatalf("filegraph-v1 state file = %#v, %q; want nil, %q", graph, reason, graphCacheRevisionMismatch)
}
}

func TestGraphProvenanceValidation(t *testing.T) {
root := t.TempDir()
cfg := config.ProjectConfig{Only: []string{"go", "rust"}, Exclude: []string{"vendor", "generated"}}
Expand Down
Loading