status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/
Apache License 2.0
361 stars 51 forks source link

nim >1.0 introduction of `csize_t` breaks chronos #74

Closed timotheecour closed 4 years ago

timotheecour commented 4 years ago

FYI from this experiment: https://github.com/nim-lang/Nim/pull/13348

this is 1 of the only 2 packages that fails on nim CI (the other one is https://github.com/trustable-code/NiGui)

see https://dev.azure.com/nim-lang/255dfe86-e590-40bb-a8a2-3c0295ebdeb1/_apis/build/builds/2412/logs/82 https://dev.azure.com/nim-lang/255dfe86-e590-40bb-a8a2-3c0295ebdeb1/_apis/build/builds/2412/logs/78

2020-02-07T03:21:47.6984412Z PASS: https://github.com/treeform/chroma C                         ( 4.54586840 secs)
2020-02-07T03:21:47.6984758Z PASS: https://github.com/status-im/nim-chronicles C                (14.30997872 secs)
2020-02-07T03:21:47.6984982Z FAIL: https://github.com/status-im/nim-chronos C
2020-02-07T03:21:47.6985395Z Test "https://github.com/status-im/nim-chronos" in category "nimble-packages"
2020-02-07T03:21:47.6985569Z Failure: reBuildFailed
2020-02-07T03:21:47.6985643Z package test failed
2020-02-07T03:21:47.6985688Z $ nimble test
2020-02-07T03:21:47.6985808Z   Executing task test in /home/vsts/work/1/s/pkgstemp/chronos/chronos.nimble
2020-02-07T03:21:47.6985873Z 
2020-02-07T03:21:47.6986111Z nim c -r -d:useSysAssert -d:useGcAssert tests/testall

...

2020-02-07T03:21:47.6990901Z Hint: osnet [Processing]
2020-02-07T03:21:47.6990986Z /home/vsts/work/1/s/pkgstemp/chronos/chronos/transports/osnet.nim(636, 24) Warning: use `csize_t` instead; csize is deprecated [Deprecated]
2020-02-07T03:21:47.6991305Z /home/vsts/work/1/s/pkgstemp/chronos/chronos/transports/osnet.nim(636, 19) Error: type mismatch: got <csize> but expected 'csize_t = uint'
2020-02-07T03:21:47.6991397Z stack trace: (most recent call last)
2020-02-07T03:21:47.6991465Z /tmp/nimblecache/nimscriptapi.nim(165, 16)
2020-02-07T03:21:47.6991522Z /home/vsts/work/1/s/pkgstemp/chronos/chronos_18881.nims(25, 8) testTask
2020-02-07T03:21:47.6991600Z /home/vsts/work/1/s/lib/system/nimscript.nim(252, 7) exec
2020-02-07T03:21:47.6991886Z /home/vsts/work/1/s/lib/system/nimscript.nim(252, 7) Error: unhandled exception: FAILED: nim c -r -d:useSysAssert -d:useGcAssert tests/testall [OSError]
2020-02-07T03:21:47.6992136Z        Tip: 1 messages have been suppressed, use --verbose to show them.
2020-02-07T03:21:47.6992201Z      Error: Exception raised during nimble script execution
2020-02-07T03:21:47.6992237Z 
2020-02-07T03:31:11.1880921Z PASS: https://github.com/c-blake/cligen.git C                      ( 2.71678019 secs)

...
2020-02-07T03:34:38.7368188Z FAILURE! total: 82 passed: 81 skipped: 0 failed: 1

and this: https://dev.azure.com/nim-lang/255dfe86-e590-40bb-a8a2-3c0295ebdeb1/_apis/build/builds/2412/logs/78


2020-02-07T03:21:33.3882250Z     Unhandled exception: Timeout exceeded!
2020-02-07T03:21:33.3882310Z Async traceback:
2020-02-07T03:21:33.3882390Z   /Users/runner/runners/2.164.7/work/1/s/lib/pure/unittest.nim(647)                      testdatagram
2020-02-07T03:21:33.3882480Z   /Users/runner/runners/2.164.7/work/1/s/pkgstemp/chronos/chronos/asyncloop.nim(946)     waitFor
2020-02-07T03:21:33.3882570Z   /Users/runner/runners/2.164.7/work/1/s/pkgstemp/chronos/chronos/asyncloop.nim(278)     poll
2020-02-07T03:21:33.3882660Z   /Users/runner/runners/2.164.7/work/1/s/pkgstemp/chronos/chronos/asyncmacro2.nim(275)   testBroadcast
2020-02-07T03:21:33.3882760Z   /Users/runner/runners/2.164.7/work/1/s/pkgstemp/chronos/chronos/asyncfutures2.nim(407) internalCheckComplete
2020-02-07T03:21:33.3882850Z Exception message: Timeout exceeded!
2020-02-07T03:21:33.3883530Z Exception type: [AsyncTimeoutError]
2020-02-07T03:21:33.3883700Z   [FAILED] Broadcast test
2020-02-07T03:21:33.3883820Z     /Users/runner/runners/2.164.7/work/1/s/pkgstemp/chronos/tests/testdatagram.nim(509, 54): Check failed: getTracker("datagram.transport").isLeaked() == false
2020-02-07T03:21:33.3883910Z     getTracker("datagram.transport").isLeaked() was true
2020-02-07T03:21:33.3883990Z     false was false
2020-02-07T03:21:33.3884140Z   [FAILED] Transports leak test```
arnetheduck commented 4 years ago

FWIW, this is because of a backwards-incompatible change in Nim (csize -> csize_t).

"1 of the only 2 packages" is a red herring in this case, every such breaking change will make an unknown amount of packages fail and sometimes they'll be covered by the nim ci - at best, it's misleading.

timotheecour commented 4 years ago

well that only proves the usefulness of nim-lang/Nim#13348 ; you have to start somewhere; definitely better than not knowing anything about what breaks

cheatfate commented 4 years ago

We are using officially released Nim 1.0.6 and all tests are fine.

Araq commented 4 years ago

@cheatfate IMO you can simply close this for now, I'll investigate why csize_t breaks your code.

arnetheduck commented 4 years ago

@Araq it's because we use IOVec from posix.nim where the type of one of the fields was changed from csize to csize_t, while in chronos we try to assign a csize to it..