repeated addurl behavior reversion fix

addurl: When the file youtube-dl will download is already an annexed file,
don't download it again and fail to overwrite it, instead just do nothing,
like it used to when quvi was used.

This commit was sponsored by Anthony DeRobertis on Patreon.

Annex/YoutubeDl.hs

@@ -11,6 +11,7 @@ module Annex.YoutubeDl (
 	youtubeDlSupported,
 	youtubeDlCheck,
 	youtubeDlFileName,
+	youtubeDlFileName',
 ) where
 
 import Annex.Common
@@ -144,7 +145,16 @@ youtubeDlCheck url
 youtubeDlFileName :: URLString -> Annex (Either String FilePath)
 youtubeDlFileName url
 	| supportedScheme url = flip catchIO (pure . Left . show) $
-		htmlOnly url nomedia go
+		htmlOnly url nomedia (youtubeDlFileName' url)
+	| otherwise = return nomedia
+  where
+	nomedia = Left "no media in url"
+
+-- Does not check if the url contains htmlOnly; use when that's already
+-- been verified.
+youtubeDlFileName' :: URLString -> Annex (Either String FilePath)
+youtubeDlFileName' url
+	| supportedScheme url = flip catchIO (pure . Left . show) go
 	| otherwise = return nomedia
   where
 	go = do

CHANGELOG

@@ -5,6 +5,9 @@ git-annex (6.20171215) UNRELEASED; urgency=medium
     downloaded.
   * Fix bug introduced in version 6.20171018 that caused some commands
     to print out "ok" twice after processing a file.
+  * addurl: When the file youtube-dl will download is already an annexed
+    file, don't download it again and fail to overwrite it, instead just do
+    nothing, like it used to when quvi was used.
 
  -- Joey Hess <id@joeyh.name>  Wed, 20 Dec 2017 12:11:46 -0400
 

Command/AddUrl.hs

@@ -272,26 +272,42 @@ downloadWeb o url urlinfo file =
 		showDestinationFile file
 		liftIO $ createDirectoryIfMissing True (parentDir file)
 		finishDownloadWith tmp webUUID url file
-	tryyoutubedl tmp = withTmpWorkDir mediakey $ \workdir ->
+	tryyoutubedl tmp
-		Transfer.notifyTransfer Transfer.Download url $
+		| isJust (fileOption o) = go file
-			Transfer.download webUUID mediakey (AssociatedFile Nothing) Transfer.noRetry $ \_p ->
+		-- Ask youtube-dl what filename it will download
-				youtubeDl url workdir >>= \case
+		-- first, and check if that is already an annexed file,
-					Right (Just mediafile) -> do
+		-- to avoid unnecessary work in that case.
-						pruneTmpWorkDirBefore tmp (liftIO . nukeFile)
+		| otherwise = youtubeDlFileName' url >>= \case
-						let dest = if isJust (fileOption o)
+			Right dest -> ifAnnexed dest 
-							then file
+				(alreadyannexed dest)
-							else takeFileName mediafile
+				(dl dest)
-						checkCanAdd dest $ do
+			Left _ -> normalfinish tmp
-							showDestinationFile dest
-							addWorkTree webUUID mediaurl dest mediakey (Just mediafile)
-							return $ Just mediakey
-					Right Nothing -> normalfinish tmp
-					Left msg -> do
-						warning msg
-						return Nothing
 	  where
+		dl dest = withTmpWorkDir mediakey $ \workdir ->
+			Transfer.notifyTransfer Transfer.Download url $
+				Transfer.download webUUID mediakey (AssociatedFile Nothing) Transfer.noRetry $ \_p ->
+					youtubeDl url workdir >>= \case
+						Right (Just mediafile) -> do
+							pruneTmpWorkDirBefore tmp (liftIO . nukeFile)
+							checkCanAdd dest $ do
+								showDestinationFile dest
+								addWorkTree webUUID mediaurl dest mediakey (Just mediafile)
+								return $ Just mediakey
+						Right Nothing -> normalfinish tmp
+						Left msg -> do
+							warning msg
+							return Nothing
 		mediaurl = setDownloader url YoutubeDownloader
 		mediakey = Backend.URL.fromUrl mediaurl Nothing
+		-- Does the already annexed file have the mediaurl
+		-- as an url? If so nothing to do.
+		alreadyannexed dest k = do
+			us <- getUrls k
+			if mediaurl `elem` us
+				then return (Just k)
+				else do
+					warning $ dest ++ " already exists; not overwriting"
+					return Nothing
 
 showDestinationFile :: FilePath -> Annex ()
 showDestinationFile file = do

doc/bugs/addurl_youtube-dl_behavior_change.mdwn

@@ -15,3 +15,5 @@ show-stopper, but just something I noticed.  I generally do "addurl" manually.
 > if the local file already exists and already has the url, to get back to
 > the old behavior.
 > -- [[Joey]]
+
+[[fixed|done]] --[[Joey]]