summaryrefslogtreecommitdiff
path: root/doc
diff options
context:
space:
mode:
Diffstat (limited to 'doc')
-rw-r--r--doc/todo/Wishlist:_User.hasLoginShell/comment_1_c02e8783b91c3c0326bf1b317be4694f._comment11
-rw-r--r--doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable.mdwn3
-rw-r--r--doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable/comment_12_1e09f5a3f4565a9392d7b50b703a8a69._comment17
-rw-r--r--doc/todo/type-level_trivial_avoidance.mdwn92
-rw-r--r--doc/writing_properties.mdwn9
5 files changed, 121 insertions, 11 deletions
diff --git a/doc/todo/Wishlist:_User.hasLoginShell/comment_1_c02e8783b91c3c0326bf1b317be4694f._comment b/doc/todo/Wishlist:_User.hasLoginShell/comment_1_c02e8783b91c3c0326bf1b317be4694f._comment
index 52043406..91ca404e 100644
--- a/doc/todo/Wishlist:_User.hasLoginShell/comment_1_c02e8783b91c3c0326bf1b317be4694f._comment
+++ b/doc/todo/Wishlist:_User.hasLoginShell/comment_1_c02e8783b91c3c0326bf1b317be4694f._comment
@@ -25,17 +25,16 @@ run:
shellSetTo :: UserName -> FilePath -> Property
shellSetTo user shell = cmdProperty "chsh" ["--shell", shell, user]
-
+:
The only remaining problem with this is that shellSetTo runs chsh every
time, and propellor will always display that it's made a change each time
it runs, even when it didn't really do much. Now, there's an easy way to
-avoid that problem, we could just tell propellor that it's a trivial
-property, and then it will run chsh every time and not think it made any
-change:
+avoid that problem, we could just tell propellor to assume it's not made
+any change:
shellSetTo :: UserName -> FilePath -> Property
- shellSetTo user shell = trivial $
- cmdProperty "chsh" ["--shell", shell, user]
+ shellSetTo user shell = cmdProperty "chsh" ["--shell", shell, user]
+ `assume` NoChange
But, it's not much harder to do this right. Let's make the property
check if the user's shell is already set to the desired value and avoid
diff --git a/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable.mdwn b/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable.mdwn
index a27dc2e1..275ea9f5 100644
--- a/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable.mdwn
+++ b/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable.mdwn
@@ -1,3 +1,6 @@
Please consider merging branch `builddepfix` of repo `https://git.spwhitton.name/propellor`
Patches `Apt.buildDep` to check whether the build deps are installable, so that it no longer registers a change every spin.
+
+> Apt.buildDep now checks if the dpkg status file has changed, so [[done]]
+> --[[Joey]]
diff --git a/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable/comment_12_1e09f5a3f4565a9392d7b50b703a8a69._comment b/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable/comment_12_1e09f5a3f4565a9392d7b50b703a8a69._comment
new file mode 100644
index 00000000..3db6fd1b
--- /dev/null
+++ b/doc/todo/pull_request:_patch_Apt.buildDep_to_only_proceed_if_installable/comment_12_1e09f5a3f4565a9392d7b50b703a8a69._comment
@@ -0,0 +1,17 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 12"""
+ date="2015-12-05T22:52:42Z"
+ content="""
+I had a thought about this;
+[[trivial is a code smell|type-level_trivial_avoidance]] and adding
+UncheckedProperty type avoids needing to use trivial.
+
+So, now cmdProperty, runApt, and other things that make a Property but
+can't really detect when it MadeChange can instead make an
+UncheckedProperty, and changesFile is one of the ways to convert that into
+a Property.
+
+My implementation also allows applying changesFile multiple times, to
+detect a change to multiple files.
+"""]]
diff --git a/doc/todo/type-level_trivial_avoidance.mdwn b/doc/todo/type-level_trivial_avoidance.mdwn
new file mode 100644
index 00000000..797a1f8e
--- /dev/null
+++ b/doc/todo/type-level_trivial_avoidance.mdwn
@@ -0,0 +1,92 @@
+The `trivial` property combinator is a bit of a code smell. It's almost
+always better for a property to do the work to return MadeChange
+accurately. While it doesn't matter if a property directly attached to a
+Host is trivial, it can matter a great deal when eg, a disk image needs to
+be regenerated when a property makes a change to the source chroot.
+
+So, I'd like to move propellor to having all properties return an accurate
+MadeChange. Of course, it's up to the implementation to get that right, but
+avoiding `trivial` would go a long way.
+
+At the same time, it's sometimes useful to use trivial along the way to a
+non-trivial property.
+
+ trivial (cmdProperty "apt-get" ["-y", "install", "foo"])
+ `changesFile` "/var/lib/dpkg/status"
+
+Here the cmdProperty normally returns MadeChange, so trivial is used to
+throw that innaccurate value away and the changesFile combinator checks for
+changes.
+
+(The alternative would be for cmdProperty to normally return NoChange, and
+then have changesFile cause MadeChange to be returned. However, this
+approach has plenty of foot-shooting potential of its own, for example
+using cmdProperty and forgetting to check if it made any changes. If
+trivial is a code smell, making cmdProperty and similar generic property
+building tools trivial by default is surely not good..)
+
+----
+
+## So, could this be fixed at the type level?
+
+----
+
+### UncheckedProperty as an alternative to Property
+
+Perhaps it would make sense to
+have a UncheckedProperty, which could be used for things like
+`cmdProperty`. Combinators like `changesFile` would convert it to a
+Property.
+
+(A `trivial` combinator could still be written of course, but it wouldn't be
+necessary in cases like the above example anymore, so it would be more
+clearly a code smell to use `trivial`.)
+
+If UncheckedProperty was added, we'd want all the usual property
+combinators to also work with it. Including `requires`. This is entirely
+doable, but it's going to need quite a lot of duplicated code.
+
+For instance, there are 4 instances currently to handle combining properties
+with and without info; here's one of them:
+
+ instance Combines (Property HasInfo) (Property HasInfo) where
+ combineWith f _ (IProperty d1 a1 i1 cs1) y@(IProperty _d2 a2 _i2 _cs2) =
+ IProperty d1 (f a1 a2) i1 (y : cs1)
+
+Adding UncheckedProperty to the mix, we need another 4 instances for combining
+two of those. Plus 4 more for Property + UncheckedProperty = UncheckedProperty.
+Plus 4 more for combining UncheckedProperty + Property! Each of those instances
+has to be implemented separately. The code duplication doesn't stop at
+instances; also need constructors for UncheckedProperty, etc.
+
+### extending Property
+
+Another approach would be `Property i Unchecked|Checked`. But that seems
+overcomplicating for the end user, since most properties that users will
+deal with are not checked.
+
+### minimal UncheckedProperty
+
+Maybe add UncheckedProperty, but without the combining instances?
+
+How about this simple interface:
+
+ unchecked :: Property i -> UncheckedProperty i
+
+ checkResult :: ResultCheckable p => IO a -> (a -> IO Result) -> p i -> Property i
+
+ -- Both Property and UncheckedProperty are ResultCheckable.
+
+ changesFile :: Checkable p => p i -> FilePath -> Property i
+ changesFile p f = checkWith getstat comparestat p
+ where
+ getstat = ...
+ comparestat old = do
+ new <- getstat
+ return $ if old == new then MadeChange else NoChange
+
+Then, cmdProperty would construct a regular property, but apply `unchecked`
+to it. Users of `cmdProperty` would need to apply changesFile or a similar
+check to it before combining it with any other properties.
+
+> Yes, let's go this way. [[done]] --[[Joey]]
diff --git a/doc/writing_properties.mdwn b/doc/writing_properties.mdwn
index 9a700ac3..2209026f 100644
--- a/doc/writing_properties.mdwn
+++ b/doc/writing_properties.mdwn
@@ -53,13 +53,12 @@ run:
The only remaining problem with this is that shellSetTo runs chsh every
time, and propellor will always display that it's made a change each time
it runs, even when it didn't really do much. Now, there's an easy way to
-avoid that problem, we could just tell propellor that it's a trivial
-property, and then it will run chsh every time and not think it made any
-change:
+avoid that problem, we could just tell propellor to assume that chsh
+has not made a change:
shellSetTo :: UserName -> FilePath -> Property
- shellSetTo user shell = trivial $
- cmdProperty "chsh" ["--shell", shell, user]
+ shellSetTo user shell = cmdProperty "chsh" ["--shell", shell, user]
+ `assume` NoChange
But, it's not much harder to do this right. Let's make the property
check if the user's shell is already set to the desired value and avoid