Skip to content

Commit d784314

Browse files
bhanurpbhanurp
authored
Fix inconsistent RequestedBy Paths for .NET Dependencies (#3279)
1 parent e7921b0 commit d784314

File tree

5 files changed

+167
-5
lines changed

5 files changed

+167
-5
lines changed

‎go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,8 @@ replace github.com/docker/docker => github.com/docker/docker v27.5.1+incompatibl
282282

283283
replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 v0.0.0-20230825095122-9bc1711434ab
284284

285+
// replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20251118093509-ebe86db606fa
286+
285287
//replace github.com/jfrog/jfrog-cli-artifactory => github.com/naveenku-jfrog/jfrog-cli-artifactory v0.0.0-20251210184507-0c8d138690cb
286288

287289
//replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.8.9-0.20251006061821-8b1be6a65215

‎nuget_test.go‎

Lines changed: 104 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@ package main
33
import (
44
"encoding/xml"
55
"fmt"
6-
"github.com/jfrog/jfrog-cli-core/v2/utils/ioutils"
7-
"github.com/jfrog/jfrog-client-go/http/httpclient"
8-
"github.com/jfrog/jfrog-client-go/utils/io"
96
"net/http"
107
"os"
118
"os/exec"
@@ -14,6 +11,10 @@ import (
1411
"strings"
1512
"testing"
1613

14+
"github.com/jfrog/jfrog-cli-core/v2/utils/ioutils"
15+
"github.com/jfrog/jfrog-client-go/http/httpclient"
16+
"github.com/jfrog/jfrog-client-go/utils/io"
17+
1718
dotnetUtils "github.com/jfrog/build-info-go/build/utils/dotnet"
1819
buildInfo "github.com/jfrog/build-info-go/entities"
1920
biutils "github.com/jfrog/build-info-go/utils"
@@ -163,9 +164,8 @@ func testNugetCmd(t *testing.T, projectPath, buildName, buildNumber string, expe
163164
}
164165

165166
// Add allow insecure connection for testings to work with localhost server
166-
func allowInsecureConnectionForTests(args *[]string) *[]string {
167+
func allowInsecureConnectionForTests(args *[]string) {
167168
*args = append(*args, "--allow-insecure-connections")
168-
return args
169169
}
170170

171171
func assertNugetDependencies(t *testing.T, module buildInfo.Module, moduleName string) {
@@ -375,3 +375,102 @@ func prepareSetupTest(t *testing.T, packageManager project.ProjectType) func() {
375375
restoreDir()
376376
}
377377
}
378+
379+
// TestDotnetRequestedByDeterminism verifies that the RequestedBy paths in build-info
380+
// are deterministic across multiple runs. This addresses the bug where dependencies
381+
// could flip between "direct" and "transitive" attribution due to non-deterministic
382+
// map iteration order in Go.
383+
//
384+
// The test uses the "determinism" project which has dependencies that are BOTH
385+
// direct AND transitive (e.g., Newtonsoft.Json is a direct dep and also a transitive
386+
// dep of NuGet.Core). This creates multiple RequestedBy paths that must be
387+
// consistently sorted across runs.
388+
func TestDotnetRequestedByDeterminism(t *testing.T) {
389+
initNugetTest(t)
390+
const numRuns = 5
391+
buildName := tests.DotnetBuildName + "-determinism"
392+
393+
// Use "determinism" project which has deps that are both direct AND transitive
394+
projectPath := createNugetProject(t, "determinism")
395+
err := createConfigFileForTest([]string{projectPath}, tests.NugetRemoteRepo, "", t, project.Dotnet, false)
396+
require.NoError(t, err)
397+
398+
wd, err := os.Getwd()
399+
require.NoError(t, err)
400+
chdirCallback := clientTestUtils.ChangeDirWithCallback(t, wd, projectPath)
401+
defer chdirCallback()
402+
403+
// Store RequestedBy maps from each run for comparison
404+
var allRunsRequestedBy []map[string][][]string
405+
406+
for i := 1; i <= numRuns; i++ {
407+
buildNumber := strconv.Itoa(i)
408+
args := []string{dotnetUtils.DotnetCore.String(), "restore"}
409+
allowInsecureConnectionForTests(&args)
410+
args = append(args, "--build-name="+buildName, "--build-number="+buildNumber)
411+
412+
err := runNuGet(t, args...)
413+
require.NoError(t, err, "Run %d failed", i)
414+
415+
// Publish build info
416+
require.NoError(t, artifactoryCli.Exec("bp", buildName, buildNumber))
417+
418+
// Get published build info
419+
publishedBuildInfo, found, err := tests.GetBuildInfo(serverDetails, buildName, buildNumber)
420+
require.NoError(t, err)
421+
require.True(t, found, "Build info not found for run %d", i)
422+
423+
bi := publishedBuildInfo.BuildInfo
424+
require.NotEmpty(t, bi.Modules, "No modules in build info for run %d", i)
425+
426+
// Extract RequestedBy for each dependency
427+
requestedByMap := make(map[string][][]string)
428+
for _, module := range bi.Modules {
429+
for _, dep := range module.Dependencies {
430+
requestedByMap[dep.Id] = dep.RequestedBy
431+
}
432+
}
433+
allRunsRequestedBy = append(allRunsRequestedBy, requestedByMap)
434+
435+
// Clean up this build number
436+
inttestutils.DeleteBuild(serverDetails.ArtifactoryUrl, buildName, artHttpDetails)
437+
}
438+
439+
// Compare all runs - they should be identical
440+
firstRun := allRunsRequestedBy[0]
441+
for i := 1; i < numRuns; i++ {
442+
currentRun := allRunsRequestedBy[i]
443+
444+
// Check that all dependencies have the same RequestedBy paths
445+
for depId, expectedRequestedBy := range firstRun {
446+
actualRequestedBy, exists := currentRun[depId]
447+
assert.True(t, exists, "Dependency %s missing in run %d", depId, i+1)
448+
assert.Equal(t, expectedRequestedBy, actualRequestedBy,
449+
"RequestedBy mismatch for %s between run 1 and run %d.\nExpected: %v\nActual: %v",
450+
depId, i+1, expectedRequestedBy, actualRequestedBy)
451+
}
452+
453+
// Also check for any extra dependencies in current run
454+
for depId := range currentRun {
455+
_, exists := firstRun[depId]
456+
assert.True(t, exists, "Extra dependency %s found in run %d but not in run 1", depId, i+1)
457+
}
458+
}
459+
460+
// Verify that dependencies with multiple paths have direct path first
461+
// Newtonsoft.Json should have direct path ["determinism"] before transitive paths
462+
for depId, requestedBy := range firstRun {
463+
if len(requestedBy) > 1 {
464+
// Direct paths (length 1) should come before transitive paths (length > 1)
465+
for j := 1; j < len(requestedBy); j++ {
466+
assert.LessOrEqual(t, len(requestedBy[j-1]), len(requestedBy[j]),
467+
"RequestedBy paths not sorted by length for %s: path %d has length %d, path %d has length %d",
468+
depId, j-1, len(requestedBy[j-1]), j, len(requestedBy[j]))
469+
}
470+
t.Logf("Dependency %s has %d RequestedBy paths (verified sorted)", depId, len(requestedBy))
471+
}
472+
}
473+
474+
t.Logf("Successfully verified RequestedBy determinism across %d runs", numRuns)
475+
cleanTestsHomeEnv()
476+
}

‎testdata/nuget/determinism/Program.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Test project for RequestedBy determinism fix
2+
// See determinism.csproj for dependency setup details
3+
Console.WriteLine("Determinism test project");
4+

‎testdata/nuget/determinism/determinism.csproj‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<!--
3+
Test project for RequestedBy determinism fix.
4+
5+
This project has dependencies that are BOTH direct AND transitive:
6+
- Newtonsoft.Json: Direct dependency AND transitive (via NuGet.Core)
7+
- Microsoft.CSharp: Direct dependency AND transitive (via NuGet.Core)
8+
9+
This creates multiple RequestedBy paths for these dependencies:
10+
- Direct path: [["determinism"]]
11+
- Transitive path: [["determinism", "NuGet.Core"]]
12+
13+
The fix ensures these paths are always sorted consistently:
14+
1. Direct paths first (shorter length)
15+
2. Lexicographically sorted within same length
16+
-->
17+
<Project Sdk="Microsoft.NET.Sdk">
18+
19+
<PropertyGroup>
20+
<OutputType>Exe</OutputType>
21+
<TargetFramework>net6.0</TargetFramework>
22+
<ImplicitUsings>enable</ImplicitUsings>
23+
<Nullable>enable</Nullable>
24+
</PropertyGroup>
25+
26+
<ItemGroup>
27+
<!-- Direct dependency - also a transitive dep of NuGet.Core -->
28+
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
29+
30+
<!-- This package depends on Newtonsoft.Json transitively -->
31+
<PackageReference Include="NuGet.Core" Version="2.14.0" />
32+
33+
<!-- Additional packages for more complex graph -->
34+
<PackageReference Include="RestSharp" Version="110.2.0" />
35+
</ItemGroup>
36+
37+
</Project>
38+

‎testdata/nuget/determinism/determinism.sln‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
Microsoft Visual Studio Solution File, Format Version 12.00
2+
# Visual Studio Version 17
3+
VisualStudioVersion = 17.0.31903.59
4+
MinimumVisualStudioVersion = 10.0.40219.1
5+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "determinism", "determinism.csproj", "{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}"
6+
EndProject
7+
Global
8+
GlobalSection(SolutionConfigurationPlatforms) = preSolution
9+
Debug|Any CPU = Debug|Any CPU
10+
Release|Any CPU = Release|Any CPU
11+
EndGlobalSection
12+
GlobalSection(ProjectConfigurationPlatforms) = postSolution
13+
{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
14+
{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Debug|Any CPU.Build.0 = Debug|Any CPU
15+
{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Release|Any CPU.ActiveCfg = Release|Any CPU
16+
{D1FFA0DC-0ACC-4108-ADC1-2A71122C09AF}.Release|Any CPU.Build.0 = Release|Any CPU
17+
EndGlobalSection
18+
EndGlobal
19+

0 commit comments

Comments
 (0)