-
Notifications
You must be signed in to change notification settings - Fork 91
DO NOT MERGE: makefile to make windows great again #19605
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
Draft
anastasiyaig
wants to merge
1
commit into
master
Choose a base branch
from
windows-makefile
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+427
−3
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,9 @@ GIT_ROOT ?= $(shell git rev-parse --show-toplevel 2>/dev/null || echo .) | |
| run-statusq-sanity-checker \ | ||
| statusq-tests \ | ||
| run-statusq-tests \ | ||
| statusq-import-lib \ | ||
| nim-sds \ | ||
| copy-windows-dlls \ | ||
| storybook-build \ | ||
| run-storybook \ | ||
| run-storybook-tests \ | ||
|
|
@@ -249,6 +252,56 @@ NIMSDS_LIBFILE := $(NIMSDS_LIBDIR)/libsds.$(LIB_EXT) | |
| NIM_EXTRA_PARAMS += --passL:"-L$(NIMSDS_LIBDIR)" --passL:"-lsds" | ||
| STATUSGO_MAKE_PARAMS += NIM_SDS_SOURCE_DIR="$(NIM_SDS_SOURCE_DIR)" | ||
|
|
||
| # Common nim-sds build recipe (used by both Windows and non-Windows) | ||
| define BUILD_NIMSDS | ||
| @echo -e "\033[92mBuilding:\033[39m nim-sds" | ||
| @if [ ! -d "$(NIM_SDS_SOURCE_DIR)" ]; then \ | ||
| echo "Error: nim-sds directory not found at $(NIM_SDS_SOURCE_DIR)"; \ | ||
| echo "Please clone it or set NIM_SDS_SOURCE_DIR environment variable"; \ | ||
| exit 1; \ | ||
| fi | ||
| @$(MAKE) -C $(NIM_SDS_SOURCE_DIR) libsds USE_SYSTEM_NIM=1 SHELL=/bin/bash $(HANDLE_OUTPUT) | ||
| endef | ||
|
|
||
| ifeq ($(mkspecs),win32) | ||
| # On Windows with MinGW, create import library for nim-sds | ||
| NIMSDS_DLL := $(NIMSDS_LIBDIR)/libsds.dll | ||
| NIMSDS_DEF := $(NIMSDS_LIBDIR)/libsds.def | ||
| NIMSDS_IMPORT_LIB := $(NIMSDS_LIBDIR)/libsds.dll.a | ||
|
|
||
| $(NIMSDS_DEF): $(NIMSDS_DLL) | ||
| @echo -e "\033[92mCreating:\033[39m libsds.def" | ||
| @mkdir -p $(NIMSDS_LIBDIR) | ||
| @(echo "EXPORTS"; \ | ||
| echo "SdsCleanupReliabilityManager"; \ | ||
| echo "SdsMarkDependenciesMet"; \ | ||
| echo "SdsNewReliabilityManager"; \ | ||
| echo "SdsResetReliabilityManager"; \ | ||
| echo "SdsSetEventCallback"; \ | ||
| echo "SdsStartPeriodicTasks"; \ | ||
| echo "SdsUnwrapReceivedMessage"; \ | ||
| echo "SdsWrapOutgoingMessage"; \ | ||
| echo "libsdsNimDestroyGlobals"; \ | ||
| echo "libsdsNimMain") > $(NIMSDS_DEF) | ||
|
|
||
| $(NIMSDS_IMPORT_LIB): $(NIMSDS_DLL) $(NIMSDS_DEF) | ||
| @echo -e "\033[92mCreating:\033[39m libsds.dll.a" | ||
| @rm -f $(NIMSDS_IMPORT_LIB) | ||
| @cd $(NIMSDS_LIBDIR) && dlltool --dllname libsds.dll --def libsds.def --output-lib libsds.dll.a || \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, I think we can do that in |
||
| (echo "Warning: Failed to create import library. Ensure dlltool is in PATH." && exit 0) | ||
|
|
||
| $(NIMSDS_LIBFILE): | deps | ||
| $(BUILD_NIMSDS) | ||
| @$(MAKE) $(NIMSDS_IMPORT_LIB) || true | ||
|
|
||
| nim-sds: $(NIMSDS_LIBFILE) $(NIMSDS_IMPORT_LIB) | ||
| else | ||
| $(NIMSDS_LIBFILE): | deps | ||
| $(BUILD_NIMSDS) | ||
|
|
||
| nim-sds: $(NIMSDS_LIBFILE) | ||
| endif | ||
|
|
||
| INCLUDE_DEBUG_SYMBOLS ?= false | ||
| ifeq ($(INCLUDE_DEBUG_SYMBOLS),true) | ||
| # We need `-d:debug` to get Nim's default stack traces | ||
|
|
@@ -326,10 +379,35 @@ statusq-build: | statusq-configure | |
|
|
||
| statusq-install: | statusq-build | ||
| echo -e "\033[92mInstalling:\033[39m StatusQ" | ||
| @mkdir -p $(STATUSQ_INSTALL_PATH)/StatusQ || true | ||
| cmake --install $(STATUSQ_BUILD_PATH) \ | ||
| $(HANDLE_OUTPUT) | ||
|
|
||
| ifeq ($(mkspecs),win32) | ||
| # On Windows with MinGW, create import libraries (.dll.a) from DLLs | ||
| STATUSQ_DLL := $(STATUSQ_INSTALL_PATH)/StatusQ/StatusQ.dll | ||
| STATUSQ_DEF := $(STATUSQ_INSTALL_PATH)/StatusQ/StatusQ.def | ||
| STATUSQ_IMPORT_LIB := $(STATUSQ_INSTALL_PATH)/StatusQ/libStatusQ.dll.a | ||
|
|
||
| $(STATUSQ_DEF): $(STATUSQ_DLL) | ||
| @echo -e "\033[92mCreating:\033[39m StatusQ.def" | ||
| @mkdir -p $(STATUSQ_INSTALL_PATH)/StatusQ || true | ||
| @(echo "EXPORTS"; \ | ||
| echo "statusq_getMobileUIScaleFactor"; \ | ||
| echo "statusq_registerQmlTypes") > $(STATUSQ_DEF) | ||
|
|
||
| $(STATUSQ_IMPORT_LIB): $(STATUSQ_DLL) $(STATUSQ_DEF) | ||
| @echo -e "\033[92mCreating:\033[39m libStatusQ.dll.a" | ||
| @mkdir -p $(STATUSQ_INSTALL_PATH)/StatusQ | ||
| @cd $(STATUSQ_INSTALL_PATH)/StatusQ && dlltool --dllname StatusQ.dll --def StatusQ.def --output-lib libStatusQ.dll.a || \ | ||
| (echo "Warning: Failed to create import library. Ensure dlltool is in PATH." && exit 0) | ||
|
|
||
| statusq-import-lib: $(STATUSQ_IMPORT_LIB) | ||
|
|
||
| statusq: | statusq-install statusq-import-lib | ||
| else | ||
| statusq: | statusq-install | ||
| endif | ||
|
|
||
| statusq-clean: | ||
| echo -e "\033[92mCleaning:\033[39m StatusQ" | ||
|
|
@@ -474,13 +552,25 @@ STATUSGO := vendor/status-go/build/bin/libstatus.$(LIB_EXT) | |
| STATUSGO_LIBDIR := $(shell pwd)/$(shell dirname "$(STATUSGO)") | ||
| export STATUSGO_LIBDIR | ||
|
|
||
| ifeq ($(mkspecs),win32) | ||
| $(STATUSGO): | deps status-go-deps | ||
| echo -e $(BUILD_MSG) "status-go" | ||
| # FIXME: Nix shell usage breaks builds due to Glibc mismatch. | ||
| $(STATUSGO_MAKE_PARAMS) $(MAKE) -C vendor/status-go statusgo-shared-library SHELL=/bin/sh \ | ||
| SENTRY_CONTEXT_NAME="status-desktop" \ | ||
| SENTRY_CONTEXT_VERSION="$(DESKTOP_VERSION)" \ | ||
| $(HANDLE_OUTPUT) | ||
| # On Windows, ensure import library exists after status-go build | ||
| @if [ -f "$(NIMSDS_LIBFILE)" ]; then $(MAKE) $(NIMSDS_IMPORT_LIB) || true; fi | ||
| else | ||
| $(STATUSGO): | deps status-go-deps | ||
| echo -e $(BUILD_MSG) "status-go" | ||
| # FIXME: Nix shell usage breaks builds due to Glibc mismatch. | ||
| $(STATUSGO_MAKE_PARAMS) $(MAKE) -C vendor/status-go statusgo-shared-library SHELL=/bin/sh \ | ||
| SENTRY_CONTEXT_NAME="status-desktop" \ | ||
| SENTRY_CONTEXT_VERSION="$(DESKTOP_VERSION)" \ | ||
| $(HANDLE_OUTPUT) | ||
| endif | ||
|
|
||
| status-go: $(STATUSGO) | ||
|
|
||
|
|
@@ -611,7 +701,33 @@ $(NIM_STATUS_CLIENT): update-qmake-previous | |
| endif | ||
|
|
||
| $(NIM_STATUS_CLIENT): NIM_PARAMS += $(RESOURCES_LAYOUT) | ||
| $(NIM_STATUS_CLIENT): $(NIM_SOURCES) | statusq dotherside check-qt-dir $(STATUSGO) $(STATUSKEYCARDGO) $(QRCODEGEN) rcc deps | ||
| ifeq ($(mkspecs),win32) | ||
| # Copy DLLs to bin directory for Windows runtime | ||
| copy-windows-dlls: | statusq dotherside $(STATUSGO) $(STATUSKEYCARDGO) $(NIMSDS_LIBFILE) | ||
| @echo -e "\033[92mCopying:\033[39m Windows DLLs to bin directory" | ||
| @mkdir -p $(STATUSQ_INSTALL_PATH) | ||
| @mkdir -p $(STATUSQ_INSTALL_PATH)/StatusQ | ||
| @if [ -f "$(STATUSQ_INSTALL_PATH)/StatusQ/StatusQ.dll" ]; then \ | ||
| echo "StatusQ.dll already in $(STATUSQ_INSTALL_PATH)/StatusQ/"; \ | ||
| elif [ -f "$(STATUSQ_BUILD_PATH)/bin/$(COMMON_CMAKE_BUILD_TYPE)/StatusQ.dll" ]; then \ | ||
| cp "$(STATUSQ_BUILD_PATH)/bin/$(COMMON_CMAKE_BUILD_TYPE)/StatusQ.dll" "$(STATUSQ_INSTALL_PATH)/StatusQ/" 2>/dev/null || true; \ | ||
| fi | ||
| @rm -f "$(STATUSQ_INSTALL_PATH)/StatusQ.dll" 2>/dev/null || true | ||
| @if [ -f "$(DOTHERSIDE_LIBFILE)" ]; then \ | ||
| cp "$(DOTHERSIDE_LIBFILE)" "$(STATUSQ_INSTALL_PATH)/" 2>/dev/null || true; \ | ||
| fi | ||
| @if [ -f "$(STATUSGO)" ]; then \ | ||
| cp "$(STATUSGO)" "$(STATUSQ_INSTALL_PATH)/" 2>/dev/null || true; \ | ||
| fi | ||
| @if [ -f "$(STATUSKEYCARDGO)" ]; then \ | ||
| cp "$(STATUSKEYCARDGO)" "$(STATUSQ_INSTALL_PATH)/" 2>/dev/null || true; \ | ||
| fi | ||
| @if [ -f "$(NIMSDS_LIBFILE)" ]; then \ | ||
| cp "$(NIMSDS_LIBFILE)" "$(STATUSQ_INSTALL_PATH)/" 2>/dev/null || true; \ | ||
| fi | ||
| endif | ||
|
|
||
| $(NIM_STATUS_CLIENT): $(NIM_SOURCES) | statusq dotherside check-qt-dir $(STATUSGO) $(STATUSKEYCARDGO) $(QRCODEGEN) rcc deps $(if $(filter win32,$(mkspecs)),copy-windows-dlls) | ||
| echo -e $(BUILD_MSG) "$@" | ||
| $(ENV_SCRIPT) nim c $(NIM_PARAMS) \ | ||
| --mm:refc \ | ||
|
|
@@ -887,9 +1003,9 @@ run-macos: nim_status_client | |
| ./bin/StatusDev.app/Contents/MacOS/nim_status_client $(ARGS) | ||
|
|
||
| run-windows: STATUS_RC_FILE = status-dev.rc | ||
| run-windows: compile_windows_resources nim_status_client | ||
| run-windows: compile_windows_resources nim_status_client $(if $(filter win32,$(mkspecs)),copy-windows-dlls) | ||
| echo -e "\033[92mRunning:\033[39m bin/nim_status_client.exe" | ||
| PATH="$(DOTHERSIDE_LIBDIR)":"$(STATUSGO_LIBDIR)":"$(STATUSKEYCARDGO_LIBDIR)":"$(STATUSQ_INSTALL_PATH)/StatusQ":"$(PATH)" \ | ||
| PATH="$(DOTHERSIDE_LIBDIR)":"$(STATUSGO_LIBDIR)":"$(STATUSKEYCARDGO_LIBDIR)":"$(STATUSQ_INSTALL_PATH)/StatusQ":"$(STATUSQ_INSTALL_PATH)":"$(PATH)" \ | ||
| ./bin/nim_status_client.exe $(ARGS) | ||
|
|
||
| NIM_TEST_FILES := $(wildcard test/nim/*.nim) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| # Windows Build Process Fixes | ||
|
|
||
| This document explains the automated fixes added to the Makefile for Windows/MinGW builds. | ||
|
|
||
| ## Overview | ||
|
|
||
| Three main issues were fixed and automated: | ||
| 1. **nim-sds library build** - Ensures the library is built before linking | ||
| 2. **Import library generation** - Creates `.dll.a` files for MinGW from DLLs | ||
| 3. **DLL copying** - Copies all required DLLs to `bin/` directory for runtime | ||
|
|
||
| ## Changes Made | ||
|
|
||
| ### 1. nim-sds Build Automation | ||
|
|
||
| **Location:** Lines ~255-282 in Makefile | ||
|
|
||
| **What it does:** | ||
| - Builds the `nim-sds` library if it doesn't exist | ||
| - On Windows, automatically creates the import library (`libsds.dll.a`) after building the DLL | ||
| - Uses `dlltool` to generate the import library from the DLL | ||
|
|
||
| **Key targets:** | ||
| - `nim-sds` - Builds nim-sds library and import library (Windows only) | ||
| - `$(NIMSDS_LIBFILE)` - Dependency that triggers the build | ||
|
|
||
| **How it works:** | ||
| ```makefile | ||
| $(NIMSDS_LIBFILE): | deps | ||
| # Builds libsds.dll | ||
| $(MAKE) -C $(NIM_SDS_SOURCE_DIR) libsds | ||
|
|
||
| # Creates import library | ||
| dlltool --dllname libsds.dll --def libsds.def --output-lib libsds.dll.a | ||
| ``` | ||
|
|
||
| ### 2. StatusQ Import Library Generation | ||
|
|
||
| **Location:** Lines ~376-397 in Makefile | ||
|
|
||
| **What it does:** | ||
| - After StatusQ is installed, automatically creates `libStatusQ.dll.a` import library | ||
| - Creates a `.def` file with the exported symbols (`statusq_registerQmlTypes`, `statusq_getMobileUIScaleFactor`) | ||
| - Uses `dlltool` to generate the MinGW-compatible import library | ||
|
|
||
| **Key targets:** | ||
| - `statusq-import-lib` - Creates the import library | ||
| - `statusq` - Now depends on `statusq-import-lib` on Windows | ||
|
|
||
| **Files created:** | ||
| - `bin/StatusQ/StatusQ.def` - Module definition file | ||
| - `bin/StatusQ/libStatusQ.dll.a` - Import library for MinGW | ||
|
|
||
| **How it works:** | ||
| ```makefile | ||
| $(STATUSQ_DEF): $(STATUSQ_DLL) | ||
| # Creates .def file with exports | ||
| echo "EXPORTS" > StatusQ.def | ||
| echo "statusq_registerQmlTypes" >> StatusQ.def | ||
| echo "statusq_getMobileUIScaleFactor" >> StatusQ.def | ||
|
|
||
| $(STATUSQ_IMPORT_LIB): $(STATUSQ_DLL) $(STATUSQ_DEF) | ||
| # Creates import library | ||
| dlltool --dllname StatusQ.dll --def StatusQ.def --output-lib libStatusQ.dll.a | ||
| ``` | ||
|
|
||
| ### 3. DLL Copying for Runtime | ||
|
|
||
| **Location:** Lines ~680-690 in Makefile | ||
|
|
||
| **What it does:** | ||
| - Copies all required DLLs to the `bin/` directory before building | ||
| - Ensures the executable can find DLLs at runtime (Windows searches the executable's directory) | ||
|
|
||
| **Key targets:** | ||
| - `copy-windows-dlls` - Copies all DLLs to bin/ | ||
|
|
||
| **DLLs copied:** | ||
| - `StatusQ.dll` | ||
| - `DOtherSide.dll` | ||
| - `libstatus.dll` | ||
| - `libkeycard.dll` | ||
| - `libsds.dll` | ||
|
|
||
| **How it works:** | ||
| ```makefile | ||
| copy-windows-dlls: | statusq dotherside $(STATUSGO) $(STATUSKEYCARDGO) $(NIMSDS_LIBFILE) | ||
| cp $(STATUSQ_INSTALL_PATH)/StatusQ/StatusQ.dll $(STATUSQ_INSTALL_PATH)/ | ||
| cp $(DOTHERSIDE_LIBFILE) $(STATUSQ_INSTALL_PATH)/ | ||
| # ... copies other DLLs | ||
| ``` | ||
|
|
||
| ### 4. Updated Dependencies | ||
|
|
||
| **Location:** Line ~692 in Makefile | ||
|
|
||
| **What changed:** | ||
| - `nim_status_client` now depends on: | ||
| - `nim-sds` (Windows only) - Ensures nim-sds is built | ||
| - `copy-windows-dlls` (Windows only) - Ensures DLLs are in place | ||
|
|
||
| **Before:** | ||
| ```makefile | ||
| $(NIM_STATUS_CLIENT): ... | statusq dotherside ... | ||
| ``` | ||
|
|
||
| **After:** | ||
| ```makefile | ||
| $(NIM_STATUS_CLIENT): ... | statusq dotherside ... $(if $(filter win32,$(mkspecs)),nim-sds copy-windows-dlls) | ||
| ``` | ||
|
|
||
| ## Requirements | ||
|
|
||
| ### Tools Required | ||
|
|
||
| 1. **dlltool** - Part of MinGW toolchain | ||
| - Location: Usually in `C:\ProgramData\scoop\apps\gcc\current\bin\dlltool.exe` | ||
| - Used to create import libraries from DLLs | ||
|
|
||
| 2. **nim-sds source** - Must be cloned | ||
| - Default location: `../nim-sds` (relative to status-app) | ||
| - Can be overridden with `NIM_SDS_SOURCE_DIR` environment variable | ||
|
|
||
| ## Usage | ||
|
|
||
| ### Normal Build | ||
|
|
||
| Just run the normal build command: | ||
| ```bash | ||
| make nim_status_client | ||
| ``` | ||
|
|
||
| The build process will automatically: | ||
| 1. Build nim-sds if needed | ||
| 2. Build StatusQ and create import library | ||
| 3. Copy DLLs to bin directory | ||
| 4. Build the executable | ||
|
|
||
| ### Manual Steps (if needed) | ||
|
|
||
| If you need to rebuild just the import libraries: | ||
|
|
||
| ```bash | ||
| # Rebuild StatusQ import library | ||
| make statusq-import-lib | ||
|
|
||
| # Rebuild nim-sds import library | ||
| make nim-sds | ||
|
|
||
| # Copy DLLs manually | ||
| make copy-windows-dlls | ||
| ``` | ||
|
|
||
| ## Troubleshooting | ||
|
|
||
| ### Error: "dlltool not found" | ||
| - Ensure MinGW is installed and in PATH | ||
| - Or set the full path to dlltool in the Makefile | ||
|
|
||
| ### Error: "nim-sds directory not found" | ||
| - Clone nim-sds: `git clone https://github.com/waku-org/nim-sds.git ../nim-sds` | ||
| - Or set `NIM_SDS_SOURCE_DIR` environment variable | ||
|
|
||
| ### Import library creation fails silently | ||
| - Check that `dlltool` is available: `which dlltool` (Git Bash) or `where dlltool` (PowerShell) | ||
| - Verify the DLL exists before creating import library | ||
| - Check the `.def` file was created correctly | ||
|
|
||
| ## Technical Details | ||
|
|
||
| ### Why Import Libraries Are Needed | ||
|
|
||
| On Windows with MinGW: | ||
| - MSVC creates `.lib` files (not compatible with MinGW linker) | ||
| - MinGW linker needs `.dll.a` import libraries | ||
| - These contain stub functions that redirect to the DLL at runtime | ||
|
|
||
| ### Symbol Export | ||
|
|
||
| The `statusq_registerQmlTypes` function is exported from StatusQ.dll using: | ||
| - `Q_DECL_EXPORT` macro in C++ | ||
| - `extern "C"` linkage for C compatibility | ||
| - Listed in the `.def` file for import library generation | ||
|
|
||
| ## Future Improvements | ||
|
|
||
| 1. **Auto-detect exports** - Use `objdump` or `pexports` to automatically extract symbols | ||
| 2. **Better error handling** - Fail build if import library creation fails | ||
| 3. **Cleanup targets** - Add targets to remove generated import libraries | ||
| 4. **Cross-platform** - Ensure these changes don't affect Linux/macOS builds | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can create the libsds.def file in
nim-sdsrepo.Why is it needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows with MinGW, when you link against a DLL using -lsds, the linker needs an import library (.dll.a file) that contains the list of exported symbols.
libsds.def is a module definition file that tells dlltool which symbols to include in the import library. Without it, dlltool may fail to extract all exports from the DLL automatically.
The .def file simply lists:
Then dlltool uses it to create libsds.dll.a:
dlltool --dllname libsds.dll --def libsds.def --output-lib libsds.dll.a
Some build failures (undefined reference to 'SdsSetEventCallback') happened because dlltool --output-def didn't reliably extract all symbols from the DLL. Manually specifying them in the .def file ensures they're all included.