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.

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

Add logger, improve support for ARM64 & Windows 7 #13

Merged
merged 5 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions fabric-installer-native-bootstrap.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,15 @@
</ItemDefinitionGroup>
<ItemGroup>
<ClCompile Include="src\Bootstrap.cpp" />
<ClCompile Include="src\Logger.cpp" />
<ClCompile Include="src\main.cpp" />
<ClCompile Include="src\SystemHelper.cpp" />
</ItemGroup>
<ItemGroup>
<ClInclude Include="src\Bootstrap.h" />
<ClInclude Include="src\ISystemHelper.h" />
<ClInclude Include="resource.h" />
<ClInclude Include="src\Logger.h" />
<ClInclude Include="src\SystemHelper.h" />
</ItemGroup>
<ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions fabric-installer-native-bootstrap.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
<ClCompile Include="src\SystemHelper.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="src\Logger.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="src\Bootstrap.h">
Expand All @@ -38,6 +41,9 @@
<ClInclude Include="resource.h">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="src\Logger.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="fabric-installer-native-bootstrap.rc">
Expand Down
122 changes: 86 additions & 36 deletions src/Bootstrap.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "Bootstrap.h"

#include <iostream>
#include <vector>
#include <sstream>

namespace {
constexpr LPCWSTR ERROR_TITLE = L"Fabric Installer";
Expand All @@ -11,23 +11,18 @@ namespace {
constexpr LPCWSTR MC_LAUNCH_REG_PATH = LR"(SOFTWARE\Mojang\InstalledProducts\Minecraft Launcher)";
constexpr LPCWSTR MC_LAUNCH_REG_KEY = L"InstallLocation";

constexpr const LPCWSTR UWP_PATH = LR"(\Packages\Microsoft.4297127D64EC6_8wekyb3d8bbwe\LocalCache\Local\)";

static const std::vector<LPCWSTR> MC_JAVA_PATHS = {
LR"(runtime\java-runtime-gamma\windows-x64\java-runtime-gamma\bin\javaw.exe)", // Java 17.0.8
LR"(runtime\java-runtime-gamma\windows-x86\java-runtime-gamma\bin\javaw.exe)",
LR"(runtime\java-runtime-beta\windows-x64\java-runtime-beta\bin\javaw.exe)", // Java 17.0.1
LR"(runtime\java-runtime-beta\windows-x86\java-runtime-beta\bin\javaw.exe)",
LR"(runtime\java-runtime-alpha\windows-x64\java-runtime-alpha\bin\javaw.exe)", // Java 16
LR"(runtime\java-runtime-alpha\windows-x86\java-runtime-alpha\bin\javaw.exe)",
LR"(runtime\jre-legacy\windows-x64\jre-legacy\bin\javaw.exe)", // Java 8 new location
LR"(runtime\jre-legacy\windows-x86\jre-legacy\bin\javaw.exe)",
LR"(runtime\jre-x64\bin\javaw.exe)", // Java 8 old location
LR"(runtime\jre-x86\bin\javaw.exe)",
constexpr LPCWSTR UWP_PATH = LR"(\Packages\Microsoft.4297127D64EC6_8wekyb3d8bbwe\LocalCache\Local\)";

// Find these here: https://piston-meta.mojang.com/v1/products/java-runtime/2ec0cc96c44e5a76b9c8b7c39df7210883d12871/all.json
static const std::vector<LPCWSTR> JAVA_NAMES = {
L"java-runtime-gamma", // Java 17.0.8
L"java-runtime-beta", // Java 17.0.1
L"java-runtime-alpha", // Java 16
L"jre-legacy", // Java 8
};
}

Bootstrap::Bootstrap(const std::shared_ptr<ISystemHelper>& systemHelper) : systemHelper(systemHelper) {}
Bootstrap::Bootstrap(ISystemHelper& systemHelper, Logger& logger) : systemHelper(systemHelper), logger(logger) {}

void Bootstrap::launch() {
bool launched;
Expand All @@ -44,92 +39,147 @@ void Bootstrap::launch() {
}

bool Bootstrap::launchMinecraftLauncher() {
if (auto minecraftLauncherPath = systemHelper->getRegValue(HKEY_CURRENT_USER, MC_LAUNCH_REG_PATH, MC_LAUNCH_REG_KEY); minecraftLauncherPath) {
const HostArchitecture::Value hostArch = systemHelper.getHostArchitecture();
logger.log(L"Host archiecture: " + HostArchitecture::AsString(hostArch));

const auto javaPaths = getMinecraftJavaPaths(hostArch);

if (auto minecraftLauncherPath = systemHelper.getRegValue(HKEY_CURRENT_USER, MC_LAUNCH_REG_PATH, MC_LAUNCH_REG_KEY); minecraftLauncherPath) {
std::wstring installPath = minecraftLauncherPath.value();

// This is weird, on my tests machine the reg key value is just "C:\Program Files (x86)\"
if (!installPath.ends_with(LR"(Minecraft Launcher\)")) {
installPath = installPath + LR"(Minecraft Launcher\)";
std::wcout << "Install path did not appear to be Minecraft, appending guess." << std::endl;
}

std::wcout << "Found Minecraft launcher installation path: " << installPath << std::endl;
logger.log(L"Minecraft launcher installation path: " + installPath);

for (const LPCWSTR path : MC_JAVA_PATHS) {
for (const auto& path : javaPaths) {
const std::wstring fullPath = installPath + path;
if (attemptLaunch(fullPath, true)) {
return true;
}
}
}
else {
std::wcout << "Could not find minecraft launcher installation in registry.: " << std::endl;
logger.log(L"Failed to find minecraft launcher installation directory in registry.");
}

// Check %LOCALAPPDATA% for the UWP installer
if (auto localAppData = systemHelper->getEnvVar(L"LOCALAPPDATA"); localAppData) {
if (auto localAppData = systemHelper.getEnvVar(L"LOCALAPPDATA"); localAppData) {
std::wstring launcherPath = localAppData.value() + UWP_PATH;

if (systemHelper->dirExists(launcherPath)) {
for (const LPCWSTR path : MC_JAVA_PATHS) {
if (systemHelper.dirExists(launcherPath)) {
for (const auto& path : javaPaths) {
if (attemptLaunch(launcherPath + path, true)) {
return true;
}
}
}
else { std::wcout << "Could not find minecraft UWP launcher directory.: " << launcherPath << std::endl; }
else {
logger.log(L"Did not find Minecraft UWP at " + launcherPath);
}
}
else {
// Something has gone really wrong :)
throw std::runtime_error("Failed to get LOCALAPPDATA env var!");
}

logger.log(L"Failed to launch using Java from the Minecraft Launcher");

return false;
}

bool Bootstrap::launchSystemJava() {
// Check %JAVA_HOME% for system java
if (auto javaHome = systemHelper->getEnvVar(L"JAVA_HOME"); javaHome) {
if (auto javaHome = systemHelper.getEnvVar(L"JAVA_HOME"); javaHome) {
logger.log(L"JAVA_HOME = " + javaHome.value());
std::wstring path = javaHome.value() + LR"(bin\javaw.exe)";
if (attemptLaunch(path, true)) {
return true;
}
}
else {
std::wcout << "Could not find JAVA_HOME env var" << std::endl;
logger.log("Could not find JAVA_HOME env var");
}

std::wcout << "Trying java on the path" << std::endl;
logger.log("Trying Java from the system path");
return attemptLaunch(L"javaw.exe", false);
}

void Bootstrap::showErrorMessage() {
std::wcout << "Failed to launch showing error dialog" << std::endl;
systemHelper->showErrorMessage(ERROR_TITLE, ERROR_MESSAGE, ERROR_URL);
logger.log("Failed to launch showing error dialog");
systemHelper.showErrorMessage(ERROR_TITLE, ERROR_MESSAGE, ERROR_URL);
}

bool Bootstrap::attemptLaunch(const std::wstring& path, bool checkExists) {
if (checkExists) {
if (!systemHelper->fileExists(path)) {
std::wcout << "Java path (" << path << ") does not exist" << std::endl;
if (!systemHelper.fileExists(path)) {
logger.log(L"Java path (" + path + L") does not exist");
return false;
}
}

std::wcout << "Testing for valid java @ (" << path << ")" << std::endl;
DWORD exit = systemHelper->createProcess({ path, L"-version" });
logger.log(L"Testing for valid java at (" + path + L")");
DWORD exit = systemHelper.createProcess({ path, L"-version" });
if (exit != 0) {
std::wcout << "Java @ (" << path << ") returned an exit code of: " << std::to_wstring(exit) << std::endl;
logger.log(L"Java at (" + path + L") returned an exit code of: " + std::to_wstring(exit));
return false;
}

std::wcout << "Found valid java @ (" << path << ")" << std::endl;
logger.log(L"Found valid Java path (" + path + L")");

exit = systemHelper->createProcess({ path, L"-jar", systemHelper->getBootstrapFilename(), L"-fabricInstallerBootstrap", L"true" });
exit = systemHelper.createProcess({ path, L"-jar", systemHelper.getBootstrapFilename(), L"-fabricInstallerBootstrap", L"true" });
if (exit != 0) {
// The installer returned a none 0 exit code, meaning that most likely the installer crashed.
logger.log(L"Installer failed or crashed, exit code: " + std::to_wstring(exit));
throw std::runtime_error("Installer returned a none 0 exit code: " + exit);
}

return true;
}

// Return all of the possible java paths, starting with the newest on the host platform, down to the oldest on the none host platforms.
const std::vector<std::wstring> Bootstrap::getMinecraftJavaPaths(const HostArchitecture::Value& hostArch) {
std::vector<std::wstring> paths;

for (const HostArchitecture::Value& arch : HostArchitecture::VALUES) {
if (arch == HostArchitecture::UNKNOWN) {
continue;
}

if (arch < hostArch) {
// Skip arches that the host does not support.
// E.g: On x64 there is no need to go looking for arm64 JDKs as its never going to run.
logger.log(L"Arch not supported: " + HostArchitecture::AsString(arch));
continue;
}

std::wstring javaName;

switch (arch) {
case HostArchitecture::X64:
javaName = L"windows-x64";
break;
case HostArchitecture::ARM64:
javaName = L"windows-arm64";
break;
case HostArchitecture::X86:
javaName = L"windows-x86";
break;
default:
continue;
}

for (const LPCWSTR& name : JAVA_NAMES) {
std::wstringstream buffer;
// runtime\java-runtime-gamma\windows-x64\java-runtime-gamma\bin\javaw.exe
buffer << LR"(runtime\)" << name << LR"(\)" << javaName << LR"(\)" << name << LR"(\bin\javaw.exe)";
paths.push_back(buffer.str());

logger.log(L"Adding possible java path: " + buffer.str());
}
}

return paths;
}
8 changes: 6 additions & 2 deletions src/Bootstrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
#include <memory>

#include "ISystemHelper.h"
#include "Logger.h"

class Bootstrap {
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
public:
explicit Bootstrap(const std::shared_ptr<ISystemHelper>& systemHelper);
explicit Bootstrap(ISystemHelper& systemHelper, Logger& logger);

Choose a reason for hiding this comment

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

Suggested change
explicit Bootstrap(ISystemHelper& systemHelper, Logger& logger);
Bootstrap(const ISystemHelper& systemHelper, Logger& logger);


public:
void launch();
Expand All @@ -19,6 +20,9 @@ class Bootstrap {

void showErrorMessage();

const std::vector<std::wstring> getMinecraftJavaPaths(const HostArchitecture::Value& hostArch);

private:
std::shared_ptr<ISystemHelper> systemHelper;
ISystemHelper& systemHelper;

Choose a reason for hiding this comment

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

Suggested change
ISystemHelper& systemHelper;
const ISystemHelper& systemHelper;

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for taking a look at this :) I have fixed the other points.

I was told a while back that you generally shouldn't have const refrences as member vars, ill be honest I didn't fully understand the reasoning. There seems to be a bit of litrature about this online such as: https://lesleylai.info/en/const-and-reference-member-variables/ And then other places seem to suggest having const is fine.

The orignal design around this is to allow a mock of ISystemHelper for unit tests (that dont exist atm), the formal name for this pattern seems to be "dependency injection via constructor".

Going back to using a shared_ptr seems like it might solve most of the possible problems? Either way I'm not too worried about being 100% correct as long as it works, but it is an intresting point of conversation.

Choose a reason for hiding this comment

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

It's generally bad practice to have (even non-const) references as member variables, as that article explains, because references cannot be rebound, i.e. are effectively const. There is no need to change back to shared_ptr, if you want you can use std::reference_wrapper instead, but I left this out of the review because I think the codebase is too simple for this to become a problem.

Logger& logger;
};
31 changes: 31 additions & 0 deletions src/ISystemHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,34 @@
#include <string>
#include <vector>

namespace HostArchitecture {
// Ordered to ensure that we only check for JVMs that will run on the host arch.
// On an x64 host only check for x64 and x86 as arm will never run.
// On x86 there is no need to try any other arch as it wont run.
enum Value {
UNKNOWN,
ARM64,
X64,
X86,
};

constexpr Value VALUES[] = { UNKNOWN, ARM64, X64, X86 };

inline std::wstring AsString(const Value& arch) {
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
switch (arch) {
case X64:
return L"x64";
case ARM64:
return L"arm64";
case X86:
return L"x86";
case UNKNOWN:
default:
return L"unknown";
}
}
}

class ISystemHelper {
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
public:
virtual std::optional<std::wstring> getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) = 0;
Expand All @@ -15,4 +43,7 @@ class ISystemHelper {
virtual bool fileExists(const std::wstring& path) = 0;
virtual bool dirExists(const std::wstring& path) = 0;
virtual std::wstring getBootstrapFilename() = 0;
virtual std::wstring getTempDir() = 0;
virtual HostArchitecture::Value getHostArchitecture() = 0;
virtual long long getEpochTime() = 0;
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
};
19 changes: 19 additions & 0 deletions src/Logger.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include "Logger.h"
#include <sstream>

namespace {
std::wofstream openLogFile(ISystemHelper& systemHelper) {
std::wstringstream fileNameBuf;
fileNameBuf << systemHelper.getTempDir() << L"/fabric-installer-" << systemHelper.getEpochTime() << L".log";
return std::wofstream(fileNameBuf.str());
}
}

Logger::Logger(ISystemHelper& systemHelper) : file_(openLogFile(systemHelper))
{
}

Logger::~Logger()
{
file_.close();
}
19 changes: 19 additions & 0 deletions src/Logger.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#pragma once

#include "ISystemHelper.h"
#include <fstream>

class Logger
{
public:
Logger(ISystemHelper& systemHelper);
~Logger();

template<class T>
void log(const T& line) {
file_ << line << std::endl;
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
}

private:
std::wofstream file_;
};
Loading
Loading