Fail if boolean env vars are set with common negative strings

Boolean environment variables have strange semantics: if the variable
is set then the configuration setting is considered "true".  This means
users can easily be mistaken that eg CCACHE_DISABLE=false means that
the "disable" setting is set to false.

To avoid too much backwards-incompatibility, we now exit with an error if
some common negative-sounding (case-insensitive) values:
"0", "false", "disable", "no".

All other values (including the empty string) are still considered to mean
"true".

Resolves https://github.com/ccache/ccache/issues/182

diff --git a/MANUAL.txt b/MANUAL.txt
index 6205667d03ef9d8ea2727accc819819f2937e01a..01dafae2fd5c49a08b71f6f367bf2574c18dc45e 100644 (file)
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -222,11 +222,12 @@ Boolean values
 Some settings are boolean values (i.e. truth values). In a configuration file,
 such values must be set to the string *true* or *false*. For the corresponding
 environment variables, the semantics are a bit different: a set environment
-variable means ``true'' regardless of the value (even if set to the empty
-string), and an unset environment variable means ``false''. Each boolean
-environment variable also has a negated form starting with *CCACHE_NO*. For
-example, *CCACHE_COMPRESS* can be set to force compression and
-*CCACHE_NOCOMPRESS* can be set to force no compression.
+variable means ``true'' (even if set to the empty string), the following
+case-insensitive negative values are considered an error (rather than surprise
+the user): "0", "false", "disable" and "no", and an unset environment variable
+means ``false''. Each boolean environment variable also has a negated form
+starting with *CCACHE_NO*. For example, *CCACHE_COMPRESS* can be set to force
+compression and *CCACHE_NOCOMPRESS* can be set to force no compression.
 
 
 Configuration settings

diff --git a/NEWS.txt b/NEWS.txt
index 1b94f119afa9f4c23ea8ab450c180cb5f1aedc10..c46d555430aa9b57bbdef4891b8bd77e065fb917 100644 (file)
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -22,6 +22,11 @@ New features and improvements
 - Added support for nvcc compiler options `--compiler-bindir/-ccbin`,
   `--output-directory/-odir` and `--libdevice-directory/-ldir`.
 
+- Boolean configuration settings no longer accept the following
+  (case-insensitive) values: "0", "false", "disable" and "no". All other values
+  are accepted and taken to mean "true". This is intended to avoid users
+  setting eg CCACHE_DISABLE=0 and expecting the cache to be used.
+
 
 Bug fixes
 ~~~~~~~~~

diff --git a/conf.c b/conf.c
index cfa2874af096d4de319f4ba3a9842727b6714bb6..6a0a9e66f5e44850de3d787c7071c9dd8471e9af 100644 (file)
--- a/conf.c
+++ b/conf.c
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2016 Joel Rosdahl
+// Copyright (C) 2011-2018 Joel Rosdahl
 //
 // This program is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by the Free
@@ -241,8 +241,19 @@ handle_conf_setting(struct conf *conf, const char *key, const char *value,
        }
 
        if (from_env_variable && item->parser == parse_bool) {
-               // Special rule for boolean settings from the environment: any value means
-               // true.
+               // Special rule for boolean settings from the environment:
+               // "0", "false", "disable" and "no" (case insensitive) are invalid,
+               // and all other values mean true.
+               //
+               // Previously any value meant true, but this was surprising to
+               // users, who might do something like CCACHE_DISABLE=0 and expect
+               // ccache to be enabled.
+
+               if (!strcmp(value, "0") || !strcasecmp(value, "false") ||
+                               !strcasecmp(value, "disable") || !strcasecmp(value, "no")) {
+                       fatal("invalid boolean environment variable value \"%s\"", value);
+               }
+
                bool *value = (bool *)((char *)conf + item->offset);
                *value = !negate_boolean;
                goto out;

diff --git a/test/suites/base.bash b/test/suites/base.bash
index 3c836dd15cf5b23fde0e78d790b4ec732d9fb351..06eac9179e2937e6231de934c839ea4f7ec515fe 100644 (file)
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -909,6 +909,20 @@ EOF
     if [ "$stderr" != "2Pu1Cc" ]; then
         test_failed "Unexpected stderr: $stderr != 2Pu1Cc"
     fi
+
+    # -------------------------------------------------------------------------
+    TEST "Invalid boolean environment configuration options"
+
+    for invalid_val in 0 false FALSE disable no ; do
+        CCACHE_DISABLE=$invalid_val $CCACHE $COMPILER --version > /dev/null 2>&1
+        if [ $? -eq 0 ] ; then
+            test_failed "'$invalid_val' should be rejected for boolean env vars"
+        fi
+        CCACHE_NODISABLE=$invalid_val $CCACHE $COMPILER --version > /dev/null 2>&1
+        if [ $? -eq 0 ] ; then
+            test_failed "'$invalid_val' should be rejected for boolean env vars"
+        fi
+    done
 }
 
 # =============================================================================