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

Adds dependencies to the README.md #52

Merged
merged 3 commits into from
Dec 29, 2018

Conversation

officialgupta
Copy link
Contributor

No description provided.

README.md Outdated

These can be included in your gopath by the following 'get' commands:

* "go get github.com/go-gl/gl/v3.2-core/gl"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary. A go get github.com/fyne-io/fyne/... would pull in all of these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my experience it has never pulled in these dependencies

Copy link
Member

Choose a reason for hiding this comment

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

Was that back when you needed to pass -tags gl? Since we moved this to the default driver it should indeed be downloading the dependencies at the same time.

I will try on a fresh installation and see what happens.

Copy link
Member

Choose a reason for hiding this comment

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

You're right it does not work.
However, instead of listing the dependencies how about we do this instead? (as it will always work, even when the deps change)

go get -u github.com/fyne-io/fyne
cd $GOPATH/src/github.com/fyne-io/fyne
go get -u ./...

README.md Outdated
* "go get github.com/srwiley/rasterx"
* "go get golang.org/x/image/font"

You will also require a GCC compiler if you do not have one, this can be included by installing TDM-GCC.
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes a Windows user, which is not necessarily the case. This could very well be a Darwin/Linux/*BSD/other user.

Also, TDM-GCC is not necessarily the best way to handle Windows CGO support (I had not even heard of it before now). If we even think that we need to explain this, we should link to the Go wiki: https://github.com/golang/go/wiki/WindowsBuild

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this could be phrased more along the lines of "Fyne uses CGo, which requires a C compiler to be installed. If you don't already have one installed you can ...".
After all it's not like we require a C compiler directly so it would be good if this sounds like less of a special case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, we may be able to remove CGo dependency on windows in the near future 😄

go-gl/glow#102

README.md Outdated
@@ -143,3 +143,24 @@ func main() {

The main examples have been moved - you can find them in their [own repository](https://github.com/fyne-io/examples/).

## Non-standard library dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

We could just rephrase this as "dependencies". This would be more succinct.

@niaow
Copy link
Contributor

niaow commented Dec 24, 2018

We may also want to think about whether we need to document these at all. If we are going to document these, then we will need to remember to update this when adding or removing dependencies.

@andydotxyz
Copy link
Member

Hmm, @officialgupta is right - the dependencies are not pulled automatically. I would prefer that we can fix that problem rather than list the dependencies in documentation - if possible.

The CGo portion of this is still important - perhaps we can follow two different threads?
I have created #53 to track the install problem.

I'm sorry this PR has not simply been accepted - what do you think about just focussing on the C portion, with the expectation that #53 will be resolved?

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I have left 2 last comments - I think if these are rounded out then we can accept the PR - thanks :)

README.md Outdated

These can be included in your gopath by the following 'get' commands:

* "go get github.com/go-gl/gl/v3.2-core/gl"
Copy link
Member

Choose a reason for hiding this comment

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

You're right it does not work.
However, instead of listing the dependencies how about we do this instead? (as it will always work, even when the deps change)

go get -u github.com/fyne-io/fyne
cd $GOPATH/src/github.com/fyne-io/fyne
go get -u ./...

README.md Outdated
* "go get github.com/srwiley/rasterx"
* "go get golang.org/x/image/font"

You will also require a GCC compiler if you do not have one, if on Windows this can be included by installing TDM-GCC.
Copy link
Member

Choose a reason for hiding this comment

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

I agree this GCC mention is probably needed, but can we structure it a little more to be about CGo is required and link to some document about setting this up. If I was new to developing with Windows I'm not sure that "instal TDM-GCC" would be enough detail for me to get up and running?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some digging and the official Golang documentation states that for Windows C support to use minGW; however many people complain this solution doesn't work and to use TDM-GCC. Hence I continued to incude it.

What exactly would be required for the CGo details? This shouldn't be a problem if users do a full Go install, usually issues witch CGo arise when a GCC compiler doesn't exist on the system.

@andydotxyz
Copy link
Member

With #53 resolved the dependency issue should have gone away, so it should work with no workaround now.

-> Removes library dependencies fixed in fyne-io#53
-> Adds link to where to download the TDM-GCC used for windows.
@officialgupta
Copy link
Contributor Author

officialgupta commented Dec 29, 2018

With #53 resolved the dependency issue should have gone away, so it should work with no workaround now.

These have been removed in 311b544

@andydotxyz
Copy link
Member

Thanks for your work on this :)
For CGo it is always there but may not work without the C compiler. I guess this covers it for Windows users. Personally I use mingw_w64 which seems to work OK.

@andydotxyz andydotxyz merged commit 1be697d into fyne-io:develop Dec 29, 2018
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