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 759838 - Add Visual Studio build support
Add Visual Studio build support
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
unspecified
Other Windows
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-24 09:34 UTC by Fan, Chun-wei
Modified: 2016-01-26 15:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtksourceview/gtksourcelanguage.c: Check for G_OS_WIN32 later (1.03 KB, patch)
2015-12-24 09:38 UTC, Fan, Chun-wei
committed Details | Review
gtksourceview/gtksourcegutterrendermarks.c: Don't use empty structs (968 bytes, patch)
2015-12-24 09:40 UTC, Fan, Chun-wei
none Details | Review
GtkSourceTag: Ensure items are annotated for export (1.32 KB, patch)
2015-12-24 09:48 UTC, Fan, Chun-wei
committed Details | Review
Decorate the prototypes of the generated enumeration sources/headers (2.05 KB, patch)
2015-12-24 09:52 UTC, Fan, Chun-wei
none Details | Review
gtksourceview/gtksourceview.c: Force inclusion of libgtksourceviewwordsprovider (2.78 KB, patch)
2015-12-24 10:01 UTC, Fan, Chun-wei
none Details | Review
build: Add common autotools modules for generating Visual Studio files (15.27 KB, patch)
2015-12-24 10:06 UTC, Fan, Chun-wei
committed Details | Review
The Visual Studio support files (115.83 KB, patch)
2015-12-24 10:12 UTC, Fan, Chun-wei
none Details | Review
gtksourceview/gtksourcegutterrendermarks.c: Don't use empty structs (take ii) (1.98 KB, patch)
2015-12-28 10:06 UTC, Fan, Chun-wei
committed Details | Review
Decorate the prototypes of the generated enumeration sources/headers (take ii) (2.09 KB, patch)
2015-12-28 10:08 UTC, Fan, Chun-wei
committed Details | Review
libgtksourcecompletionwords: Annotate public symbols for export (1.63 KB, patch)
2015-12-28 10:09 UTC, Fan, Chun-wei
committed Details | Review
gtksourceutils.c: Add missed config.h include (817 bytes, patch)
2015-12-28 10:12 UTC, Fan, Chun-wei
committed Details | Review
Private headers: Add GTK_SOURCE_INTERNAL (17.68 KB, patch)
2015-12-28 10:27 UTC, Fan, Chun-wei
none Details | Review
The Visual Studio support files (take ii) (85.38 KB, patch)
2015-12-28 10:33 UTC, Fan, Chun-wei
none Details | Review
test-file-saver: Fix running on Windows (2.02 KB, patch)
2015-12-28 10:56 UTC, Fan, Chun-wei
none Details | Review
Private headers: Add GTK_SOURCE_INTERNAL (take ii) (21.33 KB, patch)
2015-12-28 11:06 UTC, Fan, Chun-wei
committed Details | Review
The Visual Studio support files (take iii) (85.46 KB, patch)
2015-12-28 11:09 UTC, Fan, Chun-wei
committed Details | Review
test-file-saver: Fix running on Windows (take ii) (7.99 KB, patch)
2016-01-26 06:55 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2015-12-24 09:34:50 UTC
Hi,

This is the bug that attempts to:

-Make the updates to the code that are necessary to build
 GTKSourceView on Visual Studio
-Add Visual Studio 2008-2015 project files to build
 GTKSourceview.
-Support build of introspection files via NMake.

Patches to follow...

With blessings, thank you!
Comment 1 Fan, Chun-wei 2015-12-24 09:38:22 UTC
Created attachment 317847 [details] [review]
gtksourceview/gtksourcelanguage.c: Check for G_OS_WIN32 later

Hi,

In gtksourceview/gtksourcelanguage.c, we are actually checking for G_OS_WIN32 prematurely, as that is something that is defined after the GLib headers are included, so we need to defer that inclusion a bit.

MinGW builds aren't affected by this at least because they ship with an implementation of unistd.h which contains the necessary includes and prototypes...
Comment 2 Fan, Chun-wei 2015-12-24 09:40:39 UTC
Created attachment 317848 [details] [review]
gtksourceview/gtksourcegutterrendermarks.c: Don't use empty structs

Hi,

Empty structs are items which will probably not be supported in any Visual Studio release, as with VLA's, so we need a dummy member to avoid this problem.  Let me know if some other way is preferred-this is checked with 'make check' via 'make distcheck'...
Comment 3 Fan, Chun-wei 2015-12-24 09:48:06 UTC
Created attachment 317849 [details] [review]
GtkSourceTag: Ensure items are annotated for export

Hi,

gtksourcetag.c missed an include for config.h, meaning that the symbols there are not exported via the version macros, and the prototype of gtk_source_tag_get_type(), which is done using G_DECLARE_DERIVABLE_TYPE, was not annotated by such version macros.  Make up for these...
Comment 4 Fan, Chun-wei 2015-12-24 09:52:24 UTC
Created attachment 317850 [details] [review]
Decorate the prototypes of the generated enumeration sources/headers

Hi,

Since the generated enumeration header is installed, I think the symbols in there should be decorated for export as well.  This patch adds a macro that equals _GTK_SOURCE_EXTERN that is used by the generated enumeration header, and include config.h in the generated enumeration source...
Comment 5 Fan, Chun-wei 2015-12-24 10:01:09 UTC
Created attachment 317851 [details] [review]
gtksourceview/gtksourceview.c: Force inclusion of libgtksourceviewwordsprovider

Hi,

Apparently the code in main GTKSourceView sources does not refer to the words completion provider sources.   The words completion provider was to be built as a static lib that is to be linked into the GTKSourceView DLL, but on Visual Studio builds at least, the static lib is not included in the DLL as a result, so the public symbols are not linked into nor exported from the resulting GTKSourceView DLL.

Attempt to fix that by using a dummy reference to it, when ENABLE_PROVIDERS is defined...
Comment 6 Fan, Chun-wei 2015-12-24 10:06:57 UTC
Created attachment 317852 [details] [review]
build: Add common autotools modules for generating Visual Studio files

Hi,

As with the other GNOME C-language projects where I maintain the Visual Studio projects, copy the autotools modules from GLib and gobject-introspection which is used to:

-Generate the complete projects for the libs gtksourceview-completion-words,
 gtksourceview-core and gtksourceview, and the property sheet to install
 the built items, headers and data files.
-Generate the Visual Studio 2012-2015 projects from the 2010 ones by copying
 them and updating the items as necessary.
-Generate the complete command lines for generating introspection files...
Comment 7 Fan, Chun-wei 2015-12-24 10:12:42 UTC
Created attachment 317853 [details] [review]
The Visual Studio support files

Hi,

This is the patch to add the Visual Studio projects, support files, and associated items for building GTKSourceView and introspection for it, for Visual Studio 2008-2015.

Please note that since gtksourceversion.h.in defines GTK_SOURCE_VERSION_MIN_REQUIRED as GTK_SOURCE_VERSION_CUR_STABLE by default, in the projects we define GTK_SOURCE_VERSION_MIN_REQUIRED as GTK_SOURCE_VERSION_3_0 in the projects, so that we get the maximum set of APIs built into (and exported from) the DLL.  Just wondering, is this intended in the autotools builds as well?

With blessings, thank you, and Happy Holidays (Merry Christmas from my part of the world)!
Comment 8 Paolo Borelli 2015-12-24 11:13:54 UTC
Hi Fan, Happy Holidays to you too, and thanks for these patches, VS support is a wonderful Christmas present!


I'll start marking the obvious ones as accept-commit-now, for the ones I am not sure I'll try to test in the next days or let swilmet and nacho comment (e.g. swilmet is more familiar than me with the itstool changes)
Comment 9 Paolo Borelli 2015-12-24 11:14:26 UTC
Review of attachment 317847 [details] [review]:

looks good
Comment 10 Paolo Borelli 2015-12-24 11:16:57 UTC
Review of attachment 317848 [details] [review]:

I think we can simply remove the struct and the typedef and replace the *priv pointer in the main struct with


gpointer *priv; /* unused for now */
Comment 11 Paolo Borelli 2015-12-24 11:19:01 UTC
Review of attachment 317849 [details] [review]:

I did not know that G_DECLARE macros could be annotated... does it work just because the get_type prototype is at the start of the expansion?

It looks ok though.
Comment 12 Paolo Borelli 2015-12-24 11:20:35 UTC
Review of attachment 317850 [details] [review]:

looks good, though maybe we need to mark the macro as "skip" for gtk-doc so it does not complain about it (see recent commits by swilmet)
Comment 13 Paolo Borelli 2015-12-24 11:23:02 UTC
Review of attachment 317851 [details] [review]:

mmm, I do not like this very much... is there any other way to achieve it? For instance on gcc I would have probably fiddled with -Wl,whole-archive linker option or somthing like that. Maybe MS has something similar?
Comment 14 Paolo Borelli 2015-12-24 11:24:23 UTC
Review of attachment 317852 [details] [review]:

For this patch, you are the VS master, so feel free to add it and make changes to it whenever you think it is right
Comment 15 Paolo Borelli 2015-12-24 11:25:21 UTC
Review of attachment 317853 [details] [review]:

For this patch, you are the VS master, so feel free to add it and make changes to it whenever you think it is right
Comment 16 Fan, Chun-wei 2015-12-24 14:53:13 UTC
Hi Paolo,

Thanks for the comments and reviews, sorry I can't deliver the whole package in time for Christmas, couldn't get enough time for them :)

I pushed these patches as follows:
-Attachment 317847 [details]: 463c7f8
-Attachment 317849 [details]: d5b1b9e (Yes, the G_DECLARE can be annotated, see GTK+'s gtk/gtknativedialog.h for an example)

Sorry, I forgot to add the bug URL reference to these patches as I pushed them.

---
For 317851, this is a bit more open for discussion, where the situation on Visual Studio can be outlined on the MSDN blog page[1].  Some other other ways are possible for Visual Studio, on top of my head:
-Use the /INCLUDE linker directive in the project settings,
 but this is not very maintainable.

-Build the words completion provider sources directly into the DLL,
 not via a static lib.  This could mean dropping the
 gtksourceview-completion-words.vcproj and:
--Add the words completion provider sources into the gtksourceview project
  directly -or-
--Update autotools build files-don't build libgtksourcecompletionwords, but
  instead build these sources directly into libgtksourceview, like what is
  done on GTK+ for GTKInspector and GTK+'s a11y.

-Like the first solution, possible do something a bit similar to
 glib/gconstructor.h[2], that is define a macro in the C source
 that also calls /INCLUDE:xxx appropriately, so that the symbol is
 force-included.

My take on this.

[1]: https://blogs.msdn.microsoft.com/oldnewthing/20140321-00/?p=1433/
[2]: https://git.gnome.org/browse/glib/commit/?id=db4df9908e0137c14f5aeeefba899240c4724970

Thanks, with blessings, and Happy Holidays (and Merry Christmas).
Comment 17 Paolo Borelli 2015-12-24 16:30:01 UTC
I would not mind dropping the intermediate static lib and just list the wordscompletion stuff in the main dll
Comment 18 Fan, Chun-wei 2015-12-28 10:06:04 UTC
Created attachment 317957 [details] [review]
gtksourceview/gtksourcegutterrendermarks.c: Don't use empty structs (take ii)

Hi,

This is the patch for not using an empty struct for gtksourcegutterrenderermarks.c, as suggested by Paolo...
Comment 19 Fan, Chun-wei 2015-12-28 10:08:09 UTC
Created attachment 317958 [details] [review]
Decorate the prototypes of the generated enumeration sources/headers (take ii)

Hi,

This is the patch to export the enum functions from the generated enum headers and sources, with the check for GTK-doc to "skip" that macro...
Comment 20 Fan, Chun-wei 2015-12-28 10:09:49 UTC
Created attachment 317959 [details] [review]
libgtksourcecompletionwords: Annotate public symbols for export

Hi,

We still need to annotate the public symbols for export, for any cases...
Comment 21 Fan, Chun-wei 2015-12-28 10:12:11 UTC
Created attachment 317960 [details] [review]
gtksourceutils.c: Add missed config.h include

Hi,

This is needed as otherwise the public symbols implemented here won't get exported...
Comment 22 Paolo Borelli 2015-12-28 10:21:26 UTC
Review of attachment 317957 [details] [review]:

looks good
Comment 23 Paolo Borelli 2015-12-28 10:22:05 UTC
Review of attachment 317958 [details] [review]:

looks good
Comment 24 Paolo Borelli 2015-12-28 10:23:03 UTC
Review of attachment 317960 [details] [review]:

sure
Comment 25 Paolo Borelli 2015-12-28 10:23:26 UTC
Review of attachment 317959 [details] [review]:

go ahead
Comment 26 Fan, Chun-wei 2015-12-28 10:27:27 UTC
Created attachment 317961 [details] [review]
Private headers: Add GTK_SOURCE_INTERNAL

Hi,

This adds a GTK_SOURCE_INTERNAL macro into gtksourcetypes-private.h, which is defined as __declspec(dllexport) for Visual Studio, and G_GNUC_INTERNAL for the other compilers (so that things work as before on non-Visual Studio).  This is necessary as:

-We can't have an empty project that just links 2 static libs to
 produce the final DLL for GTKSourceView at the current state of the
 code, as the linker will not include the items in the static libs
 unless they are used from the code in the main DLL (and we don't have
 any code here).
-There isn't a -Wl,whole-archive option for the linker for Visual
 Studio, or anything like that.
-As a result, we need to build everything in one shot in the main
 GTKSourceView project.  This is necessary to build the unit tests
 against Visual Studio builds...
Comment 27 Fan, Chun-wei 2015-12-28 10:33:07 UTC
Created attachment 317963 [details] [review]
The Visual Studio support files (take ii)

Hi,

This is the set of Visual Studio project files that is used to build GTKSourceView.  As noted earlier, this builds libgtksourcecompletionwords and libgtksourceview-core in one shot, so that we can ensure everything that should be in the DLL is included, without making much additional changes to the code...
Comment 28 Paolo Borelli 2015-12-28 10:34:59 UTC
Review of attachment 317961 [details] [review]:

A couple of minor issues below.

Feel free to push directly once you have fixed them

::: gtksourceview/gtksourcegutterrenderermarks.h
@@ -45,3 @@
 	GtkSourceGutterRendererPixbuf parent;
 
-	gpointer priv; /* unused for now */

This change is unrelated and it is also incorrect since gpointer is already a pointer

::: gtksourceview/gtksourcetypes-private.h
@@ -40,2 +40,3 @@
 typedef struct _GtkSourceUndoManagerDefault	GtkSourceUndoManagerDefault;
 
+#ifdef _MSC_VER

can we add a short comment here? like

/* on MSVC we need to export symbols used by unit tests */
Comment 29 Paolo Borelli 2015-12-28 10:41:13 UTC
Review of attachment 317963 [details] [review]:

A tiny nitpick below.

Apart from that feel free to push and in the future make any chages in the win32 subdir

::: gtksourceview/Makefile.am
@@ +332,3 @@
+# MSVC items
+
+# Pull in the libgtksourcecompletionwords sources

Let's be more explicit in this comment:

# Pull in the libgtksourcecompletionwords sources since on MSVC we want to
# build everything in the same .dll with no intermediate libs
Comment 30 Fan, Chun-wei 2015-12-28 10:56:45 UTC
Created attachment 317964 [details] [review]
test-file-saver: Fix running on Windows

Hi,

This fixes the fix-file-saver test on Windows as we don't have /tmp unless we are in a bash-style shell.  I know this will have leaks, but as a test program...

Please let me know if I should try to do better in the leak pluggin department.
----
Hi Paolo,

Thanks for the fast reviews when I was uploading the patches, I pushed them to master as:
-attachment 317957 [details] [review]: 826e11d
-attachment 317958 [details] [review]: 262c23f
-attachment 317959 [details] [review]: 8c31df4
-attachment 317960 [details] [review]: 259f6b9

With blessings, thank you!
Comment 31 Fan, Chun-wei 2015-12-28 11:06:52 UTC
Created attachment 317965 [details] [review]
Private headers: Add GTK_SOURCE_INTERNAL (take ii)

Hi,

I am re-uploading this patch as I forgot about those that needed similar treatment in gtksourceview/completion-providers/words...

The gtksourcegutterrenderermarks.h was really my fault, copied that file by accident, which clearly should not have been :)

With blessings, thank you, and Happy New Year!
Comment 32 Paolo Borelli 2015-12-28 11:09:06 UTC
Review of attachment 317964 [details] [review]:

This is a bit ugly... I do not care much about the leaks, but those defines are for constants, I do not like hiding a function call in them.

I checked and each of the modified defines is used just in a place or two, I would prefer if we just define something like:

#define DEFAULT_LOCAL_FILENAME "gtksourceview-file-saver-test.txt"

and then call get_tmp_dir in the caller and free the path when it is not needed anymore
Comment 33 Fan, Chun-wei 2015-12-28 11:09:52 UTC
Created attachment 317966 [details] [review]
The Visual Studio support files (take iii)

Hi Paolo,

Thanks, I have added the comment as suggested.

(BTW, there are some symbols marked with G_GNUC_INTERNAL in gtksourcestylescheme.h that are used for the tests, which is a public header--I didn't touch them as I was not sure about them, but this means test-stylescheme.c would not link for Visual Studio builds).

With blessings, thanks, and Happy New Year!
Comment 34 Paolo Borelli 2015-12-28 11:09:57 UTC
Review of attachment 317965 [details] [review]:

sure
Comment 35 Paolo Borelli 2015-12-28 11:10:44 UTC
Review of attachment 317966 [details] [review]:

go ahead
Comment 36 Paolo Borelli 2015-12-28 11:12:58 UTC
Review of attachment 317966 [details] [review]:

about those prototypes, I am ok with splitting a private.h header where needed.
Comment 37 Fan, Chun-wei 2015-12-28 11:21:00 UTC
Hi Paolo,

Thanks for the reviews, I have pushed the patches as follows:
attachment 317965 [details] [review]: e875bab5
attachment 317852 [details] [review]: ba388c5
attachment 317966 [details] [review]: e368ed2

I will look at the tests item later on, so now the GTKSourceView library can be built by Visual Studio out of the box.

Happy New Year, with blessings!
Comment 38 Fan, Chun-wei 2016-01-26 06:55:15 UTC
Created attachment 319724 [details] [review]
test-file-saver: Fix running on Windows (take ii)

Hi,

Sorry, I didn't get back to this until today...
Comment 39 Paolo Borelli 2016-01-26 08:05:25 UTC
Review of attachment 319724 [details] [review]:

Thanks looks good.

Am I too annoying if I ask you to split variable declaration and assignment?
I prefer to to just declare the variable at the start of the block and do the assignment later if the assignment involves calling a function instead of beeing just "int foo = 0".
Comment 40 Sébastien Wilmet 2016-01-26 11:50:02 UTC
Are all the accepted patches pushed to master? If so, this bug can be closed or the merged patches should be marked as committed (this can be done automatically with git-bz).
Comment 41 Paolo Borelli 2016-01-26 11:58:15 UTC
we can close this once Fan pushes the test patch
Comment 42 Fan, Chun-wei 2016-01-26 15:45:16 UTC
Hi Paolo,

Thanks!  I pushed the patch...

(In reply to Paolo Borelli from comment #39)
> Am I too annoying if I ask you to split variable declaration and assignment?
> I prefer to to just declare the variable at the start of the block and do
> the assignment later if the assignment involves calling a function instead
> of beeing just "int foo = 0".

with this suggested change, as 12cb529.

---

Hi Sébastien,

Sorry, I forgot to mark the pushed patches... Since all the patches here are pushed to master at this time, I will close the bug shortly.

---

With blessings, thank you!
Comment 43 Fan, Chun-wei 2016-01-26 15:48:25 UTC
Hi Paolo,

(In reply to Paolo Borelli from comment #36)
> about those prototypes, I am ok with splitting a private.h header where
> needed.

I agree with this, let's do this in another bug...

With blessings, thank you!