a622488758
Removed undocumented special case in handling of a CHECKURL-MULTI response with only a single file listed. Rather than ignoring the url that was in the response, use it. This allows external special remotes that want to provide some better url to do so, although I don't entirely agree with using CHECKURL-MULTI to accomplish that. I'm more of the feeling that an undocumented special case that throws data away is just not a good idea. This could in theory break some external special remote program that relied on the current behavior, but its seems unlikely that it would because such a program must already handle the multiple url case, unless it only ever provides a single url response to CHECKURL-MULTI. Make addurl --file work with a single item CHECKURL-MULTI response. It already did for external special remotes due to the special case, but now it also will for builtin ones like the BitTorrent special remote. This commit was sponsored by Ilya Shlyakhter on Patron.
45 lines
2.2 KiB
Text
45 lines
2.2 KiB
Text
[[!comment format=mdwn
|
|
username="joey"
|
|
subject="""comment 1"""
|
|
date="2018-10-29T17:51:27Z"
|
|
content="""
|
|
Looking at the code, it addurl clearly *does* use the urls returned by
|
|
CHECKURL. In Command/AddUrl.hs:
|
|
|
|
go deffile (Right (UrlMulti l))
|
|
| isNothing (fileOption (downloadOptions o)) =
|
|
forM_ l $ \(u', sz, f) -> do
|
|
let f' = adjustFile o (deffile </> fromSafeFilePath f)
|
|
void $ commandAction $
|
|
startRemote r o f' u' sz
|
|
|
|
`l` is the list of values it returns, and `u'` is individual urls from that list,
|
|
as opposed to `u` which is the url the user provided.
|
|
`u'` is passed to `startRemote`, and `u` is not.
|
|
|
|
Hmm, but in Remote/External.hs there is a special case:
|
|
|
|
-- Treat a single item multi response specially to
|
|
-- simplify the external remote implementation.
|
|
CHECKURL_MULTI ((_, sz, f):[]) ->
|
|
result $ UrlContents sz $ Just $ mkSafeFilePath f
|
|
CHECKURL_MULTI l -> result $ UrlMulti $ map mkmulti l
|
|
|
|
That does not have any kind of rationalle in [[!commit 8a17bcb0be91c345a52d78c08009285b0fcd6e3a]],
|
|
but the next commit added `doc/special_remotes/external/git-annex-remote-torrent`
|
|
and I think I can see why I felt it simplified things. That script always
|
|
replies with CHECKURL-MULTI, but a torrent often contains a single file, and
|
|
it would be perhaps bettter to use the original url provided to the user for such a
|
|
file from a torrent, rather than an url that asks for file "#1" from the torrent.
|
|
Although AFAICS either would work, and Remote/BitTorrent.hs contains just the kind
|
|
of special case for a single item torrent that I was wanting to avoid external
|
|
special remotes needing to worry about.
|
|
|
|
The other benefit to special casing UrlContents is that lets addurl --file specify
|
|
where to put the file, which the fileOption check in the first code block
|
|
above prevents for UrlMulti. But, that could just as well be handled by
|
|
adding a single file special case to the code in AddUrl.
|
|
|
|
I suppose changing this won't break anything, or if it does it was relying
|
|
on this undocumented behavior.
|
|
"""]]
|