support setting empty values (sanely) & removing support for

space-gapping: '-o option= value'

That is a very old feature (straight from 1998), but it is super
surprising if you try setting empty values and instead get error
messages or a non-empty value as the next parameter is treated as the
value – which could have been empty, so if for some reason you need a
compatible way of setting an empty value try: '-o option="" ""'.

I can only guess that the idea was to support '-o option value', but we
survived 17 years without it, we will do fine in the future I guess.

Similar is the case for '-t= testing' even through '-t testing' existed
before and the code even tried to detect mistakes like '-t= -b' … all
gone now.

Technically that is as its removing a feature replacing it with another
a major interface break. In practice I really hope for my and their
sanity that nobody was using this; but if for some reaon you do: Remove
the space and be done.

I found the patch and the bugreport actually only after the fact, but
its reassuring that others are puzzled by this as well and hence a
thanks is in perfect order here as the patch is practical identical
[expect that this one here adds tests and other bonus items].

Thanks: Daniel Hartwig for initial patch.
Closes: 693092

diff --git a/apt-pkg/contrib/cmndline.cc b/apt-pkg/contrib/cmndline.cc
index eb48d1d75c034015cb13f08be0b6a61562fb8d70..c8a6e27873341cd3129394facc283e5953740451 100644 (file)
--- a/apt-pkg/contrib/cmndline.cc
+++ b/apt-pkg/contrib/cmndline.cc
@@ -205,17 +205,11 @@ bool CommandLine::HandleOpt(int &I,int argc,const char *argv[],
 
    /* Determine the possible location of an option or 0 if their is
       no option */
-   if (Opt[1] == 0 || (Opt[1] == '=' && Opt[2] == 0))
+   if (Opt[1] == 0)
    {
       if (I + 1 < argc && argv[I+1][0] != '-')
         Argument = argv[I+1];
-      
-      // Equals was specified but we fell off the end!
-      if (Opt[1] == '=' && Argument == 0)
-        return _error->Error(_("Option %s requires an argument."),argv[I]);
-      if (Opt[1] == '=')
-        CertainArg = true;
-        
+
       IncI = 1;
    }
    else
@@ -244,20 +238,11 @@ bool CommandLine::HandleOpt(int &I,int argc,const char *argv[],
       // Arbitrary item specification
       if ((A->Flags & ArbItem) == ArbItem)
       {
-        const char *J = strchr(Argument, '=');
-        if (J == NULL)
+        const char * const J = strchr(Argument, '=');
+        if (J == nullptr)
            return _error->Error(_("Option %s: Configuration item specification must have an =<val>."),argv[I]);
 
-        // = is trailing
-        if (J[1] == 0)
-        {
-           if (I+1 >= argc)
-              return _error->Error(_("Option %s: Configuration item specification must have an =<val>."),argv[I]);
-           Conf->Set(string(Argument,J-Argument),string(argv[I++ +1]));
-        }
-        else
-           Conf->Set(string(Argument,J-Argument),string(J+1));
-        
+        Conf->Set(string(Argument,J-Argument), J+1);
         return true;
       }
       

diff --git a/test/integration/framework b/test/integration/framework
index ee67dd52cb83689ad7a1ece8e30c638a3798e9ea..49525cbd091c1d9b6110ee83ad20a20fbe5399bc 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -982,25 +982,13 @@ generatereleasefiles() {
                        local VERSION="$(getreleaseversionfromsuite $SUITE)"
                        local LABEL="$(getlabelfromsuite $SUITE)"
                        local ORIGIN="$(getoriginfromsuite $SUITE)"
-                       if [ -n "$VERSION" ]; then
-                               VERSION="-o APT::FTPArchive::Release::Version=${VERSION}"
-                       fi
-                       if [ -n "$LABEL" ]; then
-                               LABEL="-o APT::FTPArchive::Release::Label=${LABEL}"
-                       fi
-                       if [ -n "$ORIGIN" ]; then
-                               ORIGIN="-o APT::FTPArchive::Release::Origin=${ORIGIN}"
-                       fi
-                       if [ -n "$ARCHITECTURES" ]; then
-                               ARCHITECTURES="-o APT::FTPArchive::Release::Architectures=${ARCHITECTURES}"
-                       fi
                        aptftparchiverelease "$dir" \
                                -o APT::FTPArchive::Release::Suite="${SUITE}" \
                                -o APT::FTPArchive::Release::Codename="${CODENAME}" \
-                               ${ARCHITECTURES} \
-                               ${LABEL} \
-                               ${ORIGIN} \
-                               ${VERSION} \
+                               -o APT::FTPArchive::Release::Architectures="${ARCHITECTURES}" \
+                               -o APT::FTPArchive::Release::Label="${LABEL}" \
+                               -o APT::FTPArchive::Release::Origin="${ORIGIN}" \
+                               -o APT::FTPArchive::Release::Version="${VERSION}" \
                                > "$dir/Release"
                        if [ "$SUITE" = "experimental" -o "$SUITE" = "experimental2" ]; then
                                sed -i '/^Date: / a\

diff --git a/test/libapt/commandline_test.cc b/test/libapt/commandline_test.cc
index d135ddd7b6c9a3bfab46f45e5893d508562c6702..7783c47a4f800e2d6509bbfaf0c6a83e27d2b182 100644 (file)
--- a/test/libapt/commandline_test.cc
+++ b/test/libapt/commandline_test.cc
@@ -38,7 +38,9 @@ TEST(CommandLineTest,Parsing)
 {
    CommandLine::Args Args[] = {
       { 't', 0, "Test::Worked", 0 },
+      { 'T', "testing", "Test::Worked", CommandLine::HasArg },
       { 'z', "zero", "Test::Zero", 0 },
+      { 'o', "option", 0, CommandLine::ArbItem },
       {0,0,0,0}
    };
    ::Configuration c;
@@ -60,6 +62,79 @@ TEST(CommandLineTest,Parsing)
    CmdL.Parse(3 , argv2);
    EXPECT_TRUE(c.FindB("Test::Worked", false));
    EXPECT_FALSE(c.FindB("Test::Zero", false));
+
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-T", "yes" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.FindB("Test::Worked", false));
+   EXPECT_EQ("yes", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(0, CmdL.FileSize());
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-T=yes" };
+   CmdL.Parse(2 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("yes", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(0, CmdL.FileSize());
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-T=", "yes" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("no", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(1, CmdL.FileSize());
+   }
+
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "--testing", "yes" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.FindB("Test::Worked", false));
+   EXPECT_EQ("yes", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(0, CmdL.FileSize());
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "--testing=yes" };
+   CmdL.Parse(2 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("yes", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(0, CmdL.FileSize());
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "--testing=", "yes" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("no", c.Find("Test::Worked", "no"));
+   EXPECT_EQ(1, CmdL.FileSize());
+   }
+
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-o", "test::worked=yes" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.FindB("Test::Worked", false));
+   EXPECT_EQ("yes", c.Find("Test::Worked", "no"));
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-o", "test::worked=" };
+   CmdL.Parse(3 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("no", c.Find("Test::Worked", "no"));
+   }
+   c.Clear("Test");
+   {
+   char const * argv[] = { "test", "-o", "test::worked=", "yes" };
+   CmdL.Parse(4 , argv);
+   EXPECT_TRUE(c.Exists("Test::Worked"));
+   EXPECT_EQ("no", c.Find("Test::Worked", "no"));
+   }
+   c.Clear("Test");
 }
 
 TEST(CommandLineTest, BoolParsing)