-
Notifications
You must be signed in to change notification settings - Fork 609
Basic implementation of build cache. #8754
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.
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
base: main
Are you sure you want to change the base?
Conversation
ArgumentNullException.ThrowIfNull(runner); | ||
ArgumentNullException.ThrowIfNull(interactionService); | ||
ArgumentNullException.ThrowIfNull(projectLocator); | ||
ArgumentNullException.ThrowIfNull(appHostBuilder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya know, this argument null checking isn't really necessary 😄. We not shipping a public API.
|
||
private string GetAppHostStateBasePath(FileInfo projectFile) | ||
{ | ||
var fullPath = projectFile.FullName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lowercase this?
|
||
_ = logger; | ||
|
||
var msBuildResult = await runner.GetProjectItemsAndPropertiesAsync( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we feel about transitivity here? You might need the same finger print for the closure of project refs (that aren't aspire projects).
9fe83d1
to
a4eb146
Compare
Force pushed updated code base on latest in main. In terms of transitivity we could look at all the project references, and for every project reference compute the same fingerprint. Hacky as - and the cure could be worse than the disease in terms of overall build performance - particularly on larger projects. |
@@ -331,7 +331,7 @@ public async Task<int> NewProjectAsync(string templateName, string name, string | |||
internal static string GetBackchannelSocketPath() | |||
{ | |||
var homeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); | |||
var dotnetCliPath = Path.Combine(homeDirectory, ".dotnet", "aspire", "cli", "backchannels"); | |||
var dotnetCliPath = Path.Combine(homeDirectory, ".aspire", "cli", "backchannels"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏾
|
||
_ = logger; | ||
|
||
var msBuildResult = await runner.GetProjectItemsAndPropertiesAsync( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are environment variables and global properties already ingested at this point?
var json = msBuildResult.Output?.RootElement.ToString(); | ||
|
||
var jsonBytes = Encoding.UTF8.GetBytes(json!); | ||
var hash = _sha256.ComputeHash(jsonBytes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, Sha256.HashData is static these days
Fixes: #8990
This PR introduces a build cache for the Aspire CLI. It works by first executing msbuild to get a collection of properties and items and computing a hash of that content. If the inputs to the build have not changed, then they should remain the same. If a build has successfully completed successfully previously with the same hash then we don't bother triggering a build again.
This implementation has a few problems (not quite ready yet):
dotnet clean
is performed then the fingerprint will match but the outputs won't be available. We need to figure out the best way of validating that build outputs are available to run.--no-cache
option to bothaspire run
andaspire publish
.aspire-run-with-cache.mp4