Custom action library, initial version

installer/src/installer-lib/database.cpp

Index: installer/src/installer-lib/database.cpp
===================================================================
new file mode 100644
--- /dev/null
+++ b/installer/src/installer-lib/database.cpp
@@ -0,0 +1,58 @@
+/**
+ * \file database.h MSI database
+ */
+
+#include "database.h"
+#include "msiquery.h"
+
+//-----------------------------------------------------------------------------------------
+// Database
+//-----------------------------------------------------------------------------------------
+/**
+ * \par Implementation Notes
+ *    An MSI database handle is an overloaded type, used both for installation databases and one opened outside an installation.
+ *    Hence this base constructor initializes with that handle.
+ * 
+ * \sa MSDN "Obtaining a Database Handle"
+ *    http://msdn.microsoft.com/en-us/library/windows/desktop/aa370541(v=vs.85).aspx
+ */
+Database::Database( MSIHANDLE handle )
+  : handle( handle )
+{
+}
+
+Database::~Database()
+{
+  MsiCloseHandle( handle ) ;
+}

[Wladimir Palant, 2013/10/29 08:49:26]

I cannot really imagine the situation where our custom action would open any
database but the one belonging to the current installation. This means that we
can definitely simplify the logic here:

Database constructor takes a session reference. It also calls
MsiGetActiveDatabase() to get the database handle and it is obvious why the
destructor closes that handle then. Neither get_active_database() nor a derived
Installation_Database class are necessary.

Note that I generally don't accept the "we might need it at some point" as a
valid argument when introducing complexity into the codebase. If we don't know
right now why we will need it then we don't need it. If the requirements change
later we can still make the approach more complex then - but we don't have the
increased maintenance burden in the meantime.

Frankly, this same logic might apply to the Database object itself - we aren't
developing a generic solution, reading from the database should only be
necessary if we want the custom action to be configurable, something that IMHO
isn't a requirement.

[Eric, 2013/10/29 14:00:58]

Before I get into everything, let me start off by saying that if this code were
only appear only in the CA, I would completely agree and we'd have a single
class representing the installation database. Yet the bad news is that we may
need this outside the CA. The reason for a database class hierarchy is that
there's a decent chance we'll need to build some custom build tools. Initially,
I wrote this just to wrap my head around the Windows Installer API, which makes
a bunch of Windows-like assumptions. Later, I realized we might need to keep it.

Here's a brief version why.

1) Getting automatic translation means generating a transform file and
incorporating it into an MSI file.

2) The existing tools to do this have deficits.
2a) There's no WiX tool for one piece, and the WiX tool for another has a
defect.
2b) The Microsoft tools, while freely available, don't seem to be packaged by
default in Visual Studio (at least the Express edition). That means incurring
yet another external dependency. The tool suite is called "Windows Installer
Development Tools"
http://msdn.microsoft.com/en-us/library/windows/desktop/aa372834%28v=vs.85%29...
. From that page:
2c) There are some tools out there (both Microsoft and independent) that use the
Automation interface with the Windows Scripting Host. While I may be willing to
use Microsoft NMAKE, this is beyond even me.

3) All these tools seem to be largely wrappers around the Windows Installer API.
For example, the tool "msitran.exe" (which generates transforms, MST files)
seems to be mostly a wrapper around the API call MsiDatabaseGenerateTransform.
All the heavy lifting is already done, so it looks to be feasible just to write
our own wrappers.

4) None of the command line tools are targeted at the use case we have. That
means we'll need some kind of top-level logic for the build regardless. We can
put it into the build system (with a script, in all likelihood) or into our own
tool. If we want to avoid a new dependency to the code base and end up writing
our own tool, there's no need to script it for the build system separately.

5) The Windows Installer API uses a single kind of handle for anything involving
a database, whether open during a CA or in a tool. Some of the API calls that
take databases overlap, some don't.

Therefore, given the possibility that we need tools, I spent a small bit of time
putting in the stubs for a class hierarchy, but more importantly, writing
documentation for what was happening for future recall.

If we end up needing our own tools, we'll want the code. If we don't, we can
remove it.

(Note: This comment likely deserves to be copied into Discourse.)

[Wladimir Palant, 2013/10/29 15:06:38]

Ok, I understand it that we will need a tool to generate MST files. But is it
really a good idea to write that tool in C++? I would prefer Python (with
ctypes) which is a more maintainable option.

Even *if* we want to use C++ for that, I don't see why we want to share code
between custom actions and that tool. From all I can tell, there is exactly zero
overlap between the MSI functions that these need to use.

Feel free to move the general discussion to Discourse.

+
+//-----------------------------------------------------------------------------------------
+// Installation_Database
+//-----------------------------------------------------------------------------------------
+
+/**
+ * Helper function for Installation_Database constructor.
+ *
+ * \par Resource Allocator
+ *    Return value of this function, a handle, must be released in order to avoid a resource leak.
+ *    Passing it as an argument to the Database constructor is adequate.
+ */
+MSIHANDLE get_active_database( Immediate_Session & session )
+{
+  MSIHANDLE h = MsiGetActiveDatabase( session.handle ) ;
+  if ( h == 0 )
+  {
+    throw std::runtime_error( "Failed to retrieve active databases" ) ;
+  }
+  return h ;
+}
+
+/**
+ * \par Implementation Notes
+ *    The only thing this constructor needs to do is to initialize the base class.
+ */
+Installation_Database::Installation_Database( Immediate_Session & session )
+  : Database( get_active_database( session ) )
+{
+  // empty body
+}