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

Add glib #529

Merged
merged 8 commits into from
Jan 19, 2017
Merged

Add glib #529

merged 8 commits into from
Jan 19, 2017

Conversation

codicodi
Copy link
Contributor

Fixes #176.
Aside from glib port this PR adds a new function: vcpkg_copy_tool_dependencies, which provides dlls for installed tools. It's important that glib's tools are ready to use since other GNOME libraries often need them in the build process.

@ras0219-msft
Copy link
Contributor

Wow, this is quite an extensive port. Nice work!

  • I'm a bit curious about the force-use-libiconv. Does this enable additional functionality on Windows that wasn't present? Or does this just switch glib to use a few less Win32 calls and use libiconv instead? Since this isn't what upstream is doing, it seems like it would be better to simply have one fewer dependency.
  • To avoid collisions with other packages doing the same thing, any tools that require DLLs should be put in a subdirectory of tools\ corresponding to the package (in this case, tools\glib\ probably). Otherwise, another package that needs to deploy tools\pcre.dll won't be simultaneously installable -- we insist on each file having a single owner.

@codicodi
Copy link
Contributor Author

codicodi commented Jan 14, 2017

I initially thought static builds may work, so embedded iconv symbol would cause problems. I guess this patch can be safely dropped now.

As for tools you're totally right, I didn't think about this. Moving the tools to subdir is probably the best solution although it means find_program calls won't just work :( and will have to be fed with appropriate PATH_SUFFIXES or so

@codicodi
Copy link
Contributor Author

Updated

@ras0219-msft
Copy link
Contributor

I didn't look very far into the libiconv issue; if the alternative is using a checked-in copy of libiconv, then your patch is very needed! Even in the non-static case, if the user wants to debug into glib.dll, nightmares ensue when the same symbol leads to different code in different contexts.

find_program() is definitely an issue. I think it would be reasonable to file(GLOB) all the directories inside tools\ and add them to  CMAKE_SYSTEM_PROGRAM_PATH in our toolchain file, which should fix the problem.

@codicodi
Copy link
Contributor Author

Yeah, glib uses iconv one way or another (on other systems just assumes it's available, on Windows unconditionally falls back to a signle-file implementation backed with WinAPI).

It is required to build gtk+
@ras0219-msft
Copy link
Contributor

Great, thanks for the port!

@ras0219-msft ras0219-msft merged commit 2b48e78 into microsoft:master Jan 19, 2017
@codicodi codicodi deleted the add-glib branch January 19, 2017 23:44
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.

3 participants