Merge remote-tracking branch 'upstream/master' into pin

5 files changed, 142 insertions, 0 deletions

diff --git a/debian/changelog b/debian/changelog
index 0e5aa8d5..30af1b88 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -10,6 +10,8 @@ propellor (3.2.4) UNRELEASED; urgency=medium
     Thanks, Sean Whitton.
   * stack.yaml: Compile with GHC 8.0.1 against lts-7.16.
     Thanks, Andrew Cowie.
+  * Added Propellor.Property.File.configFileName and related functions
+    to generate good filenames for config directories.
 
  -- Joey Hess <id@joeyh.name>  Sat, 24 Dec 2016 15:06:36 -0400
 
diff --git a/doc/forum/Docker.hs_will_Break_in_Stretch/comment_1_8a4f16ae6d04b9d4bedb437ef333562b._comment b/doc/forum/Docker.hs_will_Break_in_Stretch/comment_1_8a4f16ae6d04b9d4bedb437ef333562b._comment
new file mode 100644
index 00000000..949f8d0c
--- /dev/null
+++ b/doc/forum/Docker.hs_will_Break_in_Stretch/comment_1_8a4f16ae6d04b9d4bedb437ef333562b._comment
@@ -0,0 +1,11 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2017-02-02T17:28:49Z"
+ content="""
+Apparently the Debian way to install docker will be from backports.
+<https://bugs.debian.org/cgi-bin/bugreport.cgi?att=3;bug=781554;msg=9>
+
+Note that I'm no longer using any docker Properties myself, so
+propellor users who are will need to send patches..
+"""]]
diff --git a/doc/todo/new_apt_pinning_properties/comment_2_c82f7e83f3fcc7648222d9dbf90e5ddd._comment b/doc/todo/new_apt_pinning_properties/comment_2_c82f7e83f3fcc7648222d9dbf90e5ddd._comment
new file mode 100644
index 00000000..4fd7c824
--- /dev/null
+++ b/doc/todo/new_apt_pinning_properties/comment_2_c82f7e83f3fcc7648222d9dbf90e5ddd._comment
@@ -0,0 +1,66 @@
+[[!comment format=mdwn
+ username="spwhitton"
+ avatar="http://cdn.libravatar.org/avatar/9c3f08f80e67733fd506c353239569eb"
+ subject="reply to review"
+ date="2017-02-02T17:40:11Z"
+ content="""
+Thank you for your feedback, Joey.
+
+> I wonder if it would be better to separate `suiteAvailablePinned`
+> into `suiteAvailable` and `suitePinned`? The latter could require
+> the former.
+
+I see how this could be useful, in particular if you want to make a
+suite like Debian experimental available, which won't cause any packages
+to be automatically upgraded.
+
+However, it makes it less convenient, and perhaps dangerous, to revert a
+pinned suite.  For example, suppose on my Debian testing system I have
+`Apt.suitePinned Unstable 100`.  If I revert this property, it will
+remove the pin but not remove the source.  Then my system might get
+mass-upgraded to sid if I'm not careful.
+
+We couldn't have the revert of `Apt.suitePinned` remove the source
+because then if I have both `& Apt.suiteAvailable Unstable` and `!
+Apt.suitePinned Unstable 100`, the second property would cancel out the
+first, which doesn't make sense.
+
+On balance, I think it's best to keep the current property.  A property
+adding sources to apt.sources.d should probably force the user to pick a
+pin value, to avoid any unexpected upgrades.
+
+> `pinnedTo` should probably be DebianLike not UnixLike.
+
+This was my 'TODO'.  (Since the property takes a `DebianSuite`, I think
+it should be `Debian` not `DebianLike`.)
+
+I tried applying `tightenTargets` to `pinnedTo`, but that only seems to
+affect one half of the revertable property.  Do I need to implement a
+new tightening function?
+
+> And its `[String]` parameter ought to be `[Package]`.
+
+I don't think so.  The parameter to `pinnedTo` can be a wildcard
+expression or a regex (per `apt_preferences(5)`).  Neither of these are
+accepted by other existing properties that take `[Package]`, such as
+`Apt.installed`.  I could add a new type alias, if you prefer.
+
+> Is `File.containsBlock` necessary? Seems that if you care about
+> ordering of blocks in the file, you generally should use
+> `File.hasContent` to specify the full content. Rather than using
+> /etc/apt/preferences.d/10propellor.pref for multiple properties,
+> you could use a separate file for each `pinnedTo'` with the parameters
+> encoded in the filename.
+
+This was what I tried on my first attempt, but it gets very complicated
+if the user passes a wildcard expression or a regex instead of a package
+name.  I would need to convert that wildcard expression or regex to a
+cross-platform filename, and the conversion would need to be isomorphic
+to avoid any clashes.  The `File.containsBlock` seems more sane than
+that.
+
+> As to the TODO, I tried adding this: [...]
+
+I don't understand how `robustly` is relevant to my TODO -- please see
+above.
+"""]]
diff --git a/doc/todo/new_apt_pinning_properties/comment_3_58d323602f293471ce3d2d9b4d271130._comment b/doc/todo/new_apt_pinning_properties/comment_3_58d323602f293471ce3d2d9b4d271130._comment
new file mode 100644
index 00000000..b0ff271e
--- /dev/null
+++ b/doc/todo/new_apt_pinning_properties/comment_3_58d323602f293471ce3d2d9b4d271130._comment
@@ -0,0 +1,23 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 3"""
+ date="2017-02-02T18:45:01Z"
+ content="""
+That example with reverting one property overriding another property
+is a general problem propellor has with conflicting properties. 
+Normally I don't much worry about it, but I agree an accidental mass
+upgrade is a good reason to avoid that problem here.
+
+Yes please add a new type alias for String (or an ADT) 
+if Package is not appropriate.
+
+I had misunderstood which function the TODO was for..
+
+Nice surprise that tightenTargets works on RevertableProperty at all.
+Since it does, you should be able to tighten one side, revert, tighten the
+other side, and re-revert. Or, deconstruct the RevertableProperty, 
+tighten both sides individually, and reconstruct it.
+
+I've added a Propellor.Property.File.configFileName that 
+should be suitable for your purposes, and others..
+"""]]
diff --git a/src/Propellor/Property/File.hs b/src/Propellor/Property/File.hs
index 9f73b14a..869fa48b 100644
--- a/src/Propellor/Property/File.hs
+++ b/src/Propellor/Property/File.hs
@@ -9,6 +9,7 @@ import qualified Data.ByteString.Lazy as L
 import Data.List (isInfixOf, isPrefixOf)
 import System.Posix.Files
 import System.Exit
+import Data.Char
 
 type Line = String
 
@@ -244,3 +245,42 @@ viaStableTmp a f = bracketIO setup cleanup go
 	go tmpfile = do
 		a tmpfile
 		liftIO $ rename tmpfile f
+
+-- | Generates a base configuration file name from a String, which
+-- can be put in a configuration directory, such as
+-- </etc/apt/sources.list.d/>
+--
+-- The generated file name is limited to using ASCII alphanumerics,
+-- \'_\' and \'.\' , so that programs that only accept a limited set of
+-- characters will accept it. Any other characters will be encoded
+-- in escaped form.
+--
+-- Some file extensions, such as ".old" may be filtered out by
+-- programs that use configuration directories. To avoid such problems,
+-- it's a good idea to add an static prefix and extension to the 
+-- result of this function. For example:
+--
+-- > aptConf foo = "/etc/apt/apt.conf.d" </> "propellor_" ++ configFileName foo <.> ".conf"
+configFileName :: String -> FilePath
+configFileName = concatMap escape
+  where
+	escape c
+		| isAscii c && isAlphaNum c = [c]
+		| c == '.' = [c]
+		| otherwise = '_' : show (ord c)
+
+-- | Applies configFileName to any value that can be shown.
+showConfigFileName :: Show v => v -> FilePath
+showConfigFileName = configFileName . show
+
+-- | Inverse of showConfigFileName.
+readConfigFileName :: Read v => FilePath -> Maybe v
+readConfigFileName = readish . unescape
+  where
+	unescape [] = []
+	unescape ('_':cs) = case break (not . isDigit) cs of
+		([], _) -> '_' : unescape cs
+		(ns, cs') -> case readish ns of
+			Nothing -> '_' : ns ++ unescape cs'
+			Just n -> chr n : unescape cs'
+	unescape (c:cs) = c : unescape cs