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

Init golang module #37

Merged
merged 3 commits into from
Nov 15, 2020
Merged

Init golang module #37

merged 3 commits into from
Nov 15, 2020

Conversation

ninedraft
Copy link
Contributor

Init golang modules to pin dependencies

@coveralls
Copy link

coveralls commented Nov 14, 2020

Coverage Status

Coverage remained the same at 95.298% when pulling 1eff170 on ninedraft:init-go-module into 4c378b9 on galeone:master.

@galeone
Copy link
Owner

galeone commented Nov 14, 2020

I'm not completely familiar with go.mod, can you explain to me exactly what every line of the file does?

require github.com/tensorflow/tensorflow v2.1.0+incompatible

I mean, it looks that explicit the dependencies, however, as I wrote in the readme, we should depend on the fork I created, otherwise we have a problem with the compiled protocol buffers that are not available in the tensorflow official repo.

Moreover, I see the go version: 1.15. That's not enough for TensorFlow (and thus, tfgo), it's too general. We need to depend on a specific minor version, or better, we have to exclude a specific minor version.

The TensorFlow Go bindings use a lot of opaque struct pointers, that in version 1.15.3 are broken ( golang/go#42032 ), thus we should be able to explicit: every go version != 1.15.3. Something like go >= 1.15, !=1.15.3 is it possible?

@galeone
Copy link
Owner

galeone commented Nov 14, 2020

I see the new commit with the latest change, is this line correct?

replace github.com/tensorflow/tensorflow => github.com/galeone/tensorflow v1.12.2-0.20201110143501-1b6f13331f4d

I understand that this is used to replace with galeone/tensorflow, but is this pointing to the correct branch? The correct branch is r2.3-go

@ninedraft
Copy link
Contributor Author

I mean, it looks that explicit the dependencies, however, as I wrote in the readme, we should depend on the fork I created, otherwise we have a problem with the compiled protocol buffers that are not available in the tensorflow official repo.

You are right, I fixed go.mod to use your fork

@ninedraft
Copy link
Contributor Author

Go mod reference can be found here: https://golang.org/ref/mod

@galeone
Copy link
Owner

galeone commented Nov 14, 2020

👍 but is it pointing to branch r.2.3-go of my fork?

It looks like is pointing to another branch

@ninedraft
Copy link
Contributor Author

The TensorFlow Go bindings use a lot of opaque struct pointers, that in version 1.15.3 are broken ( golang/go#42032 ), thus we should be able to explicit: every go version != 1.15.3. Something like go >= 1.15, !=1.15.3 is it possible?

I think not, I think it can be achieved by using build tag https://golang.org/cmd/go/#hdr-Build_constraints

@ninedraft
Copy link
Contributor Author

👍 but is it pointing to branch r.2.3-go of my fork?
It looks like is pointing to another branch

You're right, my fault!

I'll fix it as soon I'l get to my laptop

@ninedraft
Copy link
Contributor Author

done

@galeone
Copy link
Owner

galeone commented Nov 15, 2020

Thank you @ninedraft 👍

Merging it right now.

About the build tags it looks like there isn't the support to specify the Go compiler, looking at the doc you linked. I'll give another look in the future (as soon as I have time) - but in the meantime, thank you for letting me know about this feature!

@galeone galeone merged commit eabbb5b into galeone:master Nov 15, 2020
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.

None yet

3 participants