Name and cleanup extra VC files (.pdb, .ilk, .idb)

6 files changed, 162 insertions, 52 deletions

diff --git a/build2/cxx/compile b/build2/cxx/compile
index 5134e78..16c62e6 100644
--- a/build2/cxx/compile
+++ b/build2/cxx/compile
@@ -26,6 +26,9 @@ namespace build2
       static target_state
       perform_update (action, target&);
 
+      static target_state
+      perform_clean (action, target&);
+
       static compile instance;
     };
   }
diff --git a/build2/cxx/compile.cxx b/build2/cxx/compile.cxx
index 25416e8..504df7f 100644
--- a/build2/cxx/compile.cxx
+++ b/build2/cxx/compile.cxx
@@ -223,7 +223,7 @@ namespace build2
       switch (a)
       {
       case perform_update_id: return &perform_update;
-      case perform_clean_id: return &perform_clean_depdb;
+      case perform_clean_id: return &perform_clean;
       default: return noop_recipe; // Configure update.
       }
     }
@@ -1208,7 +1208,7 @@ namespace build2
       append_options (args, t, "cxx.poptions");
       append_options (args, t, "cxx.coptions");
 
-      string std, out; // Storage.
+      string std, out, out1; // Storage.
 
       append_std (args, rs, cid, t, std);
 
@@ -1219,11 +1219,38 @@ namespace build2
         if (verb < 3)
           args.push_back ("/nologo");
 
-        //@@ VC: What is the default value for /MP, should we override?
-
-        // The /Fo: option (object file name) only became available in
-        // VS2013/12.0.
+        // The /F*: option variants with separate names only became available
+        // in VS2013/12.0. Why do we bother? Because the command line suddenly
+        // becomes readable.
+
+        // The presence of /Zi or /ZI causes the compiler to write debug info
+        // to the .pdb file. By default it is a shared file called vcNN.pdb
+        // (where NN is the VC version) created (wait for it) in the current
+        // working directory (and not the directory of the .obj file). Also,
+        // because it is shared, there is a special Windows service that
+        // serializes access. We, of course, want none of that so we will
+        // create a .pdb per object file.
+        //
+        // Note that this also changes the name of the .idb file (used for
+        // minimal rebuild and incremental compilation) by taking /Fd value
+        // replacing the .pdb extension to .idb.
+        //
+        // Note also that what we are doing here appears to be incompatible
+        // with PCH (/Y* options) and /Gm (minimal rebuild).
         //
+        if (find_option ("/Zi", args) || find_option ("/ZI", args))
+        {
+          if (cver >= 18)
+            args.push_back ("/Fd:");
+          else
+            out1 = "/Fd";
+
+          out1 += relo.string ();
+          out1 += ".pdb";
+
+          args.push_back (out1.c_str ());
+        }
+
         if (cver >= 18)
         {
           args.push_back ("/Fo:");
@@ -1303,6 +1330,24 @@ namespace build2
       }
     }
 
+    target_state compile::
+    perform_clean (action a, target& xt)
+    {
+      file& t (static_cast<file&> (xt));
+
+      scope& rs (t.root_scope ());
+      const string& cid (cast<string> (rs["cxx.id"]));
+
+      initializer_list<const char*> e;
+
+      if (cid == "msvc")
+        e = {"+.d", "+.idb", "+.pdb"};
+      else
+        e = {"+.d"};
+
+      return clean_extra (a, t, e);
+    }
+
     compile compile::instance;
   }
 }
diff --git a/build2/cxx/link.cxx b/build2/cxx/link.cxx
index 4759a78..7a527e0 100644
--- a/build2/cxx/link.cxx
+++ b/build2/cxx/link.cxx
@@ -1206,6 +1206,8 @@ namespace build2
           append_std (args, rs, cid, t, std);
         }
 
+        append_options (args, t, "cxx.loptions");
+
         // Handle soname/rpath.
         //
         if (tclass == "windows")
@@ -1295,9 +1297,6 @@ namespace build2
         for (size_t i (0); i != sargs.size (); ++i)
           cs.append (sargs[i]);
 
-        if (lt != type::a)
-          hash_options (cs, t, "cxx.loptions");
-
         if (dd.expect (cs.string ()) != nullptr)
           l4 ([&]{trace << "options mismatch forcing update of " << t;});
       }
@@ -1365,7 +1364,7 @@ namespace build2
 
       // Ok, so we are updating. Finish building the command line.
       //
-      string out; // Storage.
+      string out, out1; // Storage.
 
       // Translate paths to relative (to working directory) ones. This results
       // in easier to read diagnostics.
@@ -1385,6 +1384,22 @@ namespace build2
             if (verb < 3)
               args.push_back ("/NOLOGO");
 
+            // Unless explicitly enabled with /INCREMENTAL, disable
+            // incremental linking (it is implicitly enabled if /DEBUG is
+            // specified). The reason is the .ilk file: its name cannot be
+            // changed and if we have, say, foo.exe and foo.dll, then they
+            // will end up stomping on each other's .ilk's.
+            //
+            // So the idea is to disable it by default but let the user
+            // request it explicitly if they are sure their project doesn't
+            // suffer from the above issue. We can also have something like
+            // 'incremental' config initializer keyword for this.
+            //
+            // It might also be a good idea to ask Microsoft to add an option.
+            //
+            if (!find_option ("/INCREMENTAL", args, true))
+              args.push_back ("/INCREMENTAL:NO");
+
             // Take care of the manifest.
             //
             if (!manifest.empty ())
@@ -1395,15 +1410,21 @@ namespace build2
               args.push_back (std.c_str ());
             }
 
+            // If we have /DEBUG then name the .pdb file. We call it
+            // foo.exe.pdb rather than foo.pdb because we can have, say,
+            // foo.dll in the same directory.
+            //
+            if (find_option ("/DEBUG", args, true))
+            {
+              out1 = "/PDB:" + relt.string () + ".pdb";
+              args.push_back (out1.c_str ());
+            }
+
+            // @@ An executable can have an import library and VS seems to
+            //    always name it. I wonder what would trigger its generation?
+
             out = "/OUT:" + relt.string ();
             args.push_back (out.c_str ());
-
-            //@@ VC: /PDB: if /DEBUG is specified, then created. By default
-            //   is basename.pdb. Do we want it to be basename.exe.pdb?
-            //   Probably not, too unconventional. Need to clean it though.
-            //
-            //   In a similar vein, we may also be generating an import lib
-            //   for the executable.
           }
           else
           {
@@ -1475,8 +1496,6 @@ namespace build2
         }
       }
 
-      size_t oend (sargs.size ()); // Note the end of options in sargs.
-
       for (target* pt: t.prerequisite_targets)
       {
         path_target* ppt;
@@ -1508,12 +1527,7 @@ namespace build2
       // Because of potential reallocations.
       //
       for (size_t i (0); i != sargs.size (); ++i)
-      {
-        if (lt != type::a && i == oend)
-          append_options (args, t, "cxx.loptions");
-
         args.push_back (sargs[i].c_str ());
-      }
 
       if (lt != type::a)
         append_options (args, t, "cxx.libs");
@@ -1608,29 +1622,49 @@ namespace build2
     {
       file& t (static_cast<file&> (xt));
 
-      type lt (link_type (t));
-
       scope& rs (t.root_scope ());
       const string& tsys (cast<string> (rs["cxx.target.system"]));
       const string& tclass (cast<string> (rs["cxx.target.class"]));
 
-      // @@ VC .pdb, etc cleanup -- maybe make them ad hoc group members?
-      //    One might want to install PDBs.
-      //
+      initializer_list<const char*> e;
 
-      // On Windows we need to clean up manifest business.
-      //
-      if (lt == type::e && tclass == "windows")
+      switch (link_type (t))
       {
-        return clean_extra (a,
-                            t,
-                            {"+.d",
-                             "/+.dlls",
-                             tsys == "mingw32" ? "+.manifest.o" : nullptr,
-                             "+.manifest"});
+      case type::a:
+        {
+          e = {"+.d"};
+          break;
+        }
+      case type::e:
+        {
+          if (tclass == "windows")
+          {
+            if (tsys == "mingw32")
+            {
+              e = {"+.d", "/+.dlls", "+.manifest.o", "+.manifest"};
+            }
+            else
+            {
+              // Assuming it's VC or alike.
+              //
+              // Clean up .ilk in case the user enabled incremental linking.
+              //
+              e = {"+.d", "/+.dlls", "+.manifest", ".ilk", "+.pdb"};
+            }
+          }
+          else
+            e = {"+.d"};
+
+          break;
+        }
+      case type::so:
+        {
+          e = {"+.d"};
+          break;
+        }
       }
-      else
-        return clean_extra (a, t, {"+.d"});
+
+      return clean_extra (a, t, e);
     }
 
     link link::instance;
diff --git a/build2/utility b/build2/utility
index e7674c0..a5c4407 100644
--- a/build2/utility
+++ b/build2/utility
@@ -164,15 +164,24 @@ namespace build2
   void
   hash_options (sha256&, const strings&);
 
-  // Check if a specified option is present in the variable value.
+  // Check if a specified option is present in the variable or value.
   // T is either target or scope.
   //
   template <typename T>
   bool
-  find_option (const char* option, T&, const char* variable);
+  find_option (const char* option,
+               T&,
+               const char* variable,
+               bool ignore_case = false);
 
   bool
-  find_option (const char* option, const lookup&);
+  find_option (const char* option, const lookup&, bool ignore_case = false);
+
+  bool
+  find_option (const char* option, const strings&, bool ignore_case = false);
+
+  bool
+  find_option (const char* option, const cstrings&, bool ignore_case = false);
 
   // Parse version string in the X.Y.Z[-{a|b}N] to a version integer in the
   // AABBCCDD form, where:
diff --git a/build2/utility.cxx b/build2/utility.cxx
index 8b772e0..5fcdebf 100644
--- a/build2/utility.cxx
+++ b/build2/utility.cxx
@@ -177,15 +177,34 @@ namespace build2
   }
 
   bool
-  find_option (const char* option, const lookup& l)
+  find_option (const char* option, const lookup& l, bool ic)
   {
-    if (l)
+    return l && find_option (option, cast<strings> (l), ic);
+  }
+
+  bool
+  find_option (const char* option, const strings& strs, bool)
+  {
+    //@@ TODO ignore case
+
+    for (const string& s: strs)
     {
-      for (const string& s: cast<strings> (l))
-      {
-        if (s == option)
-          return true;
-      }
+      if (s == option)
+        return true;
+    }
+
+    return false;
+  }
+
+  bool
+  find_option (const char* option, const cstrings& cstrs, bool)
+  {
+    //@@ TODO ignore case
+
+    for (const char* s: cstrs)
+    {
+      if (s != nullptr && strcmp (s, option) == 0)
+        return true;
     }
 
     return false;
diff --git a/build2/utility.ixx b/build2/utility.ixx
index d42d6fd..d215ea3 100644
--- a/build2/utility.ixx
+++ b/build2/utility.ixx
@@ -50,8 +50,8 @@ namespace build2
 
   template <typename T>
   inline bool
-  find_option (const char* option, T& s, const char* var)
+  find_option (const char* option, T& s, const char* var, bool ic)
   {
-    return find_option (option, s[var]);
+    return find_option (option, s[var], ic);
   }
 }