From f175d4cc908a3c4042ded4072906bf21a2468b89 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 22 Mar 2021 12:09:15 -0400 Subject: [PATCH] rmurl: remove all forms of an url, no matter what the downloader is set to * rmurl: When youtube-dl was used for an url, it no longer needs to be prefixed with "yt:" in order to be removed. * rmurl: If an url is both used by the web and also claimed by another special remote, fix a bug that caused the url to to not be removed. The youtube-dl change is a consequence of how the bug fix is implemented. But I also think it's the right thing to do. Consider that, before, git-annex addurl $url followed by git-annex rmurl $url would not remove the url in the case where youtube-dl was used. That was surprising behavior. In the unlikely case where a special remote claims an url, and it's been added using OtherDownloader, but it was also added already as a web url, it seems better for rmurl to remove both than to arbitrarily remove only one. And in the case the bug report was filed for, when an url was added as a web url, but a special remote now claims it, that should not prevent rmurl removing the web url. Calling setUrlMissing lets other callers of it behave differently. Probably the calls to it in eg, Remote.External and Remote.BitTorrent are fine, since they don't mangle the url and just remove what was provided, and the OtherDownloader form of a bittorrent url, respectively. I suspect unregisterurl needs to have a similar change made to rmurl, for similar reasons. --- CHANGELOG | 4 ++++ Command/RmUrl.hs | 8 ++++---- Logs/Web.hs | 20 ++++++++++--------- ...rks_url_not_available_in_wrong_remote.mdwn | 2 ++ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index bc1c1a5926..1a23d6722c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,10 @@ git-annex (8.20210311) UNRELEASED; urgency=medium or glacier, do not take login credentials from environment variables, as the user may not be expecting the autoenable to happen, or may have those set for other purposes and not intend git-annex to use them. + * rmurl: When youtube-dl was used for an url, it no longer needs to be + prefixed with "yt:" in order to be removed. + * rmurl: If an url is both used by the web and also claimed by another + special remote, fix a bug that caused the url to to not be removed. -- Joey Hess Fri, 12 Mar 2021 12:06:37 -0400 diff --git a/Command/RmUrl.hs b/Command/RmUrl.hs index 507f1d0099..83092800f3 100644 --- a/Command/RmUrl.hs +++ b/Command/RmUrl.hs @@ -1,6 +1,6 @@ {- git-annex command - - - Copyright 2013-2016 Joey Hess + - Copyright 2013-2021 Joey Hess - - Licensed under the GNU AGPL version 3 or higher. -} @@ -9,7 +9,6 @@ module Command.RmUrl where import Command import Logs.Web -import qualified Remote cmd :: Command cmd = notBareRepo $ @@ -55,6 +54,7 @@ start (si, (file, url)) = flip whenAnnexed file' $ \_ key -> do cleanup :: String -> Key -> CommandCleanup cleanup url key = do - r <- Remote.claimingUrl url - setUrlMissing key (setDownloader' url r) + -- Remove the url, no matter what downloader. + forM_ [minBound..maxBound] $ \dl -> + setUrlMissing key (setDownloader url dl) return True diff --git a/Logs/Web.hs b/Logs/Web.hs index 7a7b529937..88cfbc2695 100644 --- a/Logs/Web.hs +++ b/Logs/Web.hs @@ -72,14 +72,16 @@ setUrlPresent key url = do setUrlMissing :: Key -> URLString -> Annex () setUrlMissing key url = do - config <- Annex.getGitConfig - addLog (urlLogFile config key) - =<< logNow InfoMissing (LogInfo (encodeBS url)) - -- If the url was a web url (not OtherDownloader) and none of - -- the remaining urls for the key are web urls, the key must not - -- be present in the web. - when (isweb url) $ - whenM (null . filter isweb <$> getUrls key) $ + -- Avoid making any changes if the url was not registered. + us <- getUrls key + when (url `elem` us) $ do + config <- Annex.getGitConfig + addLog (urlLogFile config key) + =<< logNow InfoMissing (LogInfo (encodeBS url)) + -- If the url was a web url and none of the remaining urls + -- for the key are web urls, the key must not be present + -- in the web. + when (isweb url && null (filter isweb $ filter (/= url) us)) $ logChange key webUUID InfoMissing where isweb u = case snd (getDownloader u) of @@ -120,7 +122,7 @@ removeTempUrl key = Annex.changeState $ \s -> s { Annex.tempurls = M.delete key (Annex.tempurls s) } data Downloader = WebDownloader | YoutubeDownloader | QuviDownloader | OtherDownloader - deriving (Eq, Show) + deriving (Eq, Show, Enum, Bounded) {- To keep track of how an url is downloaded, it's mangled slightly in - the log, with a prefix indicating when a Downloader is used. -} diff --git a/doc/bugs/rmurl_marks_url_not_available_in_wrong_remote.mdwn b/doc/bugs/rmurl_marks_url_not_available_in_wrong_remote.mdwn index 90ee7992b9..958304e199 100644 --- a/doc/bugs/rmurl_marks_url_not_available_in_wrong_remote.mdwn +++ b/doc/bugs/rmurl_marks_url_not_available_in_wrong_remote.mdwn @@ -79,3 +79,5 @@ git annex `8.20210310+git49-g4d6f74477-1~ndall+1` [[!meta author=yoh]] [[!tag projects/repronim]] + +> [[fixed|done]] --[[Joey]]