Skip to content

Conversation

@dkegel-fastly
Copy link
Contributor

@dkegel-fastly dkegel-fastly commented Mar 19, 2022

This is #2515 rebased against current dev, with a few new commits fixing missing signal values, adding a missing stub or two, and crucially, fixing #2726 , now with Damian's magic hack when goroot is 1.18 or above.

The testdata change in this branch differs from qulogic's slightly for no good reason.

(Includes @QuLogic's stub fuzzer to avoid #2616;
one of the commits in this branch addresses an other little problem that come up along the way to that failure.)

@dgryski
Copy link
Member

dgryski commented Apr 1, 2022

This PR passes the test corpus, which is a nice vote of confidence.

@dgryski
Copy link
Member

dgryski commented Apr 2, 2022

Just realized my interface{} -> any rewrite patch should check that we're running 1.18 or above before doing the replacement.

@dkegel-fastly dkegel-fastly force-pushed the dkegel-go118 branch 2 times, most recently from 203b01f to 8698c31 Compare April 3, 2022 22:06
@dkegel-fastly
Copy link
Contributor Author

OK, the interface{} -> any rewrite patch is now versioned as you suggested.

@aykevl
Copy link
Member

aykevl commented Apr 6, 2022

IMHO it's not necessary to keep testing Go 1.17 on Windows or MacOS, unless you think there is a risk of regression that isn't caught by existing Linux/Go1.17 tests.

@dkegel-fastly
Copy link
Contributor Author

Yeah, I only started doing the 1.17 builds again when I saw 1.18 failed on windows, just wanted to see if 1.17 still worked there (it does). I was going to remove it once 1.18 works on windows.

@deadprogram
Copy link
Member

@dkegel-fastly can you please resolve the merge conflict and rebase against dev so we can see what is still left to do.

@deadprogram
Copy link
Member

So looks like just Windows 1.18 not passing, if I am reading the output correctly.

@dkegel-fastly
Copy link
Contributor Author

Yup.

Tests fail on windows with 1.18.
It seems the compiler crashes when building compress/flate and index/suffixarray?
Error message may vary, latest is

runtime: VirtualAlloc of 802816 bytes failed with errno=1455
fatal error: out of memory

https://github.com/tinygo-org/tinygo/runs/5871904692?check_suite_focus=true

QuLogic and others added 2 commits April 8, 2022 08:18
This simply shadows the real code temporarily to see what else is
broken. It only defines a single type to fix testing/internal/testdeps.
@dkegel-fastly
Copy link
Contributor Author

Because windows builds are failing with go 1.18, but people want to start using go 1.18, I removed
the CI changes; when this lands, motivated early adopters will be able to build dev with go 1.18
and help find problems. I can open another PR for the CI changes...

@dkegel-fastly dkegel-fastly marked this pull request as ready for review April 8, 2022 16:00
@deadprogram
Copy link
Member

Seems like a practical approach to me. @aykevl any comment before merge?

@deadprogram
Copy link
Member

Let's goooooooooooooooooooo! Merging, thank you @dkegel-fastly

@deadprogram deadprogram merged commit 8dfb317 into tinygo-org:dev Apr 9, 2022
@aykevl
Copy link
Member

aykevl commented Apr 9, 2022

A bit late, but I think it's fine to merge. However, I do think we should at least try to fix the Windows issue before release or a lot of people will be very confused.

@deadprogram
Copy link
Member

Agreed for sure we need to fix it. I think @dkegel-fastly will now create a PR just for the CI changes so we can make sure all platforms are working. This PR was just to unblock people on other platforms from making a build from dev and having it work for their testing purposes.

@aykevl
Copy link
Member

aykevl commented Apr 9, 2022

Hmm, GOOS=windows tinygo test compress/flate works on my Linux system (with wine). That's unfortunate. Looks like it needs an actual Windows system.

@dkegel-fastly
Copy link
Contributor Author

Yup. Doesn't reproduce on wine here, either.

@dkegel-fastly dkegel-fastly deleted the dkegel-go118 branch April 10, 2022 01:56
@dkegel-fastly
Copy link
Contributor Author

The abort is in the compiler, and would presumably show up if you could run the windows version of go in wine, but wine can't quite handle that yet.

@dkegel-fastly
Copy link
Contributor Author

I just tried running the compiler under wine, roughly:

$ wine64 msiexec /i go1.18.windows-amd64.msi
$ cd ~/.wine/drive_c
$ mkdir -p users/dkegel/go
$ export GOPATH='c:\users\dkegel\go'
$ cd "Program Files"/go/bin
$ wine64 go.exe test testing
...
0134:fixme:process:SetProcessPriorityBoost (FFFFFFFFFFFFFFFF,1): stub
ok      testing 7.645s

$ wine64 go.exe test compress/flate
...
write C:\users\dkegel\Temp\go-build1476500739\b023\vet.cfg: Invalid handle.
...
--- PASS: Example_synchronization (0.00s)
PASS
ok      compress/flate  6.089s
FAIL
04f4:fixme:file:NtLockFile I/O completion on lock not implemented yet
$ wine64 --version
wine-6.0.2

so while there is some problem on exit in wine, it's not the problem we're looking for, and you probably do need to run on real windows.

When running go.exe to build or test something, Mac OS 11.6.5 puts up a dialog saying delete executable or cancel about 20 times; click cancel each time to allow running a downloaded executable. (Presumably there is some way to bless go.exe and its tools, but I haven't checked.)

@dkegel-fastly
Copy link
Contributor Author

See #2762 for the bug.

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.

5 participants