Broadly adopt LibraryInstallationGoalState #794
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jimmylewis
force-pushed
the
state
branch
2 times, most recently
from
0bbf3e9 to
2064471
Compare
Now that we compute the goalstate, we do validate that all files are valid. Closes aspnet#254
This allows FileSystemProvider to handle its special cases (i.e. rename support for a single file). FileSystemProvider did not get support for fileMappings prior to this. This will enable it to support that feature as well.
Because FileSystemProvider allows specifying a file as the library and a different file name as the destination (i.e. rename scenario), it was failing during goal state validation. However, in the FSP case, this is always valid - when a single file is specified, it can be mapped to one output file of any name.
- file conflicts are now the full path, since the goal state is fully resolved. - added a helper to make it easier to read test output when errors don't match.
Invalid files were previously caught on a different code path, so this makes the behavior match.
Coverage includes fileMappings and the rename case (library name is a file, files list includes a different name).
* Remove hack using GetCachedFileLocalPath to determine if the library name is a file path. Added a new helper method that checks this directly, handling HTTP URLs, relative paths, and absolute paths. * GetCachedFileLocalPath now returns rooted (absolute) paths to the cached file. This avoids ambiguity about the working directory.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the beginning, there was ILibraryInstallationState, and it was good. This type initially represents the serialized form of a library entry, and could also be expanded in situ to represent the final set of files that would be installed. This was done in the IProvider.UpdateStateAsync method. Executing an operation on a ILibraryInstallationState was wrapped in ILibraryOperationResult, which contained both the modified state and the result (and any errors encountered during the operation).
But then came fileMappings. This feature allowed installation configurations that are too complex to be represented by ILibraryInstallationState. The main scenarios (i.e. providers) were converted to using LibraryInstallationGoalState instead, which contains a precise mapping of all the files that would be installed by the library in question, and OperationResult was introduced as a generic replacement for ILibraryOperationResult (see #740).
However, the fileSystem provider had its own way of doing things and missed in this transition. The validation code to detect file conflicts also had still relied on the old ILibraryInstallationState.
This PR changes so that ILibraryInstallationState is solely used to represent an unprocessed library entry. All code that previous used UpdateStateAsync to produce a "final" state has been converted to using the LibraryInstallationGoalState type (and OperationResult), which can better represent the set of files affected by the operation (GoalState also has a reference to the InstallationState, so the original can be preserved). FileSystemProvider now shares more code from the BaseProvider, so it is more consistent with the others; though it does still have more special cases to remain aware of.
Resolves #793, resolves #792, and also resolves #254 along the way.