Skip to content
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

Introduce LibraryInstallationGoalState #743

Merged
merged 6 commits into from May 20, 2024

Conversation

jimmylewis
Copy link
Contributor

@jimmylewis jimmylewis commented Apr 29, 2024

This change refactors how we track the state of which files get installed from each library. The LibraryInstallationGoalState is a mapping of destination files to their sources (where the source is from the local file cache if applicable).

In a manner of speaking, if LibraryInstallationState ("LIS") is the input ("I want these files from this library from that provider"), then LibraryInstallationGoalState ("LIGS") is a computed output of exactly what work needs to be performed ("These files are copied to those destinations").

This design offers some additional flexibility.

  • The main difference is that we now have a way to represent arbitrary file mappings (a building block to eventually supporting file mappings in Proposal for more granular file mappings per library #738). LIS only contains a set of file paths within the library, often represented as a glob pattern. LIGS contains the actual files that would be installed, including the expansion of any glob patterns.
  • It will make refactoring the collision detection logic (LibrariesValidator.GetFilesConflicts) pretty simple, since now we can just get a list of all the destination files. This will also be necessary for more flexible file mappings, but I left it out of this PR.
  • The collection of files is agnostic to provider. This isn't super useful today, but could open the door to refactoring some install/uninstall logic out of the providers (I had a thought that provider implementations might be simpler that way, just computing goal states instead of providing install/uninstall logic as well).

The existing LibraryInstallationState abstraction is not flexible about mapping files to their source on an individual basis.  It presumes that all files installed by a library retain the same folder hierarchy as the library they originate from.

In order to allow more flexibility, this change adds a new abstraction to wrap all installed files to their individual source files.  This eliminates the prior assumption, allowing for flexibility to install files to a different folder hierarchy than their originating library structure.

This flexibility also allows an important new concept: each destination file must be unique (to avoid collisions), but the sources are not so constrained.  A single source file may be installed to multiple destinations; by creating a mapping of destination-to-source, we can easily allow this.

This change prepares for a new feature to allow more granular mappings of files within a library.
Also updated Manifest to use the goal state for the uninstall scenario.
Needed so that FileSystemProvider can override the default behavior, since it doesn't use the same cache logic as other providers.
/// <summary>
/// Returns whether the goal is in an achieved state - that is, all files are up to date.
/// </summary>
public bool IsAchieved()
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the name "achieved" as a word to match "goal", but it seems like if http URLs can never be achieved, maybe the method name could be "IsLocallyAcheived" as opposed to an IsAchievedAsync(cancellationtoken) that would actually make network calls to try and verify. Thoughts?

@phil-allen-msft phil-allen-msft merged commit 419abd9 into aspnet:main May 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants