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

archive/tar: Example_sparseAutomatic and TestSparseEntries fails on iOS #21970

Closed
dsnet opened this issue Sep 21, 2017 · 7 comments
Closed

archive/tar: Example_sparseAutomatic and TestSparseEntries fails on iOS #21970

dsnet opened this issue Sep 21, 2017 · 7 comments
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile OS-Darwin
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Sep 21, 2017

The iOS builders are currently failing with:

(lldb) --- FAIL: Example_sparseAutomatic (0.52s)
got:
Src MD5: 33820d648d42cb3da2515da229149f74
Dst MD5: 311175294563b07db7ea80dee2e5b3c6

want:
Src MD5: 33820d648d42cb3da2515da229149f74
Dst MD5: 33820d648d42cb3da2515da229149f74

FAIL

This test was added when implementing support for sparse files hole detection and punching. On OS+FS that do not support sparse files (such as iOS), Example_sparseAutomatic should still produce a non-sparse file, whose content matches the source file. However, the test is indicating that the contents differ for unknown reasons.

@dsnet
Copy link
Member Author

dsnet commented Sep 21, 2017

@eliasnaur

I'm trying to debug this issue, but I'm not sure how invoke the failing tests manually on iOS using gomote.

I tried doing the following:

$ gomote create darwin-arm64-a1457ios
user-joetsai-darwin-arm64-a1457ios-0

$ gomote push user-joetsai-darwin-arm64-a1457ios-0 
... copies my go root to the builder ...

$ gomote run user-joetsai-darwin-arm64-a1457ios-0 go/src/iostest.bash -restart
...
##### Testing packages.
lldb setup error: exited (lldb start: exit status 253)
start timeout, trying again
lldb setup error: exited (lldb start: exit status 253)
start timeout, trying again
lldb setup error: exited (lldb start: exit status 253)
start timeout, trying again
lldb setup error: exited (lldb start: exit status 253)
start timeout, trying again
lldb setup error: exited (lldb start: exit status 253)
go_darwin_arm_exec: failed to start test harness (retry attempted)
FAIL	archive/tar	12.281s
...

Do you have any ideas?

@eliasnaur
Copy link
Contributor

eliasnaur commented Sep 22, 2017

I couldn't even get gomote as far as you did, so I debugged to issue manually. The problem seems to be that on darwin, the values of SEEK_HOLE and SEEK_DATA are swapped(!):

/usr/include/sys/_types/_seek_set.h:#define	SEEK_HOLE	3	/* set file offset to the start of the next hole greater than or equal to the supplied offset */
/usr/include/sys/_types/_seek_set.h:#define	SEEK_DATA	4	/* set file offset to the start of the next non-hole file region greater than or equal to the supplied offset */

I confirmed that this change fixes the build on darwin/arm64 (but will obviously fail on every other unix):

diff --git a/src/archive/tar/sparse_unix.go b/src/archive/tar/sparse_unix.go
index 4bc3482858..b55821d731 100644
--- a/src/archive/tar/sparse_unix.go
+++ b/src/archive/tar/sparse_unix.go
@@ -19,8 +19,8 @@ func init() {
 func sparseDetectUnix(f *os.File) (sph sparseHoles, err error) {
        // SEEK_DATA and SEEK_HOLE originated from Solaris and support for it
        // has been added to most of the other major Unix systems.
-       const seekData = 3 // SEEK_DATA from unistd.h
-       const seekHole = 4 // SEEK_HOLE from unistd.h
+       const seekData = 4 // SEEK_DATA from unistd.h
+       const seekHole = 3 // SEEK_HOLE from unistd.h
 
        // Check for seekData/seekHole support.
        // Different OS and FS may differ in the exact errno that is returned when

@dsnet
Copy link
Member Author

dsnet commented Sep 22, 2017

You got to be freaking kidding me... Solaris invents SEEK_HOLE and SEEK_DATA and most Unix systems have since adopted the same convention with the same definition and semantics (for the most part). It seems utterly insane for Darwin to decide to change it up.

\cc @rasky, do you know if High Sierra has these same (reversed) definitions of SEEK_HOLE and SEEK_DATA? It seems like a major mistake for Apple to switch these values up. Almost seems like a typo on their part.

@gopherbot
Copy link

Change https://golang.org/cl/65570 mentions this issue: archive/tar: fix sparse files support on Darwin

@rasky
Copy link
Member

rasky commented Sep 22, 2017

Confirmed, they're swapped in High Sierra as well. Example_sparseAutomatic was failing on High Sierra just like iOS. I mailed the fix.

@rasky
Copy link
Member

rasky commented Sep 24, 2017

FWIW, I could only reproduce the Example_sparseAutomatic failure on high sierra. I don't know if the patch also fixes TestSparseEntries on iOS.

@dsnet
Copy link
Member Author

dsnet commented Sep 24, 2017

The iOS builders are passing now, so I think it's safe to say it's fixed. Thanks.

@golang golang locked and limited conversation to collaborators Sep 24, 2018
@rsc rsc unassigned dsnet Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile OS-Darwin
Projects
None yet
Development

No branches or pull requests

4 participants