Skip to content

Conversation

@meganukebmp
Copy link

@meganukebmp meganukebmp commented Nov 17, 2025

This change cleans up the Makefile a bit.

From @unicab369 on discord we know that minichlink compiles fine on Clang, so it's safe to not hardcode the compiler and instead pull it from $(CC)

Remove the platform (windows) CFLAGS and LDFLAGS and just override them in the conditional like it's done for macos.

Remove the .exe build instructions. Make on Windows will actually handle this for us. If we're building an executable it will append it with .exe. Likewise for executions, deletions and so on.

Add minichlink.dll to the default build to mirror the steps for *nix. Prior it had to be invoked explicitly.

Remove the makefile itself from rule dependencies. Doesn't hurt but makes no sense. How do we invoke a build step from the makefile without the makefile existing.

all : $(TOOLS)

# will need mingw-w64-x86-64-dev gcc-mingw-w64-x86-64
minichlink.exe : $(C_S) $(H_S) Makefile
Copy link
Owner

Choose a reason for hiding this comment

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

How does the .exe get generated in the new version?

Copy link
Author

Choose a reason for hiding this comment

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

Make on windows will actually handle this for us. Not specifying an extension leads to generating an exe.

Copy link
Author

Choose a reason for hiding this comment

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

Likewise commands like rm minichlink will delete minichlink.exe. I can only assume this is intentionally done by mingw devs

LDFLAGS_WINDOWS:=-L. -lpthread -lusb-1.0 -lsetupapi -lws2_32
ifeq ($(OS),Windows_NT)
TOOLS:=minichlink.exe
TOOLS:=minichlink minichlink.dll
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't this need to be minichlink.exe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants