allow options between command and -- on commandline

This used to work before we implemented a stricter commandline parser
and e.g. the dd-schroot-cmd command constructs commandlines like this.

Reported-By: Helmut Grohne

diff --git a/apt-pkg/contrib/cmndline.cc b/apt-pkg/contrib/cmndline.cc
index 3799c82..93c1f46 100644 (file)
--- a/apt-pkg/contrib/cmndline.cc
+++ b/apt-pkg/contrib/cmndline.cc
@@ -47,23 +47,26 @@ CommandLine::~CommandLine()
 char const * CommandLine::GetCommand(Dispatch const * const Map,
       unsigned int const argc, char const * const * const argv)
 {
-   // if there is a -- on the line there must be the word we search for around it
-   // as -- marks the end of the options, just not sure if the command can be
-   // considered an option or not, so accept both
+   // if there is a -- on the line there must be the word we search for either
+   // before it (as -- marks the end of the options) or right after it (as we can't
+   // decide if the command is actually an option, given that in theory, you could
+   // have parameters named like commands)
    for (size_t i = 1; i < argc; ++i)
    {
       if (strcmp(argv[i], "--") != 0)
         continue;
-      ++i;
-      if (i < argc)
+      // check if command is before --
+      for (size_t k = 1; k < i; ++k)
         for (size_t j = 0; Map[j].Match != NULL; ++j)
-           if (strcmp(argv[i], Map[j].Match) == 0)
+           if (strcmp(argv[k], Map[j].Match) == 0)
               return Map[j].Match;
-      i -= 2;
-      if (i != 0)
+      // see if the next token after -- is the command
+      ++i;
+      if (i < argc)
         for (size_t j = 0; Map[j].Match != NULL; ++j)
            if (strcmp(argv[i], Map[j].Match) == 0)
               return Map[j].Match;
+      // we found a --, but not a command
       return NULL;
    }
    // no --, so search for the first word matching a command

diff --git a/test/libapt/commandline_test.cc b/test/libapt/commandline_test.cc
index e403a28..627f1b4 100644 (file)
--- a/test/libapt/commandline_test.cc
+++ b/test/libapt/commandline_test.cc
@@ -2,6 +2,7 @@
 
 #include <apt-pkg/cmndline.h>
 #include <apt-pkg/configuration.h>
+#include <apt-private/private-cmndline.h>
 
 #include <gtest/gtest.h>
 
@@ -85,3 +86,70 @@ TEST(CommandLineTest, BoolParsing)
    }
 
 }
+
+bool DoVoid(CommandLine &) { return false; }
+
+TEST(CommandLineTest,GetCommand)
+{
+   CommandLine::Dispatch Cmds[] = { {"install",&DoVoid}, {"remove", &DoVoid}, {0,0} };
+   {
+   char const * argv[] = { "apt-get", "-t", "unstable", "remove", "-d", "foo" };
+   char const * com = CommandLine::GetCommand(Cmds, sizeof(argv)/sizeof(argv[0]), argv);
+   EXPECT_STREQ("remove", com);
+   std::vector<CommandLine::Args> Args = getCommandArgs("apt-get", com);
+   ::Configuration c;
+   CommandLine CmdL(Args.data(), &c);
+   ASSERT_TRUE(CmdL.Parse(sizeof(argv)/sizeof(argv[0]), argv));
+   EXPECT_EQ(c.Find("APT::Default-Release"), "unstable");
+   EXPECT_TRUE(c.FindB("APT::Get::Download-Only"));
+   ASSERT_EQ(2, CmdL.FileSize());
+   EXPECT_EQ(std::string(CmdL.FileList[0]), "remove");
+   EXPECT_EQ(std::string(CmdL.FileList[1]), "foo");
+   }
+   {
+   char const * argv[] = {"apt-get", "-t", "unstable", "remove", "--", "-d", "foo" };
+   char const * com = CommandLine::GetCommand(Cmds, sizeof(argv)/sizeof(argv[0]), argv);
+   EXPECT_STREQ("remove", com);
+   std::vector<CommandLine::Args> Args = getCommandArgs("apt-get", com);
+   ::Configuration c;
+   CommandLine CmdL(Args.data(), &c);
+   ASSERT_TRUE(CmdL.Parse(sizeof(argv)/sizeof(argv[0]), argv));
+   EXPECT_EQ(c.Find("APT::Default-Release"), "unstable");
+   EXPECT_FALSE(c.FindB("APT::Get::Download-Only"));
+   ASSERT_EQ(3, CmdL.FileSize());
+   EXPECT_EQ(std::string(CmdL.FileList[0]), "remove");
+   EXPECT_EQ(std::string(CmdL.FileList[1]), "-d");
+   EXPECT_EQ(std::string(CmdL.FileList[2]), "foo");
+   }
+   {
+   char const * argv[] = {"apt-get", "-t", "unstable", "--", "remove", "-d", "foo" };
+   char const * com = CommandLine::GetCommand(Cmds, sizeof(argv)/sizeof(argv[0]), argv);
+   EXPECT_STREQ("remove", com);
+   std::vector<CommandLine::Args> Args = getCommandArgs("apt-get", com);
+   ::Configuration c;
+   CommandLine CmdL(Args.data(), &c);
+   ASSERT_TRUE(CmdL.Parse(sizeof(argv)/sizeof(argv[0]), argv));
+   EXPECT_EQ(c.Find("APT::Default-Release"), "unstable");
+   EXPECT_FALSE(c.FindB("APT::Get::Download-Only"));
+   ASSERT_EQ(CmdL.FileSize(), 3);
+   EXPECT_EQ(std::string(CmdL.FileList[0]), "remove");
+   EXPECT_EQ(std::string(CmdL.FileList[1]), "-d");
+   EXPECT_EQ(std::string(CmdL.FileList[2]), "foo");
+   }
+   {
+   char const * argv[] = {"apt-get", "install", "-t", "unstable", "--", "remove", "-d", "foo" };
+   char const * com = CommandLine::GetCommand(Cmds, sizeof(argv)/sizeof(argv[0]), argv);
+   EXPECT_STREQ("install", com);
+   std::vector<CommandLine::Args> Args = getCommandArgs("apt-get", com);
+   ::Configuration c;
+   CommandLine CmdL(Args.data(), &c);
+   ASSERT_TRUE(CmdL.Parse(sizeof(argv)/sizeof(argv[0]), argv));
+   EXPECT_EQ(c.Find("APT::Default-Release"), "unstable");
+   EXPECT_FALSE(c.FindB("APT::Get::Download-Only"));
+   ASSERT_EQ(CmdL.FileSize(), 4);
+   EXPECT_EQ(std::string(CmdL.FileList[0]), "install");
+   EXPECT_EQ(std::string(CmdL.FileList[1]), "remove");
+   EXPECT_EQ(std::string(CmdL.FileList[2]), "-d");
+   EXPECT_EQ(std::string(CmdL.FileList[3]), "foo");
+   }
+}

diff --git a/test/libapt/makefile b/test/libapt/makefile
index 69a13fd..7f23ace 100644 (file)
--- a/test/libapt/makefile
+++ b/test/libapt/makefile
@@ -14,8 +14,8 @@ test: $(BIN)/gtest$(BASENAME)
 $(BIN)/gtest$(BASENAME): $(LIB)/gtest.a
 
 PROGRAM = gtest${BASENAME}
-SLIBS = -lapt-pkg -pthread $(LIB)/gtest.a
-LIB_MAKES = apt-pkg/makefile
+SLIBS = -lapt-pkg -lapt-private -pthread $(LIB)/gtest.a
+LIB_MAKES = apt-pkg/makefile apt-private/makefile
 SOURCE = gtest_runner.cc $(wildcard *-helpers.cc *_test.cc)
 include $(PROGRAM_H)