Skip to content

Master issue for static library nonsense: Windows resource file copying and Unix build oddities #308

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

Open
andlabs opened this issue Mar 23, 2018 · 8 comments
Milestone

Comments

@andlabs
Copy link
Owner

andlabs commented Mar 23, 2018

libui needs to be available as both a static and as a dynamic library, as different users have different requirements. Static libraries have two major issues:

  • they share the same symbol namespace with whatever they are linked to
  • on Windows, static libraries can't have resources

Right now, libui tries to hack around all this by a) obfuscating non-public symbols using a trick deduced by @pcwalton b) manually copying the compiled resources.o file on Windows into the build\out folder so it can be linked into binaries.

These options have caused nothing but build issues and maintenance nightmares for everyone, including myself, so I'm going to be fixing this properly:

  • prefix all non-public names that aren't static with uipriv
  • embed compiled resources on Windows directly into the binary somehow

I need to figure out whether all structs and classes (including ones only used in one source file) need to have this too. All GObject and Objective-C classes also need this change, because they share an application-global namespace regardless.

I need to figure out how to do the Windows resource precompiling thing in a way suitable for each toolchain.

@mischnic
Copy link
Contributor

mischnic commented Mar 26, 2018

The macOS linker warning (built for newer OSX version (10.11) than being linked (10.8)) should already be fixed with this:

set(CMAKE_OSX_DEPLOYMENT_TARGET "10.8" CACHE STRING "" FORCE)

You can test this with:

$ otool -l out/libui.a | grep -A3 LC_VERSION_MIN_MACOSX
      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.8
      sdk n/a

$ otool -l out/libui.dylib | grep -A3 LC_VERSION_MIN_MACOSX
      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.8
      sdk 10.13

@andlabs
Copy link
Owner Author

andlabs commented Mar 26, 2018

Oh good, so it's already fixed then because package ui on OS X currently uses a cmake build with that patch applied. Either way, those issues can be closed. (And that problem is far from the only one here.)

@DemiMarie
Copy link

I think we should localize symbols in static libraries where we can.

@andlabs
Copy link
Owner Author

andlabs commented Mar 28, 2018

What do you mean?

@DemiMarie
Copy link

@andlabs I think we should keep using objcopy --localize-hidden whenever it is available on the build machine. CMake can easily check for this.

@andlabs
Copy link
Owner Author

andlabs commented Mar 29, 2018

Why?

For the record, the current static library hacks (all of them combined) doesn't properly inherit object options like -fPIC, relies on things having specific pathnames that have proven wrong many times, and the resultant .a file doesn't get rebuilt when the source files change, which is annoying for testing.

andlabs added a commit that referenced this issue Apr 18, 2018
Update #308. Oops, forgot to do this with the merge...
andlabs added a commit that referenced this issue May 3, 2018
@andlabs
Copy link
Owner Author

andlabs commented May 3, 2018

Windows resource copying is gone now. cmake builds should Just Work™.

andlabs added a commit that referenced this issue May 6, 2018
Doesn't handle Objective-C data yet; that'll come later.

Also starts a naming document.

Update #308
@andlabs
Copy link
Owner Author

andlabs commented May 12, 2018

Oops, forgot to mark unix-namespace-cleanup as part of this issue.

Most of the immediate work is now finished. I still need to finalize naming policy and apply these fixes to Windows, but we should be better off than before in the meantime. I'll probably make this stop blocking Alpha 4's release 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

No branches or pull requests

3 participants