Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Centralize installation logic & reduce admin reqs #11

Merged
merged 2 commits into from
Mar 10, 2022
Merged

Centralize installation logic & reduce admin reqs #11

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfba2b6
Centralize installation logic & reduce admin reqs
jugglinmike Dec 15, 2021
f960cf1
fixup! Centralize installation logic & reduce admin reqs
jugglinmike Mar 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions src/automationttsengine/AutomationTtsEngine.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,6 @@
<EnableCOMDATFolding>true</EnableCOMDATFolding>
<TargetMachine>MachineX86</TargetMachine>
</Link>
<PostBuildEvent>
<Message>Performing Registration</Message>
<Command>regsvr32 /s /c "$(TargetPath)"</Command>
</PostBuildEvent>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzgoddard I'm still only guessing about why gh-9 removes this registration, but I hesitated to accept the removal in that context because it's important for the C++ development workflow.

Since this patch relocates the registration to MakeVoice.exe, and since that doesn't be appear to be used in gh-9, I think this will address both of our needs. And (as noted in the commit message) it gets us closer to a solution where npm install just shells out to MakeVoice.exe. Although I wonder if that will have a negative impact on what you're trying to achieve in gh-9.

Copy link
Contributor

@mzgoddard mzgoddard Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back through the Actions logs for the runs testing the testing pr https://github.com/bocoup/aria-at-automation-driver/pull/9. Run 9 and 10 exited that registration building ttseng with code 3. Immediate searches on microsoft docs and regsvr32 didn't really help with what code 3 means. One commit in the process of debugging that issue made that removal. The next was able to build ttseng and Vocalizer and complete the other action steps.

Run 9: https://github.com/bocoup/aria-at-automation-driver/runs/4511094728?check_suite_focus=true#step:4:6386
Run 10: https://github.com/bocoup/aria-at-automation-driver/runs/4511137068?check_suite_focus=true#step:4:6386
Run 11: https://github.com/bocoup/aria-at-automation-driver/runs/4511238540?check_suite_focus=true#step:4:6444
regsvr32 doc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall from the difficulty getting the dll working on an image I created that the logging functionality I added to the dll locally that it'd initialize the dll scoped logging class instance when the dll is loaded. Not that logging functionality but some other change made by regsvr32 called while building ttseng encountered an error in that context.

A result from a quick google search seems to confirm that:

If something goes wrong, the regsvr32 program reports the error details to the user, and the exit code summarizes which step it got stuck at.
...
What do the various regsvr32 exit codes mean?

Exit code Meaning
3 LoadLibrary failed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some added clarity, ttseng is registered in a later step by npm ci.

https://github.com/bocoup/aria-at-automation-driver/runs/4511238540?check_suite_focus=true#step:7:28

</ItemDefinitionGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
<Midl>
Expand All @@ -196,10 +192,6 @@
<EnableCOMDATFolding>true</EnableCOMDATFolding>
<TargetMachine>MachineX64</TargetMachine>
</Link>
<PostBuildEvent>
<Message>Performing Registration</Message>
<Command>regsvr32 /s /c "$(TargetPath)"</Command>
</PostBuildEvent>
</ItemDefinitionGroup>
<ItemGroup>
<ClCompile Include="AutomationTtsEngine.cpp" />
Expand Down
52 changes: 50 additions & 2 deletions src/makevoice/MakeVoice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,59 @@ std::string getSiblingFilePath(const std::string& fileName)
return path.substr(0, 1 + path.find_last_of('\\')) + fileName;
}

HRESULT registerDll(const std::string& fileName)
{
USES_CONVERSION;

STARTUPINFO startup_info;
PROCESS_INFORMATION process_info;
ZeroMemory(&startup_info, sizeof(startup_info));
startup_info.cb = sizeof(startup_info);
ZeroMemory(&process_info, sizeof(process_info));
std::string command = std::string("regsvr32 /s /c \"") + getSiblingFilePath(fileName) + "\"";
DWORD dwFlags = CREATE_NO_WINDOW;

bool result = CreateProcessW(
NULL,
A2W(command.c_str()),
NULL,
NULL,
false,
dwFlags,
NULL,
NULL,
&startup_info,
&process_info
);

if (!result)
{
return E_FAIL;
}

WaitForSingleObject(process_info.hProcess, INFINITE);

DWORD exitCode;

if (!GetExitCodeProcess(process_info.hProcess, &exitCode))
{
return E_FAIL;
}

CloseHandle(process_info.hProcess);
CloseHandle(process_info.hThread);

return exitCode == 0 ? S_OK : E_FAIL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log the exit code in some way if it non-zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! Done in a "fixup" commit.

}

int wmain(int argc, __in_ecount(argc) WCHAR* argv[])
{
HRESULT hr = S_OK;
HRESULT hr = ::CoInitialize( NULL );

::CoInitialize( NULL );
if (SUCCEEDED(hr))
{
hr = registerDll("AutomationTtsEngine.dll");
}

// Programatically create a token for the new voice and set its attributes.
if (SUCCEEDED(hr))
Expand Down
1 change: 1 addition & 0 deletions src/makevoice/MakeVoice.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
<OptimizeReferences>true</OptimizeReferences>
<EnableCOMDATFolding>true</EnableCOMDATFolding>
<TargetMachine>MachineX86</TargetMachine>
<UACExecutionLevel>RequireAdministrator</UACExecutionLevel>
</Link>
<PostBuildEvent>
<Message>Build and register voice file</Message>
Expand Down