After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752952 - Enable the Sources to Build On Visual Studio
Enable the Sources to Build On Visual Studio
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
2.51.x
Other Windows
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2015-07-28 07:43 UTC by Fan, Chun-wei
Modified: 2015-08-06 16:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
soup-xmlrpc.c: Use g_snprintf() (1.32 KB, patch)
2015-07-28 07:46 UTC, Fan, Chun-wei
committed Details | Review
soup-websocket.c: Avoid VLAs (1.50 KB, patch)
2015-07-28 07:48 UTC, Fan, Chun-wei
committed Details | Review
soup_init(): Don't Hardcode Paths on Windows (1.78 KB, patch)
2015-07-28 07:50 UTC, Fan, Chun-wei
none Details | Review
soup_init(): Don't Hardcode Paths on Windows, take 2 (1.83 KB, patch)
2015-08-05 16:41 UTC, Fan, Chun-wei
none Details | Review
soup_init(): Don't Hardcode Paths on Windows, take 2.1 (1.83 KB, patch)
2015-08-06 11:01 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2015-07-28 07:43:56 UTC
Hi,

As requested by Nacho, I was trying to see whether libsoup was buildable on Visual Studio, and it seems that surprisingly, it was not overly complicated.

I will attach patches to the code to make this work, along with an enhancement for all Windows builds to make the libsoup DLL more relocatable.

p.s. However, as there isn't a build of glib-networking for Visual Studio, it will be so that some features of libsoup will not be working, notably TLS (harder due to the use of GNUTLS) and proxy (should be easier though), which is provided by glib-networking.  As a result, this would not contain MSVC project files for building libsoup on Visual Studio at the moment.

With blessings, thank you!
Comment 1 Fan, Chun-wei 2015-07-28 07:46:08 UTC
Created attachment 308286 [details] [review]
soup-xmlrpc.c: Use g_snprintf()

Hi,

First up is the changes to the soup-xmlrpc.c to make it use g_snprintf() instead of snprintf(), as snprintf() is not universally available...
Comment 2 Fan, Chun-wei 2015-07-28 07:48:28 UTC
Created attachment 308287 [details] [review]
soup-websocket.c: Avoid VLAs

Hi,

Visual Studio does not like any array with 0-lengths or VLAs of any sort, even if the length is a const int (or so), so we need to #define a macro/value for it, as that is the value used for that function...
Comment 3 Fan, Chun-wei 2015-07-28 07:50:04 UTC
Created attachment 308288 [details] [review]
soup_init(): Don't Hardcode Paths on Windows

Hi,

This is an enhancement for all Windows builds so that the localedir can be acquired at run-time, instead of using the LOCALEDIR which is hard-coded at build-time.

With blessings, thank you!
Comment 4 Ignacio Casal Quinteiro (nacho) 2015-08-05 12:10:54 UTC
Review of attachment 308288 [details] [review]:

Hey Fan,

could you fix the coding style here? It seems you use the gnu one while it should be the linux one.
Comment 5 Dan Winship 2015-08-05 12:18:40 UTC
Comment on attachment 308288 [details] [review]
soup_init(): Don't Hardcode Paths on Windows

>+BOOL WINAPI
>+DllMain (HINSTANCE hinstDLL,

Could this be done with G_DEFINE_CONSTRUCTOR instead?

>+  gchar* basedir = g_win32_get_package_installation_directory_of_module (soup_dll);
>+  gchar* localedir = g_build_filename (basedir, "share", "locale", NULL);

Just "char", not "gchar" please.
Comment 6 Fan, Chun-wei 2015-08-05 16:41:02 UTC
Created attachment 308806 [details] [review]
soup_init(): Don't Hardcode Paths on Windows, take 2

Hi Dan,

Thanks for the reviews, the patches were pushed as follows:
attachment 308286 [details] [review]: a83276a
attachment 308287 [details] [review]: 407985a

(In reply to Dan Winship from comment #5)
> Could this be done with G_DEFINE_CONSTRUCTOR instead?
Not really.  The Windows relocatable functions (i.e. g_win32_get_package_installation_directory_of_module()) when used against a library DLL looks for a HMODULE that comes from the HINSTANCE that comes from the DllMain() when the process is being attached.  This is similar to what is done in GLib and GTK+ and so, so this is different from G_DEFINE_CONSTRUCTOR, which is used more for GResource.

> Just "char", not "gchar" please.
Did that in the new patch, along with Nacho's suggestions.

With blessings, thank you!
Comment 7 Ignacio Casal Quinteiro (nacho) 2015-08-06 10:19:31 UTC
Review of attachment 308806 [details] [review]:

See the other minor issue.

::: libsoup/soup-session.c
@@ +107,3 @@
 {
+#ifdef G_OS_WIN32
+	char* basedir = g_win32_get_package_installation_directory_of_module (soup_dll);

minor thing here: char *basedir instead of char* basedir, same for the other variable
Comment 8 Fan, Chun-wei 2015-08-06 11:01:38 UTC
Created attachment 308844 [details] [review]
soup_init(): Don't Hardcode Paths on Windows, take 2.1

Hi Nacho,

(In reply to Ignacio Casal Quinteiro (nacho) from comment #7)
> minor thing here: char *basedir instead of char* basedir, same for the other
> variable

Oops :|  Got it.

Thanks though, with blessings.
Comment 9 Dan Winship 2015-08-06 11:39:21 UTC
Comment on attachment 308844 [details] [review]
soup_init(): Don't Hardcode Paths on Windows, take 2.1

looks good.

(Well, if you want, you could unindent the two "case" lines and the "default" line by one level :-)
Comment 10 Fan, Chun-wei 2015-08-06 16:54:03 UTC
Hi Dan,

Thanks for the review, the patch (with the indentation updated :) ) was pushed as f58c987.

With blessings, thank you!