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

Build static version #59

Closed
wants to merge 1 commit into from
Closed

Build static version #59

wants to merge 1 commit into from

Conversation

drizt
Copy link
Contributor

@drizt drizt commented May 18, 2015

Added new macro WIN_SPARKLE_STATIC to prevent
preprocessor magic which allow to export of library symbols.
This macro must be used to build static version of WinSparkle.

Added new macro WIN_SPARKLE_STATIC to prevent
preprocessor magic which allow to export of library symbols.
This macro must be used to build static version of WinSparkle.
@vslavik
Copy link
Owner

vslavik commented May 18, 2015

Nitpick: that commit message, "Build static version” is very misleading — that’s not what it does, no build changes are involved.

More importantly, I don’t think it’s appropriate to build WinSparkle as a static library, nor that it is safe to do so — the DLL carefully exposes only public symbols, having all the symbols from the dependencies in the hosting app’s linker “namespace” is unsafe. And some things that WinSparkle should do all but require a DLL (e.g. #21, needs to copy itself to a temp location to be able to safely unpack install).

@vslavik vslavik closed this May 18, 2015
@drizt
Copy link
Contributor Author

drizt commented May 18, 2015

I agree. Bad commit message.

About "unsafe". I want to use WinSparkle for my Torrent File Editor app. It's only one .exe file on Windows. Without any libraries. So I want to also build WinSparkle as static and link against my app. Update workflow will be such:

  1. Check new version on server
  2. Ask user to update
  3. Download new .exe file in the same folder as original .exe
  4. Start new .exe with special argument
  5. New .exe which started with special argument removes old .exe

@vslavik
Copy link
Owner

vslavik commented May 19, 2015

About “unsafe”

I meant the inclusion of all sorts of private symbols (mostly from wxWidgets) becoming visible — and possibly conflicting — to your linker.

Update workflow will be such:

That’s precisely my point. To accomplish such updating, you have to move the executable out of the way. This should be handled by WinSparkle transparently in the future and to be able to do that, WinSparkle will need the ability to move itself out of the way (because app update may update it too) — and that can only be done if WinSparkle is a DLL.

What you want to do (and what I agree is necessary with statically linked WinSparkle) requires additional cooperation from the application, and rather complicated one. I want using of WinSparkle to be as simple as possible, and the hops — and inevitable API changes — that the static library variant would require go against that.

Because I don’t want to support a static version, I don’t want to give the impression that it is supported, such as by having code that contains WIN_SPARKLE_STATIC in it.

I’m willing to have this block wrapped inside #ifndef WIN_SPARKLE_API instead. That should allow you to make your private static build without having to patch the sources, by simply passing -DWIN_SPARKLE_API to the compiler, shouldn’t it?

@vslavik vslavik reopened this May 19, 2015
@drizt
Copy link
Contributor Author

drizt commented May 19, 2015

Current API fully satisfies my needs. No required extra changes.
If I can just set WIN_SPARKLE_API in my CMake scripts it will be OK.

@vslavik
Copy link
Owner

vslavik commented May 24, 2015

Current API fully satisfies my needs. No required extra changes.

Sorry, did you pay any attention to what I wrote? Your needs are rather unique. As I explained (and linked to a relevant issue) there are features planned (and some partially implemented, soon to be merged) that require a DLL and would require new API to handle the static case.

Likewise with the hops you are going to do with your executable: that should be WinSparkle's job and any officially supported static build would need to make it as painless as possible — again, necessitating new API.

@vslavik vslavik closed this May 24, 2015
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