aboutsummaryrefslogtreecommitdiff
path: root/Remote.hs
diff options
context:
space:
mode:
authorGravatar Joey Hess <joey@kitenet.net>2013-03-16 17:43:42 -0400
committerGravatar Joey Hess <joey@kitenet.net>2013-03-16 17:43:42 -0400
commit6a09a1403f6d31b0d518dc9d65b9a9358fb594b9 (patch)
treef2c6832a7a327e3a8bc838bad302ef708c4c088b /Remote.hs
parent5deaa7c59bd330a7b20a19aee3fe01e6283ec9c8 (diff)
Fix several bugs caused by a bad Ord instance for Remote.
A long time ago I made Remote be an instance of the Ord typeclass, with an implementation that compared the costs of Remotes. That seemed like a good idea at the time, as it saved typing.. But at the time I was still making custom Read and Show instances too. I've since learned that this is *not* a good idea, and neither is making custom Ord instances, without deep thought about the possible sets of values in a type. Haskell typeclasses are not a toy. This Ord instance came around and bit me when I put Remotes into a Set, because now remotes with the same cost appeared to be in the Set even if they were not. Also affected putting Remotes into a Map. Rarely does a bug go this deep. I've fixed it comprehensively, first removing the Ord instance entirely, and fixing the places that wanted to order remotes by cost to do it explicitly. Then adding back an Ord instance that is much more sane. Also by checking the rest of the Ord instances in the code base (which were all ok). While doing that, I found lots of places that kept remotes in Maps and Sets. All of it was probably subtly broken in one way or another before this fix, but it would be hard to say exactly how the bugs would manifest.
Diffstat (limited to 'Remote.hs')
-rw-r--r--Remote.hs5
1 files changed, 3 insertions, 2 deletions
diff --git a/Remote.hs b/Remote.hs
index 4f8b7cf6a..01d6da3cc 100644
--- a/Remote.hs
+++ b/Remote.hs
@@ -45,6 +45,7 @@ import qualified Data.Map as M
import Text.JSON
import Text.JSON.Generic
import Data.Tuple
+import Data.Ord
import Common.Annex
import Types.Remote
@@ -208,7 +209,7 @@ keyPossibilities' key trusted = do
allremotes <- enabledRemoteList
let validremotes = remotesWithUUID allremotes validuuids
- return (sort validremotes, validtrusteduuids)
+ return (sortBy (comparing cost) validremotes, validtrusteduuids)
{- Displays known locations of a key. -}
showLocations :: Key -> [UUID] -> String -> Annex ()
@@ -249,7 +250,7 @@ logStatus remote key = logChange key (uuid remote)
{- Orders remotes by cost, with ones with the lowest cost grouped together. -}
byCost :: [Remote] -> [[Remote]]
-byCost = map snd . sort . M.toList . costmap
+byCost = map snd . sortBy (comparing fst) . M.toList . costmap
where
costmap = M.fromListWith (++) . map costpair
costpair r = (cost r, [r])