Allow multiple manifests to be downloaded from the same depot#707
Open
scarletcafe wants to merge 6 commits into
Open
Allow multiple manifests to be downloaded from the same depot#707scarletcafe wants to merge 6 commits into
scarletcafe wants to merge 6 commits into
Conversation
|
This seems to fail if |
Author
|
I've swapped the branch pattern around and it should fix this. From what I can see the other cases of Config.InstallDirectory being null were already correctly checked for so that line was the only one with the oversight. |
This file contains hidden or 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
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.
Rationale
I want to be able to download multiple versions of a game from a depot, without having to manually do each manifest at a time and without hitting authentication rate limits as brought up in #674.
Problems
There are currently 3 problems that prevent download of multiple manifests from one depot:
Problem 1
Typing
-depot 1 -manifest 2 3produces "Error: -manifest requires one id for every -depot specified"This is mostly a user experience thing, but if you wanted to e.g. download 4 manifests you would need to do
-depot 1 1 1 1 -manifest 2 3 4 5which is slightly annoying.This PR fixes this by detecting if exactly one depot ID is specified with multiple manifest IDs, and then uses that depot ID for all manifests (and produces a message so the user knows what's happening).
Problem 2
Typing
-depot 1 1 -manifest 2 3produces "Depot not listed for app 1"This happens because DepotDownloader tries to validate that all depots requested were retrieved by comparing the length of the retrieved list with the provided list. It deduplicates depots retrieved, so in this case it will compare
[1, 1]with[1]and conclude that not all depots were retrieved, failing the download.This PR fixes it by instead checking that all IDs within
depotIdsExpectedare contained withindepotIdsFound, working even if there are duplicate IDs.Problem 3
The default output directory does not have subdirectories for manifests, so manifests with the same depot ID will be downloaded to the same place. In addition, specifying your own directory will just download ALL of the contents into the same folder.
This PR fixes it by allowing
-dirto have substitution variables%(depot_id),%(depot_version)and%(manifest_id). This means you can do e.g.-depot 1 -manifest 2 3 4 -dir "depots/%(depot_id)/%(manifest_id)"to have it output each manifest into a different directory.Problem 4
DepotDownloader assumes that either deduplication is not necessary if
-diris not passed, or that it is necessary and will do it across all files if-diris passed.This means even if the outputs would go into separate directories due to the string interpolation implemented before, it will still ignore files that have the same name, producing manifest folders with incomplete installs.
This PR fixes it by rewriting the deduplication code to deduplicate only within
DepotDownloadInfos that target the exact same output folder. This means deduplication still works in all prior supported cases, but won't prevent the same files being written if they are going to different folders.This does have the caveat that identical files might be downloaded multiple times, but I'm not sure if there is a good way to detect this before the download has happened, and I think it would add a lot of complexity and need for rewriting. I think the implementation as is works for my use case, and people are free to make a follow-up PR if they think this is important enough.
Caveats
The config for DepotDownloader is usually either placed in the root of
depots/or in the location given by-dir. The config is shared across all of the downloads, so it doesn't belong to any specific depot or manifest ID.To deal with this, this PR just substitutes all of the variables with
0for the config directory. This creates an extra, potentially pretty confusing, directory, but it does allow all downloads who use the same-dirformat to share a config without issue.I think this is not an ideal solution but I didn't want to make any unintentional breaking changes so this is the workaround for that.