From fcdd9ce7884c5b8994692b1c9075568ba6864c6f Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Sun, 31 Dec 2017 14:55:51 -0400
Subject: [PATCH] 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                            | 12 ++++-
 CHANGELOG                                     |  3 ++
 Command/AddUrl.hs                             | 50 ++++++++++++-------
 .../addurl_youtube-dl_behavior_change.mdwn    |  2 +
 4 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/Annex/YoutubeDl.hs b/Annex/YoutubeDl.hs
index d1cac93684..43e63dfd6e 100644
--- a/Annex/YoutubeDl.hs
+++ b/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
diff --git a/CHANGELOG b/CHANGELOG
index 510336c651..34b91d00f3 100644
--- a/CHANGELOG
+++ b/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
 
diff --git a/Command/AddUrl.hs b/Command/AddUrl.hs
index 406682e732..6c641ec64a 100644
--- a/Command/AddUrl.hs
+++ b/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 ->
-		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)
-						let dest = if isJust (fileOption o)
-							then file
-							else takeFileName mediafile
-						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
+	tryyoutubedl tmp
+		| isJust (fileOption o) = go file
+		-- Ask youtube-dl what filename it will download
+		-- first, and check if that is already an annexed file,
+		-- to avoid unnecessary work in that case.
+		| otherwise = youtubeDlFileName' url >>= \case
+			Right dest -> ifAnnexed dest 
+				(alreadyannexed dest)
+				(dl dest)
+			Left _ -> normalfinish tmp
 	  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
diff --git a/doc/bugs/addurl_youtube-dl_behavior_change.mdwn b/doc/bugs/addurl_youtube-dl_behavior_change.mdwn
index 500ed6391d..17455358b4 100644
--- a/doc/bugs/addurl_youtube-dl_behavior_change.mdwn
+++ b/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]]