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

fix: enable static link for windows-msvc #308

Merged
merged 4 commits into from
Mar 7, 2024
Merged

fix: enable static link for windows-msvc #308

merged 4 commits into from
Mar 7, 2024

Conversation

Zagrios
Copy link
Contributor

@Zagrios Zagrios commented Mar 6, 2024

The generated .node file for Windows require certain DLLs which may not be present, especially in applications used by end-users which can lead to a `Missing DLL error.

It was my case with some users with my application so I've switch to sharp for now.

With Dependencies we can see the amount of needed DLLs is greatly reduce with static link :

before after
image image

The size of the generated .node file is a slightly larger tho

before after
4.12mo 4.29mo

Copy link

vercel bot commented Mar 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
resvg-js ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2024 4:52pm

@yisibl
Copy link
Member

yisibl commented Mar 6, 2024

Thanks!

There is a test case that needs to be skipped, so please rebase the main branch.

@yisibl yisibl self-requested a review March 6, 2024 13:23
@Zagrios
Copy link
Contributor Author

Zagrios commented Mar 6, 2024

@yisibl done 😉

@yisibl
Copy link
Member

yisibl commented Mar 6, 2024

I would like to confirm that the users who are prompted for missing dlls are doing so because they don't have "Visual C++ Redistributable" installed?

@Zagrios
Copy link
Contributor Author

Zagrios commented Mar 6, 2024

Yeah exactly

@yisibl
Copy link
Member

yisibl commented Mar 6, 2024

It looks like x86 needs to be added as well:

[target.i686-pc-windows-msvc]
rustflags = ["-C", "target-feature=+crt-static"]

@yisibl
Copy link
Member

yisibl commented Mar 6, 2024

Currently x86_64-unknown-linux-musl is reporting errors in the CI, and I've been troubleshooting it for two days with no luck.

@Zagrios
Copy link
Contributor Author

Zagrios commented Mar 6, 2024

Currently x86_64-unknown-linux-musl is reporting errors in the CI, and I've been troubleshooting it for two days with no luck.

Yeah I saw your PR 😅

@yisibl
Copy link
Member

yisibl commented Mar 6, 2024

By the way, what app are you using resvg-js in?

@Zagrios
Copy link
Contributor Author

Zagrios commented Mar 6, 2024

I was using it in bs-manager to make dynamic shortcut icon from an svg, but as I said for now I've switch to sharp due to the missing DLL issue.
But I will re-use it as soon as this fix is released

@yisibl yisibl merged commit 83517ce into thx:main Mar 7, 2024
33 of 36 checks passed
@yisibl
Copy link
Member

yisibl commented Mar 11, 2024

@Zagrios I've released v2.6.1-beta.0, please try it.

@Zagrios
Copy link
Contributor Author

Zagrios commented Mar 12, 2024

@yisibl yup, it's working perfectly fine now 👌

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