yglukhov / nimx

GUI library
MIT License
1.1k stars 75 forks source link

nim CI broke because of last commit: Error: cannot open file: layout_vars #411

Closed timotheecour closed 4 years ago

timotheecour commented 4 years ago

/cc @yglukhov

latest commit https://github.com/yglukhov/nimx/commit/f5902ce59f27d1f078894427b670d9a01036947a in nimx seems to have broken nim CI again (eg https://github.com/nim-lang/Nim/pull/13926 + 1 other PR broke, but also a fake one that I did just to double check that it's due to nimx), giving: /home/vsts/.nimble/pkgs/nimx-0.1/nimx/view.nim(2, 42) Error: cannot open file: layout_vars

(in this PR that just tests nimx: https://dev.azure.com/timotheecour/timotheecour/_build/results?buildId=1116&view=logs&j=63665ae6-b751-545e-a6c4-24432959da79&t=9b2d96d9-ecd2-5347-871c-a997abe1fc35)

how to reproduce

git clone https://github.com/yglukhov/nimx
cd nimx
git rev-parse HEAD
f5902ce59f27d1f078894427b670d9a01036947a
# use a fresh dir: rm -rf $HOME/.nimble_fake12
# do what testament does
NIMBLE_DIR=$HOME/.nimble_fake12 nimble install nimx
NIMBLE_DIR=$HOME/.nimble_fake12 nim c -r --threads:on test/main.nim

/Users/timothee/.nimble_fake12/pkgs/nimx-0.1/nimx/view.nim(2, 42) Error: cannot open file: layout_vars import types, context, animation_runner, layout_vars

I'm noticing nimx does not have a git tag, so there's no notion of stable release, which is why things tend to break often.

proposal

yglukhov commented 4 years ago

Sorry, it is already fixed.

timotheecour commented 4 years ago

/cc @yglukhov

yglukhov commented 4 years ago

also somehow after...

Please update kiwi dependency.

timotheecour commented 4 years ago

/cc @yglukhov

Please update kiwi dependency.

ok after that NIMBLE_DIR=$HOME/.nimble_fake13 nim c --threads:on test/main.nim works, thanks!

but the point about having a git tag for every release that's been tested remains.

Did the broken commit pass your CI?

also, compilation works but running doesn't: nim c -r --threads:on test/main.nim 2020-04-11 06:09:46.325 main[41548:123173162] Program linked: WARNING: Output of vertex shader 'vPos' not read by fragment shader

if i click on:

other buttons seem to work though!

yglukhov commented 4 years ago

The crash you're observing might be macos specific. Try running nake instead.

Regarding the tags, unfortunately they (nimble) don't work for my setup. Firstly you're asking to tag not just nimx, but all of its dependencies. While I want to always stick to latest versions of my libs (unless lockfiles are implemented properly). Mixing constrained and unconstrained version requirements in the dependency graph appears to be broken and results in (sometimes even non-deterministic) conflicts on fresh nimble install. Unfortunately I can't provide clear steps to reproduce as they are pretty involved and require to commit specific stuff to nimble-published packages, and it's just not worth my time at this point.

That said I don't want to interfere with anyone by breaking his nim tests, so I don't mind removing nimx from the important packages.

Another (compromise) option could be for me to create a "nim-ci-stable" branch, which I will keep as stable as possible and nim test system will be aware of it. That doesn't cover nimx dependencies, but should reduce the chance of tests breakage significantly.

timotheecour commented 4 years ago

quick question so I understand more what's the best option: did you CI catch the above breakage layout_vars.nim(19, 32) Error: type mismatch: got <Variable> ?

yglukhov commented 4 years ago

No, because kiwi was committed before nimx started to rely on this update.

timotheecour commented 4 years ago

hmm, both nimx and kiwi are under your control so could both use a CI that at least checks for what's being run in testament, eg:

export NIMBLE_DIR=...
nimble install nimx
nim c -r --threads:on test/main.nim

that way, a commit to kiwi that would break nimx wouldn't pass your CI (and therefore wouldn't break nim CI)

could that work?

yglukhov commented 4 years ago

That might reduce the possibility of breakage, yes.