From 720c69c385cdb28d536fc6d85033345f3f7b87bb Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 2 Feb 2017 13:30:52 -0400 Subject: comment --- .../comment_1_8a4f16ae6d04b9d4bedb437ef333562b._comment | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 doc/forum/Docker.hs_will_Break_in_Stretch/comment_1_8a4f16ae6d04b9d4bedb437ef333562b._comment 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. + + +Note that I'm no longer using any docker Properties myself, so +propellor users who are will need to send patches.. +"""]] -- cgit v1.2.3 From 5c170d3f8021b2120d3147103e23baf7bac6cc76 Mon Sep 17 00:00:00 2001 From: spwhitton Date: Thu, 2 Feb 2017 17:40:11 +0000 Subject: Added a comment: reply to review --- ...ent_2_c82f7e83f3fcc7648222d9dbf90e5ddd._comment | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 doc/todo/new_apt_pinning_properties/comment_2_c82f7e83f3fcc7648222d9dbf90e5ddd._comment 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. +"""]] -- cgit v1.2.3 From b1f0bd05941f5d8c42a28d33316a0a0452a62476 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 2 Feb 2017 15:40:30 -0400 Subject: Added Propellor.Property.File.configFileName and related functions to generate good filenames for config directories. spwhitton has a branch that could use this, and there are several places in propellor that do something ad-hoc that would have been better implemented using this. I was not able to switch any of the existing ad-hoc stuff, but this can be used going forward for new stuff. This commit was sponsored by Anthony DeRobertis on Patreon. --- debian/changelog | 2 ++ src/Propellor/Property/File.hs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) 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 Sat, 24 Dec 2016 15:06:36 -0400 diff --git a/src/Propellor/Property/File.hs b/src/Propellor/Property/File.hs index 95fc6f81..fe2b1057 100644 --- a/src/Propellor/Property/File.hs +++ b/src/Propellor/Property/File.hs @@ -8,6 +8,7 @@ import Utility.FileMode import qualified Data.ByteString.Lazy as L import System.Posix.Files import System.Exit +import Data.Char type Line = String @@ -221,3 +222,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 ".dpkg-new" 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: +-- +-- > aptSource foo = "/etc/apt/sources.list.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 -- cgit v1.2.3 From 5333f3fb5c1a2ac6c16c8217c52ede2c7f03333f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 2 Feb 2017 15:44:20 -0400 Subject: response --- ...ent_3_58d323602f293471ce3d2d9b4d271130._comment | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 doc/todo/new_apt_pinning_properties/comment_3_58d323602f293471ce3d2d9b4d271130._comment 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.. +"""]] -- cgit v1.2.3 From c682c51e828b82b60443b3cf368114f6506b03e9 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 2 Feb 2017 15:51:49 -0400 Subject: haddock formatting --- src/Propellor/Property/File.hs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Propellor/Property/File.hs b/src/Propellor/Property/File.hs index fe2b1057..388bcb6c 100644 --- a/src/Propellor/Property/File.hs +++ b/src/Propellor/Property/File.hs @@ -225,14 +225,14 @@ viaStableTmp a f = bracketIO setup cleanup go -- | 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 +-- \'_\' 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 ".dpkg-new" may be filtered out by +-- 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: -- cgit v1.2.3 From f861070a3ad159a60961292ccdb53e30524968cd Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 2 Feb 2017 15:52:52 -0400 Subject: better example --- src/Propellor/Property/File.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Propellor/Property/File.hs b/src/Propellor/Property/File.hs index 388bcb6c..9241cb1b 100644 --- a/src/Propellor/Property/File.hs +++ b/src/Propellor/Property/File.hs @@ -237,7 +237,7 @@ viaStableTmp a f = bracketIO setup cleanup go -- it's a good idea to add an static prefix and extension to the -- result of this function. For example: -- --- > aptSource foo = "/etc/apt/sources.list.d" "propellor_" ++ configFileName foo <.> ".conf" +-- > aptConf foo = "/etc/apt/apt.conf.d" "propellor_" ++ configFileName foo <.> ".conf" configFileName :: String -> FilePath configFileName = concatMap escape where -- cgit v1.2.3