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

pkg/netutil: use native byte ordering #7199

Merged
merged 2 commits into from
Jan 23, 2017

Conversation

heyitsanthony
Copy link
Contributor

Tests were failing on non-386/amd64 archs since the build flags were trying to enforce little endian ordering. Just detect endianness instead and have the route detection code work everywhere.

/cc @mkumatag

A package for unsafe cpu-ish things.
@mkumatag
Copy link
Contributor

I see a error in travis :

# github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd/pkg/netutil gopath/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd/pkg/netutil/routes_linux.go:31: GetDefaultHost redeclared in this block previous declaration at gopath/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd/pkg/netutil/routes.go:26 gopath/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd/pkg/netutil/routes_linux.go:124: GetDefaultInterface redeclared in this block previous declaration at gopath/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd/pkg/netutil/routes.go:31

https://github.com/coreos/etcd/blob/master/pkg/netutil/routes.go#L15 contains the fallback functions for unsupported platform, we might need to add architectures here to get away with re-declaration error.

line should be modified to :
+build !linux !386,!amd64,!arm,!arm64,!ppc64le

@mkumatag
Copy link
Contributor

mkumatag commented Jan 20, 2017

Applied following change on top of this PR and able to run test successfully

ubuntu@ubuntu:~/heyitsanthony/etcd$ git diff
diff --git a/pkg/netutil/routes.go b/pkg/netutil/routes.go
index d6979c1..4cb0a24 100644
--- a/pkg/netutil/routes.go
+++ b/pkg/netutil/routes.go
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// +build !linux !386,!amd64
+// +build !linux !386,!amd64,!arm,!arm64,!ppc64le
 
 package netutil
 
ubuntu@ubuntu:~/heyitsanthony/etcd$ 

@gyuho
Copy link
Contributor

gyuho commented Jan 20, 2017

LGTM after test fixes

@heyitsanthony
Copy link
Contributor Author

ci failures look runtime related (httptest); merging

@heyitsanthony heyitsanthony merged commit 5aab924 into etcd-io:master Jan 23, 2017
@heyitsanthony heyitsanthony deleted the netutil-test-arch branch January 23, 2017 17:41
gyuho pushed a commit that referenced this pull request Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants