Fix GTK for Windows links in README #218

rnestler · 2016-01-02T13:23:13Z

Fixes issue #216

GuillaumeGomez · 2016-01-02T13:25:48Z

README.md

@@ -32,7 +32,7 @@ __gtk__ expects __GTK+__, __GLib__ and __Cairo__ development files to be install
 ### Windows

 Install [mingw-w64](http://mingw-w64.yaxm.org/) (select the win32 threading model) and download a __GTK+__ SDK:
- * The GNOME project has an official distribution of GTK+ 3.6: [x86](http://www.gtk.org/download/win32.php), [x64](http://www.gtk.org/download/win64.php).
+ * The GNOME project has official distributions of GTK+ 3.4 upto 3.10: http://win32builder.gnome.org/


I'd prefer if the link was put "inside" "3.4 upto 3.10". Example:

The GNOME project has official distributions of GTK+ 3.4 upto 3.10.

In second thought it seems a bit weird... What form do you prefer @gkoz?

What about "The GNOME project has official distributions of GTK+ 3.4 upto 3.10" ?

Oh perfect!

GuillaumeGomez · 2016-01-02T13:26:52Z

Thanks! Once we confirmed my note, I'll merge.

rnestler · 2016-01-02T15:41:26Z

OK I updated the PR like we discussed, should be ready to merge.

GuillaumeGomez · 2016-01-02T15:47:45Z

Didn't you want to add a howto as well ?

rnestler · 2016-01-02T16:24:47Z

Ah sorry, didn't see your comment on #216 until now. Yeah I can do it in this PR as well.

GuillaumeGomez · 2016-01-02T16:25:32Z

Then ping me when it's done please, github doesn't notify when a PR has been updated.

rnestler · 2016-01-02T21:36:11Z

OK I added a first version of the HowTo. A few points to note:

Should we keep the old instructions or should I just remove them?
I only tested in a 32Bit Windows VM but wrote the HowTo for 64Bit since I guessed it's more common.
I also added a note about installing rust for the GNU ABI and not installing "Linker and plattform libraries".

gkoz · 2016-01-02T22:00:35Z

README.md

+![Screenshot](rust_setup.png)
+
+If you're not sure, check if your Rust installation has `gcc.exe` and `ld.exe`
+in its `bin` directory, you may get a linking error `ld: cannot find -limm32`.


This sentence seems slightly broken now. And I think it's now safe to say that the presence of those files will definitely cause the error.

Yes you're right. Will fix that.

gkoz · 2016-01-02T22:16:34Z

Should we keep the old instructions or should I just remove them?

It makes sense to remove them but maybe later after we have some feedback about the msys2 way.

I only tested in a 32Bit Windows VM but wrote the HowTo for 64Bit since I guessed it's more common.

I can test this on 64-bit later. BTW you don't need a local VM, appveyor allows rdp access.

gkoz · 2016-01-02T22:18:28Z

README.md

+
+#### 1. msys2 toolchain
+
+This method is recommended according to the [gtk project]


gtk should probably be spelled GTK+ here.

rnestler · 2016-01-02T22:30:33Z

I can test this on 64-bit later. BTW you don't need a local VM, appveyor allows rdp access.

Ah that's awesome I didn't now that! 😄 This can save me a lot of time while trying to get this to build with AppVeyor!

rnestler · 2016-01-02T22:38:05Z

@gkoz I tried to address all your points. Tell me if I should squash the commits or if there are any other issues.

GuillaumeGomez · 2016-01-02T22:42:18Z

Please keep the first commit and squash other in the second one please.

rnestler · 2016-01-02T22:50:44Z

@GuillaumeGomez Squashed it. Is it OK like that?

GuillaumeGomez · 2016-01-02T22:51:48Z

@rnestler: Perfect! But it's @gkoz who will merge when he thinks everything's ok.

gkoz · 2016-01-02T22:52:03Z

Some grammar nits...

gkoz · 2016-01-02T22:52:17Z

README.md

+"Linker and platform libraries" in the Rust setup.
+![Screenshot](rust_setup.png)
+
+If you already installed Rust, check if your Rust installation has `gcc.exe`


"If you already have installed"

rnestler · 2016-01-02T23:08:43Z

Some grammar nits...

Yeah sorry about that. I'm not a native english speaker and my grammar is poor 😢

GuillaumeGomez · 2016-01-02T23:10:38Z

README.md

+![Screenshot](rust_setup.png)
+
+If you already installed Rust, check if your Rust installation has `gcc.exe`
+and `ld.exe` in its `bin` directory.  In that case remove those executables,


" In" -> " In"

I assume you mean I should remove the double space? That's actually my vim setup auto formatting the sentences with two spaces after the period.

GuillaumeGomez · 2016-01-02T23:13:09Z

README.md

+ $ pacman -S mingw-w64-x86_64-toolchain
+ ```
+ - Install the gtk3 package
+ ```


Add an empty line before "```".

gkoz · 2016-01-02T23:47:46Z

Looks good 👍

GuillaumeGomez · 2016-01-02T23:48:47Z

README.md

+or follow these steps:
+
+ - Install and update [msys2](https://msys2.github.io/)
+ - Install the mingw toolchain


Please add ":" at the end of this sentence.

GuillaumeGomez · 2016-01-02T23:50:22Z

Seems good to me too. Once you have fixed my last notes, we merge.

Explain how to install the msys2 toolchain as an alternative to using wingw-w64 with a GTK distribution. Also add a note about how to correctly install Rust so there is no conflict with the linker provided by rust.

rnestler · 2016-01-02T23:56:10Z

@GuillaumeGomez Done :)

GuillaumeGomez · 2016-01-02T23:56:52Z

Thanks a lot for your contribution @rnestler! I merge.

Fix GTK for Windows links in README

rnestler · 2016-01-02T23:58:54Z

Happy to help! Thanks for your work on gtk-rs!

gkoz · 2016-01-07T13:43:07Z

README.md

+ - Install the mingw toolchain:
+
+ ```Shell
+ $ pacman -S mingw-w64-x86_64-toolchain


Does this actually work? I've tried this in appveyor and it didn't because the msys2 mingw is incorrect flavor, it's not win32+dwarf/seh. I ended up using mingw-builds, the installer linked at http://mingw-w64.org/doku.php/download/mingw-builds

I wasn't using the msys2-provided gtk though but the same distributions as before.

Temporary fix for issue gtk-rs#210

GuillaumeGomez reviewed Jan 2, 2016
View reviewed changes

Fix GTK for Windows links in README

bf38259

rnestler force-pushed the fix_gtk_on_windows_links branch from 68d4845 to bf38259 Compare January 2, 2016 15:39

rnestler force-pushed the fix_gtk_on_windows_links branch from a3083db to e08165e Compare January 2, 2016 19:23

gkoz reviewed Jan 2, 2016
View reviewed changes

README.md

#### 1. msys2 toolchain

This method is recommended according to the [gtk project]

Copy link

Member

gkoz Jan 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gtk should probably be spelled GTK+ here.

rnestler force-pushed the fix_gtk_on_windows_links branch from 18b6281 to a6bd09a Compare January 2, 2016 22:49

gkoz reviewed Jan 2, 2016
View reviewed changes

GuillaumeGomez reviewed Jan 2, 2016
View reviewed changes

README.md

$ pacman -S mingw-w64-x86_64-toolchain

```

- Install the gtk3 package

```

Copy link

Member

GuillaumeGomez Jan 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an empty line before "```".

rnestler force-pushed the fix_gtk_on_windows_links branch 2 times, most recently from e8ca0c0 to b16afdd Compare January 2, 2016 23:46

GuillaumeGomez reviewed Jan 2, 2016
View reviewed changes

Add HowTo for using the msys2 toolchain in README

1a58951

Explain how to install the msys2 toolchain as an alternative to using wingw-w64 with a GTK distribution. Also add a note about how to correctly install Rust so there is no conflict with the linker provided by rust.

rnestler force-pushed the fix_gtk_on_windows_links branch from b16afdd to 1a58951 Compare January 2, 2016 23:52

GuillaumeGomez added a commit that referenced this pull request Jan 2, 2016

Merge pull request #218 from rnestler/fix_gtk_on_windows_links

e067474

Fix GTK for Windows links in README

GuillaumeGomez merged commit e067474 into gtk-rs:master Jan 2, 2016

rnestler deleted the fix_gtk_on_windows_links branch January 2, 2016 23:58

rnestler mentioned this pull request Jan 3, 2016

GTK for Windows links in Readme are broken #216

Closed

gkoz reviewed Jan 7, 2016
View reviewed changes

This was referenced Jan 13, 2016

Update README.md - in Windows msys2 instructions, specify that PATH needs to be updated #223

Merged

Use msys2 for the AppVeyor build #234

Closed

EPashkin pushed a commit to EPashkin/gtk that referenced this pull request Apr 18, 2016

Merge pull request gtk-rs#218 from ptersilie/master

e879569

Temporary fix for issue gtk-rs#210

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GTK for Windows links in README #218

Fix GTK for Windows links in README #218

rnestler commented Jan 2, 2016

GuillaumeGomez Jan 2, 2016

rnestler Jan 2, 2016

GuillaumeGomez Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

gkoz Jan 2, 2016

rnestler Jan 2, 2016

gkoz commented Jan 2, 2016

gkoz Jan 2, 2016

rnestler commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

gkoz commented Jan 2, 2016

gkoz Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez Jan 2, 2016

rnestler Jan 2, 2016

GuillaumeGomez Jan 2, 2016

gkoz commented Jan 2, 2016

GuillaumeGomez Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

gkoz Jan 7, 2016

gkoz Jan 7, 2016


		#### 1. msys2 toolchain

		This method is recommended according to the [gtk project]

Fix GTK for Windows links in README #218

Fix GTK for Windows links in README #218

Conversation

rnestler commented Jan 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkoz commented Jan 2, 2016

Choose a reason for hiding this comment

rnestler commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

gkoz commented Jan 2, 2016

Choose a reason for hiding this comment

rnestler commented Jan 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkoz commented Jan 2, 2016

Choose a reason for hiding this comment

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

GuillaumeGomez commented Jan 2, 2016

rnestler commented Jan 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment