From 0ee04ecc43e047b00437fb660e71f7dd67dd3afc Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sat, 24 Jan 2015 22:38:10 -0400 Subject: GADT properties seem to work (untested) * Property has been converted to a GADT, and will be Property NoInfo or Property HasInfo. This was done to make sure that ensureProperty is only used on properties that do not have Info. Transition guide: - Change all "Property" to "Property NoInfo" or "Property WithInfo" (The compiler can tell you if you got it wrong!) - To construct a RevertableProperty, it is useful to use the new () operator - Constructing a list of properties can be problimatic, since Property NoInto and Property WithInfo are different types and cannot appear in the same list. To deal with this, "props" has been added, and can built up a list of properties of different types, using the same (&) and (!) operators that are used to build up a host's properties. --- doc/todo/info_propigation_out_of_nested_properties.mdwn | 2 ++ 1 file changed, 2 insertions(+) (limited to 'doc') diff --git a/doc/todo/info_propigation_out_of_nested_properties.mdwn b/doc/todo/info_propigation_out_of_nested_properties.mdwn index 1a586be6..e6427069 100644 --- a/doc/todo/info_propigation_out_of_nested_properties.mdwn +++ b/doc/todo/info_propigation_out_of_nested_properties.mdwn @@ -1,3 +1,5 @@ +> Now [[fixed|done]]!! --[[Joey]] + Currently, Info about a Host's Properties is manually gathered and propigated. propertyList combines the Info of the Properties in the list. Docker.docked extracts relevant Info from the Properties of the container -- cgit v1.2.3 From e9d5d9aff1cc2046149d3e5dcd9f4ef0f2a334a1 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 25 Jan 2015 14:45:14 -0400 Subject: remove toSimpleProp It didn't do what I thought it did with a RevertableProperty; it always returned Nothing because even if the input properties to are NoInfo, it casts them to HasInfo. Even if it had worked, it lost type safety. Better to export the Property NoInfo that is used in a RevertableProperty, so it can be used directly. --- doc/todo/RevertableProperty_with_NoInfo.mdwn | 18 ++++++++++++ src/Propellor/Property/Apt.hs | 25 +++++++++++------ src/Propellor/Property/Debootstrap.hs | 41 +++++++++++++++------------- src/Propellor/Property/OS.hs | 4 +-- src/Propellor/Property/Obnam.hs | 2 +- src/Propellor/Types.hs | 4 --- 6 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 doc/todo/RevertableProperty_with_NoInfo.mdwn (limited to 'doc') diff --git a/doc/todo/RevertableProperty_with_NoInfo.mdwn b/doc/todo/RevertableProperty_with_NoInfo.mdwn new file mode 100644 index 00000000..3b4a61a9 --- /dev/null +++ b/doc/todo/RevertableProperty_with_NoInfo.mdwn @@ -0,0 +1,18 @@ +Currently, a RevertableProperty's Properties always both HasInfo. This +means that if a Property NoInfo is updated to be a RevertableProperty, and +someplace called ensureProperty on it, that will refuse to compile. + +The workaround is generally to export the original NoInfo property under +a different name, so it can still be used with ensureProperty. + +This could be fixed: + + data RevertableProperty i1 i2 where + RProp :: Property i1 -> Property i2 -> RevertableProperty i1 i2 + +However, needing to write "RevertableProperty HasInfo NoInfo" is quite +a mouthful! + +Since only 2 places in the propellor source code currently need to deal +with this, it doesn't currently seem worth making the change, unless a less +intrusive way can be found. diff --git a/src/Propellor/Property/Apt.hs b/src/Propellor/Property/Apt.hs index d567d0ec..75c59772 100644 --- a/src/Propellor/Property/Apt.hs +++ b/src/Propellor/Property/Apt.hs @@ -266,17 +266,24 @@ data AptKey = AptKey } trustsKey :: AptKey -> RevertableProperty -trustsKey k = trust untrust +trustsKey k = trustsKey' k untrustKey k + +trustsKey' :: AptKey -> Property NoInfo +trustsKey' k = check (not <$> doesFileExist f) $ property desc $ makeChange $ do + withHandle StdinHandle createProcessSuccess + (proc "gpg" ["--no-default-keyring", "--keyring", f, "--import", "-"]) $ \h -> do + hPutStr h (pubkey k) + hClose h + nukeFile $ f ++ "~" -- gpg dropping where desc = "apt trusts key " ++ keyname k - f = "/etc/apt/trusted.gpg.d" keyname k ++ ".gpg" - untrust = File.notPresent f - trust = check (not <$> doesFileExist f) $ property desc $ makeChange $ do - withHandle StdinHandle createProcessSuccess - (proc "gpg" ["--no-default-keyring", "--keyring", f, "--import", "-"]) $ \h -> do - hPutStr h (pubkey k) - hClose h - nukeFile $ f ++ "~" -- gpg dropping + f = aptKeyFile k + +untrustKey :: AptKey -> Property NoInfo +untrustKey = File.notPresent . aptKeyFile + +aptKeyFile :: AptKey -> FilePath +aptKeyFile k = "/etc/apt/trusted.gpg.d" keyname k ++ ".gpg" -- | Cleans apt's cache of downloaded packages to avoid using up disk -- space. diff --git a/src/Propellor/Property/Debootstrap.hs b/src/Propellor/Property/Debootstrap.hs index 3feb280c..d4947ab7 100644 --- a/src/Propellor/Property/Debootstrap.hs +++ b/src/Propellor/Property/Debootstrap.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE FlexibleContexts #-} + module Propellor.Property.Debootstrap ( Url, DebootstrapConfig(..), @@ -56,18 +58,18 @@ toParams (c1 :+ c2) = toParams c1 <> toParams c2 -- Note that reverting this property does not stop any processes -- currently running in the chroot. built :: FilePath -> System -> DebootstrapConfig -> RevertableProperty -built = built' (toProp installed) - -built' :: Property HasInfo -> FilePath -> System -> DebootstrapConfig -> RevertableProperty -built' installprop target system@(System _ arch) config = setup teardown +built target system config = built' (toProp installed) target system config teardown where - setup = check (unpopulated target <||> ispartial) setupprop - `requires` installprop - teardown = check (not <$> unpopulated target) teardownprop - unpopulated d = null <$> catchDefaultIO [] (dirContents d) + teardownprop = property ("removed debootstrapped " ++ target) $ + makeChange (removetarget target) +built' :: (Combines (Property NoInfo) (Property i)) => Property i -> FilePath -> System -> DebootstrapConfig -> Property (CInfo NoInfo i) +built' installprop target system@(System _ arch) config = + check (unpopulated target <||> ispartial) setupprop + `requires` installprop + where setupprop = property ("debootstrapped " ++ target) $ liftIO $ do createDirectoryIfMissing True target -- Don't allow non-root users to see inside the chroot, @@ -92,24 +94,25 @@ built' installprop target system@(System _ arch) config = setup teardown , return FailedChange ) - teardownprop = property ("removed debootstrapped " ++ target) $ - makeChange removetarget - - removetarget = do - submnts <- filter (\p -> simplifyPath p /= simplifyPath target) - . filter (dirContains target) - <$> mountPoints - forM_ submnts umountLazy - removeDirectoryRecursive target - -- A failed debootstrap run will leave a debootstrap directory; -- recover by deleting it and trying again. ispartial = ifM (doesDirectoryExist (target "debootstrap")) ( do - removetarget + removetarget target return True , return False ) + +unpopulated :: FilePath -> IO Bool +unpopulated d = null <$> catchDefaultIO [] (dirContents d) + +removetarget :: FilePath -> IO () +removetarget target = do + submnts <- filter (\p -> simplifyPath p /= simplifyPath target) + . filter (dirContains target) + <$> mountPoints + forM_ submnts umountLazy + removeDirectoryRecursive target extractSuite :: System -> Maybe String extractSuite (System (Debian s) _) = Just $ Apt.showSuite s diff --git a/src/Propellor/Property/OS.hs b/src/Propellor/Property/OS.hs index 710428d4..7a6857fb 100644 --- a/src/Propellor/Property/OS.hs +++ b/src/Propellor/Property/OS.hs @@ -89,10 +89,10 @@ cleanInstallOnce confirmation = check (not <$> doesFileExist flagfile) $ (Just u@(System (Ubuntu _) _)) -> debootstrap u _ -> error "os is not declared to be Debian or Ubuntu" - debootstrap targetos = ensureProperty $ fromJust $ toSimpleProp $ + debootstrap targetos = ensureProperty $ -- Ignore the os setting, and install debootstrap from -- source, since we don't know what OS we're running in yet. - Debootstrap.built' (toProp Debootstrap.sourceInstall) + Debootstrap.built' Debootstrap.sourceInstall newOSDir targetos Debootstrap.DefaultConfig -- debootstrap, I wish it was faster.. -- TODO eatmydata to speed it up diff --git a/src/Propellor/Property/Obnam.hs b/src/Propellor/Property/Obnam.hs index 9d283527..adaf255c 100644 --- a/src/Propellor/Property/Obnam.hs +++ b/src/Propellor/Property/Obnam.hs @@ -118,7 +118,7 @@ latestVersion :: Property NoInfo latestVersion = withOS "obnam latest version" $ \o -> case o of (Just (System (Debian suite) _)) | isStable suite -> ensureProperty $ Apt.setSourcesListD (stablesources suite) "obnam" - `requires` (fromJust (toSimpleProp (Apt.trustsKey key))) + `requires` Apt.trustsKey' key _ -> noChange where stablesources suite = diff --git a/src/Propellor/Types.hs b/src/Propellor/Types.hs index 9a0e22ab..ba63cf9d 100644 --- a/src/Propellor/Types.hs +++ b/src/Propellor/Types.hs @@ -179,7 +179,6 @@ class IsProp p where -- | Sets description. describe :: p -> Desc -> p toProp :: p -> Property HasInfo - toSimpleProp :: p -> Maybe (Property NoInfo) getDesc :: p -> Desc -- | Gets the info of the property, combined with all info -- of all children properties. @@ -188,7 +187,6 @@ class IsProp p where instance IsProp (Property HasInfo) where describe (IProperty _ a i cs) d = IProperty d a i cs toProp = id - toSimpleProp _ = Nothing getDesc = propertyDesc getInfoRecursive (IProperty _ _ i cs) = i <> mconcat (map getInfoRecursive cs) @@ -196,7 +194,6 @@ instance IsProp (Property NoInfo) where describe (SProperty _ a cs) d = SProperty d a cs toProp = toIProperty getDesc = propertyDesc - toSimpleProp = Just getInfoRecursive _ = mempty instance IsProp RevertableProperty where @@ -205,7 +202,6 @@ instance IsProp RevertableProperty where RevertableProperty (describe p1 d) (describe p2 ("not " ++ d)) getDesc (RevertableProperty p1 _) = getDesc p1 toProp (RevertableProperty p1 _) = p1 - toSimpleProp = toSimpleProp . toProp -- | Return the Info of the currently active side. getInfoRecursive (RevertableProperty p1 _p2) = getInfoRecursive p1 -- cgit v1.2.3 From 73ad89d9734bace04b68e078fa1d46ccee6a396b Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 25 Jan 2015 15:04:05 -0400 Subject: propellor spin --- doc/todo/RevertableProperty_with_NoInfo.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc') diff --git a/doc/todo/RevertableProperty_with_NoInfo.mdwn b/doc/todo/RevertableProperty_with_NoInfo.mdwn index 3b4a61a9..e778ff85 100644 --- a/doc/todo/RevertableProperty_with_NoInfo.mdwn +++ b/doc/todo/RevertableProperty_with_NoInfo.mdwn @@ -16,3 +16,6 @@ a mouthful! Since only 2 places in the propellor source code currently need to deal with this, it doesn't currently seem worth making the change, unless a less intrusive way can be found. + +Hmm, if RevertableProperty were made a constructor in the Property GADT, +this would need to be done, and that would also allow for -- cgit v1.2.3 From 5d8bd485cbd4c88bcc09146118d1431d13a152cc Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 25 Jan 2015 15:08:40 -0400 Subject: propellor spin --- doc/todo/RevertableProperty_with_NoInfo.mdwn | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'doc') diff --git a/doc/todo/RevertableProperty_with_NoInfo.mdwn b/doc/todo/RevertableProperty_with_NoInfo.mdwn index e778ff85..e9c1eb5d 100644 --- a/doc/todo/RevertableProperty_with_NoInfo.mdwn +++ b/doc/todo/RevertableProperty_with_NoInfo.mdwn @@ -17,5 +17,13 @@ Since only 2 places in the propellor source code currently need to deal with this, it doesn't currently seem worth making the change, unless a less intrusive way can be found. -Hmm, if RevertableProperty were made a constructor in the Property GADT, -this would need to be done, and that would also allow for +Probably related would be to make RevertableProperty a constructor in the +Property GADT, which would allow more property combinators to work on +RevertableProperties. That would look like: + + data Propety i where + ... + RProp :: Property i1 -> Property i2 -> Property (CInfo i1 i2) + +In this case, there's only one Info/NoInfo encompassing both sides, and +so ensureProperty could only be used on it if both sides were NoInfo. -- cgit v1.2.3