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

x/sys/unix: TestPassFD consistently failing in aix-ppc64 builder on release-branch.go1.12 #33982

Closed
bcmills opened this issue Aug 30, 2019 · 12 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-AIX Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 30, 2019

golang.org/x/sys/unix is failing in aix-ppc64 on release-branch.go1.12 (https://build.golang.org/log/2ceea003d216da8b55d107004541d477ada9c89f):

--- FAIL: TestPassFD (0.75s)
    syscall_unix_test.go:224: child process: "WriteMsgUnix: write unix : sendmsg: bad file number", <nil>
FAIL
FAIL	golang.org/x/sys/unix	1.041s

The same failure does not occur on release-branch.go1.13. Is there a fix we should backport, or should we skip the test on AIX on 1.12?

CC @Helflym @trex58 @laboger

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-AIX labels Aug 30, 2019
@bcmills bcmills added this to the Go1.12.10 milestone Aug 30, 2019
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Aug 30, 2019
@Helflym
Copy link
Contributor

Helflym commented Sep 2, 2019

https://golang.org/cl/170537 should fix it.

@bcmills
Copy link
Contributor Author

bcmills commented Sep 3, 2019

@Helflym, as far as I can tell that CL is on the master branch (that is, Go 1.14), but the test failure was only observed on release-branch.go1.12. Is there something that can/should be backported to fix the builder?

@Helflym
Copy link
Contributor

Helflym commented Sep 4, 2019

Why 1.14 ? Yes, it has been merged in master branch but before 1.13 freeze. That's why golang.org/x/sys/unix is green for release-branch.go1.13 but fails for release-branch.go1.12. Or I'm missing something...
To explain a bit further, the test is failing because in 1.12, we are using recvmsg and sendmsg which aren't compatible with Control and Controllen structures.

@bcmills
Copy link
Contributor Author

bcmills commented Sep 4, 2019

Sorry, my mistake — for some reason I thought that CL hadn't been merged yet.

If it is expected that the golang.org/x/sys/unix test fails on AIX before 1.13, then the test should call t.Skip if runtime.GOOS == "aix" on builds that lack the go1.13 build tag.

@Helflym
Copy link
Contributor

Helflym commented Sep 4, 2019

Is it possible to check the go1.13 build tag inside an if or the whole file must be disabled for 1.12 ?

@bcmills
Copy link
Contributor Author

bcmills commented Sep 4, 2019

I'm not sure, but you can always add a separate file that sets a variable:

// […]

// +build aix,!go1.13

package unix

func init() {
	aixHasSendmsgBug = true
}

@gopherbot
Copy link

Change https://golang.org/cl/193577 mentions this issue: unix: skip TestPassFD on AIX before 1.13

@tklauser
Copy link
Member

tklauser commented Sep 5, 2019

Couldn't we just backport https://golang.org/cl/170537 (maybe sans the SockaddrDatalink addition) to release-branch.go1.12 to fix the issue instead of just skipping the test? Or is that not worth the effort?

@Helflym
Copy link
Contributor

Helflym commented Sep 5, 2019

I think it's also better to backport it. Syscall package is also using Control and Controllen structures, so nrecvmsg and nsendmsg should be used.

@ianlancetaylor
Copy link
Contributor

Sure, let's backport it.

@gopherbot
Copy link

Change https://golang.org/cl/193608 mentions this issue: [release-branch.go1.12] syscall: on AIX use nsendmsg and nrecvmsg, define SockaddrDatalink

@ianlancetaylor ianlancetaylor added the CherryPickApproved Used during the release process for point releases label Sep 6, 2019
gopherbot pushed a commit that referenced this issue Sep 6, 2019
…fine SockaddrDatalink

This commit changes sendmsg, recvmsg to use nsendmsg, nrecvmsg on AIX.
These syscalls support the new msghdr structure (with Control
and Controllen) which is needed for golang.org/x/net.
Also define SockaddrDataLink.

Fixes #33982

Change-Id: I233fbd24f9eb86648e0d4d50c2b56da3626292d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
(cherry picked from commit e014184)
Reviewed-on: https://go-review.googlesource.com/c/go/+/193608
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

Closed by merging cf06b9a to release-branch.go1.12.

@FiloSottile FiloSottile modified the milestones: Go1.12.10, Go1.12.11 Sep 25, 2019
@katiehockman katiehockman modified the milestones: Go1.12.11, Go1.12.12 Oct 17, 2019
@golang golang locked and limited conversation to collaborators Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-AIX Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

7 participants