Propagate build info from BAR to VMR outputs #4192

dkurepa · 2024-11-28T10:10:23Z

#4166
#4165

premun

I think this is much better and less invasive!

Just left some minor comments

src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs

src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs

premun · 2024-11-28T14:00:41Z

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs

@@ -76,8 +76,7 @@ public async Task<ILocalGitRepo> PrepareVmrAsync(
            cancellationToken);

        _vmrInfo.VmrPath = vmr.Path;
-        await _dependencyTracker.InitializeSourceMappings();
-        _sourceManifest.Refresh(_vmrInfo.SourceManifestPath);
+        await _dependencyTracker.RefreshMetadata();


Did you look in other usages of InitializeSourceMappings whether they can also change to this?

From what I saw we could use it almost everywhere, but it'd do some extra work that's not needed

I think it might be for the better if we just remove InitializeSourceMappings and just have RefreshMetadata (or maybe name it LoadMetadata so that we can be always sure we load everything always (and in future, if we add more metadata for instance)

I think we will need to be able to override the sourceMappingPath too

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs

premun · 2024-11-28T14:03:20Z

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs

+                var build = (await barClient.GetBuildsAsync(dependency.RepoUri, dependency.RepoUri))
+                        .SingleOrDefault();


I wonder if FirstOrDefault will be better than nothing. Thoughts?

FirstOrDefault could be incorrect tho, in cases where we have multiple builds from the same commit. I'm not sure if we should try to get the newest build or do something like that tho. Currently, if this does happen, we'll still do the code flow, but won't have the OfficialBuildId, or the BarId. This could be a topic for the meeting on tuesday

I somehow still think it's better to have an approximately good information than nothing since the OfficialBuildId is required there for SB purposes.

Since we don't get the TargetVersion from BAR, I'll add back the method that we used to calculate the OfficialBuildId in cases where we don't have it from BAR. Think that will be more accurate than a 50/50 if we get a correct Bar build or not. Also think some logging might be good here

+1 for logging

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs

premun

When reviewing this, I realized a problem I should have probably thought about earlier.

When we will start synchronizing the VMR, the VMR sync pipeline will trigger at the same time as the official SDK pipeline. Which means the VMR sync will happen before a BAR ID for the official build is created. That means that no official build ID will be found for the uri/sha combination.

Let's talk about it tomorrow.

src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs

premun · 2024-11-28T17:23:18Z

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs

@@ -76,8 +76,7 @@ public async Task<ILocalGitRepo> PrepareVmrAsync(
            cancellationToken);

        _vmrInfo.VmrPath = vmr.Path;
-        await _dependencyTracker.InitializeSourceMappings();
-        _sourceManifest.Refresh(_vmrInfo.SourceManifestPath);
+        await _dependencyTracker.RefreshMetadata();


I think it might be for the better if we just remove InitializeSourceMappings and just have RefreshMetadata (or maybe name it LoadMetadata so that we can be always sure we load everything always (and in future, if we add more metadata for instance)

premun · 2024-11-28T17:23:48Z

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs

@@ -76,8 +76,7 @@ public async Task<ILocalGitRepo> PrepareVmrAsync(
            cancellationToken);

        _vmrInfo.VmrPath = vmr.Path;
-        await _dependencyTracker.InitializeSourceMappings();
-        _sourceManifest.Refresh(_vmrInfo.SourceManifestPath);
+        await _dependencyTracker.RefreshMetadata();


I think we will need to be able to override the sourceMappingPath too

test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs

premun · 2024-11-28T17:31:15Z

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs

+                var build = (await barClient.GetBuildsAsync(dependency.RepoUri, dependency.RepoUri))
+                        .SingleOrDefault();


I somehow still think it's better to have an approximately good information than nothing since the OfficialBuildId is required there for SB purposes.

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs

…owTest.cs Co-authored-by: Přemek Vysoký <[email protected]>

Co-authored-by: Přemek Vysoký <[email protected]>

dkurepa · 2024-11-29T09:12:09Z

When reviewing this, I realized a problem I should have probably thought about earlier.

When we will start synchronizing the VMR, the VMR sync pipeline will trigger at the same time as the official SDK pipeline. Which means the VMR sync will happen before a BAR ID for the official build is created. That means that no official build ID will be found for the uri/sha combination.

Let's talk about it tomorrow.

I think if I add the fallback mechanism for the OfficialBuildId, it will be good enough for now. The sdk repo won't have a build, so it won't have a BarId, but with the fallback I'll put back, it will have the official BuildId. And then other repos should be fine, because we'll find them recursively and find their bar builds. In case we don't it will still be at least as good as before

…mrInfo2

premun

Looks nice!

dkurepa added 6 commits November 28, 2024 11:00

Add OfficialBuildId

c9af9db

Merge remote-tracking branch 'source/main' into dkurepa/VmrInfo2

4412556

Also include barId in source manifest

bf6d7bf

fix maestro stufF

d3f2a7f

actually fix it

43f60ce

Revert how we get the token in scenario tests

a083de0

dkurepa changed the title ~~[Not ready yet] Propagate build info from BAR to VMR outputs~~ Propagate build info from BAR to VMR outputs Nov 28, 2024

premun reviewed Nov 28, 2024

View reviewed changes

Fix build, resolve comments

77399ae

premun reviewed Nov 28, 2024

View reviewed changes

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs Outdated Show resolved Hide resolved

dkurepa added 4 commits November 28, 2024 16:51

small fix

fdd9130

another small fix

c3ea63a

fix failing test

089043d

add comment

923ca3e

premun reviewed Nov 28, 2024

View reviewed changes

dkurepa and others added 2 commits November 29, 2024 10:09

Update test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackfl…

7250962

…owTest.cs Co-authored-by: Přemek Vysoký <[email protected]>

Update src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs

5bfa298

Co-authored-by: Přemek Vysoký <[email protected]>

dkurepa added 2 commits November 29, 2024 10:39

Bring back the original officialBuildId calc mechanism as a fallback

ed975a0

Merge remote-tracking branch 'origin/dkurepa/VmrInfo2' into dkurepa/V…

7666913

…mrInfo2

premun approved these changes Nov 29, 2024

View reviewed changes

dkurepa merged commit 2bd1d95 into dotnet:main Nov 29, 2024
9 checks passed

dotnet-eng-status bot mentioned this pull request Nov 29, 2024

Build failed: arcade-services-internal-ci/main #20241120.1 #4169

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate build info from BAR to VMR outputs #4192

Propagate build info from BAR to VMR outputs #4192

dkurepa commented Nov 28, 2024 •

edited

Loading

premun left a comment

premun Nov 28, 2024

dkurepa Nov 28, 2024

premun Nov 28, 2024

premun Nov 28, 2024

premun Nov 28, 2024

dkurepa Nov 28, 2024

premun Nov 28, 2024

dkurepa Nov 29, 2024

premun Nov 29, 2024

premun left a comment •

edited

Loading

premun Nov 28, 2024

premun Nov 28, 2024

premun Nov 28, 2024

dkurepa commented Nov 29, 2024

premun left a comment

		var build = (await barClient.GetBuildsAsync(dependency.RepoUri, dependency.RepoUri))
		.SingleOrDefault();

Propagate build info from BAR to VMR outputs #4192

Propagate build info from BAR to VMR outputs #4192

Conversation

dkurepa commented Nov 28, 2024 • edited Loading

premun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

premun left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkurepa commented Nov 29, 2024

premun left a comment

Choose a reason for hiding this comment

dkurepa commented Nov 28, 2024 •

edited

Loading

premun left a comment •

edited

Loading