[windows] fix npm upgrade bug (#11446)

* fix npm upgrade bug * add helpful comment * Update tools/windows/install-help/cal/customactiondata.cpp Co-authored-by: Julien Lebot <[email protected]> * Update tools/windows/install-help/cal/customactiondata.cpp Co-authored-by: Julien Lebot <[email protected]> Co-authored-by: Julien Lebot <[email protected]>
DataDog · Mar 30, 2022 · 3ba5998 · 3ba5998
1 parent 91012a8
commit 3ba5998
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 15 deletions.
diff --git a/omnibus/resources/agent/msi/source.wxs.erb b/omnibus/resources/agent/msi/source.wxs.erb
@@ -79,6 +79,7 @@
     <Property Id="APIKEY" Hidden="yes" />
     <Property Id="SITE" Value="datadoghq.com" />
     <Property Id="FinalizeInstall" Hidden="yes" />
+    <Property Id="NPMFEATURESTATE" />
      <PropertyRef Id="WIX_ACCOUNT_ADMINISTRATORS" />
      <PropertyRef Id="WIX_ACCOUNT_LOCALSYSTEM" />
      <PropertyRef Id="WIX_ACCOUNT_USERS" />
@@ -101,7 +102,22 @@
     <SetProperty Id="PROJECTLOCATIONONCMDLINESLASH" Value="[PROJECTLOCATION]\" Before="AppSearch">PROJECTLOCATION </SetProperty>
     <SetProperty Id="CONFDIRONCOMMANDLINE" Value="[APPLICATIONDATADIRECTORY]" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
     <SetProperty Id="CONFDIRONCOMMANDLINESLASH" Value="[APPLICATIONDATADIRECTORY]\" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
+
+    <!--
+      The following sets a property (to be used by the custom action).
+      the "&" indicates the action state of a feature (so &NPM refers to the NPM _feature_ not the NPM property that might be set on the command line)
+      "3" refers to the action state "on the local computer" (which is the only one we support besides "not present", which happens to be "2")
+
+      So, the NPMFEATURESTATE property will be set to "off" if the NPM feature is no installed on the local computer
+      and will be set to "on" if it is used on the local computer.  
+
+      This flag will be used in the custom action to decide what service dependencies need to be set
 
+      For more information on this syntax see
+      https://www.firegiant.com/wix/tutorial/com-expression-syntax-miscellanea/expression-syntax/
+      -->
+    <SetProperty Action="SetNPMOff" Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="off" Sequence="execute"><![CDATA[not (&NPM = 3)]]></SetProperty>
+    <SetProperty Action="SetNPMOn"  Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="on"  Sequence="execute"><![CDATA[(&NPM = 3)]]></SetProperty>
     <!-- This condition allows the install if
         1) the user didn't supply the binary directory on the command line
         2) this is NOT an upgrade
@@ -651,6 +667,7 @@
       This does not affect the final artefact, as it seems to affects only the base string, not the expanded one.
       At runtime this limitation doesn\'t seem to apply.
     -->
+
     <CustomAction Id='SetFinalizeProperty' Return='check' Property='FinalizeInstall'
         Value='
             UILevel=[UILevel]
@@ -659,6 +676,7 @@
             DDAGENTUSER_NAME=[DDAGENTUSER_NAME]
             DDAGENTUSER_PASSWORD=[DDAGENTUSER_PASSWORD]
             SYSPROBE_PRESENT=[SYSPROBE_PRESENT]
+            NPMFEATURE=[NPMFEATURESTATE]
             ADDLOCAL=[ADDLOCAL]
             APIKEY=[APIKEY]
             TAGS=[TAGS]

diff --git a/releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml b/releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml
@@ -0,0 +1,11 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    For Windows, fixes problem in upgrade wherein NPM driver is not automatically started by system probe.
diff --git a/tools/windows/install-help/cal/customactiondata.cpp b/tools/windows/install-help/cal/customactiondata.cpp
@@ -145,6 +145,7 @@ bool CustomActionData::parseSysprobeData()
     std::wstring sysprobePresent;
     std::wstring addlocal;
     std::wstring npm;
+    std::wstring npmFeature;
     this->_doInstallSysprobe = false;
     this->_ddnpmPresent = false;
     if (!this->value(L"SYSPROBE_PRESENT", sysprobePresent))
@@ -172,24 +173,20 @@ bool CustomActionData::parseSysprobeData()
         this->_ddnpmPresent = true;
     }
 
-    // now check to see if we're installing the driver
-    if (!this->value(L"ADDLOCAL", addlocal))
+
+    if (this->value(L"NPMFEATURE", npmFeature))
     {
-        // should never happen.  But if the addlocalkey isn't there,
-        // don't bother trying
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL not present");
-
-        return true;
+        // this property is set to "on" or "off" depending on the desired installed state
+        // of the NPM feature.
+        WcaLog(LOGMSG_STANDARD, "NPMFEATURE key is present and (%S)", npmFeature.c_str());
+        if (_wcsicmp(npmFeature.c_str(), L"on") == 0)
+        {
+            this->_ddnpmPresent = true;
+        }
     }
-    WcaLog(LOGMSG_STANDARD, "ADDLOCAL is (%S)", addlocal.c_str());
-    if (_wcsicmp(addlocal.c_str(), L"ALL") == 0)
+    else
     {
-        // installing all components, do it
-        this->_ddnpmPresent = true;
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL is ALL");
-    } else if (addlocal.find(L"NPM") != std::wstring::npos) {
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL contains NPM %S", addlocal.c_str());
-        this->_ddnpmPresent = true;
+        WcaLog(LOGMSG_STANDARD, "NPMFEATURE not present");
     }
 
     return true;