Fix #27

[juancarlospaco]

#27 (comment)

[alaviss]

Please remove "adding strictFuncs + styleCheck:error" from this PR. It's not relevant to the issue being addressed.

Also please setup a binary cache for choosenim.

Other than that, looks fine to me.

FYI @Araq, this change will make use of choosenim instead of the current build-from-source system.

[juancarlospaco]

styleCheck literally found errors on the code, dont you want a more correct code... 😞

[alaviss]

styleCheck literally found errors on the code, dont you want a more correct code... 😞

I don't mind it, but it has no relevance to the objective of this PR, and can be done without the nim action change, so please split it into an another PR.

[timotheecour]

/cc @jiro4989 please advise regarding correct usage of jiro4989/setup-nim-action regarding caching in this PR!

My understanding is:

• we need also the snippet mentioned in https://github.com/jiro4989/setup-nim-action#devel---latest-usage
• we can disregard this note:

Please should not use Cache nimble on windows-latest. setup-nim-action may Failing to install on windows-latest.

since we're using builder: windows-2019, not windows-latest

Please remove "adding strictFuncs + styleCheck:error" from this PR. It's not relevant to the issue being addressed.

furthermore, strictFuncs still is very experimental, and it's not at all clear how they can be fixed, see nim-lang/Nim#16305 and https://github.com/nim-lang/Nim/issues?q=is%3Aissue+is%3Aopen+label%3A%22view+types%22+sort%3Aupdated-desc

nimble --experimental:strictFuncs --styleCheck:error test

the problem is this doesn't propagate through exec calls (nim calling nim); instead (in another PR), add tests/config.nims with flags there; it's more flexible (thanks to nimscript) and does propagate through exec calls

[jiro4989]

@timotheecour

My understanding is:

we need also the snippet mentioned in https://github.com/jiro4989/setup-nim-action#devel---latest-usage
we can disregard this note:

Yes.

And setup-nim-action can use Cache nimble on windows-latest and windows-2019 now.
I tried those.

• 1 (non cached yet) https://github.com/jiro4989/setup-nim-action/actions/runs/431744661
• 2 (cached) https://github.com/jiro4989/setup-nim-action/actions/runs/431783555
  • test_devel_latest jobs were succeeded.

choosenim had returned error when nimble had been cached on windows virtual machine. (setup-nim-action is simple wrapper GitHub Actions of choosenim. )

But current choosenim has not this issues.

I will fix this note.

Please should not use Cache nimble on windows-latest. setup-nim-action may Failing to install on windows-latest.

[timotheecour]

LGTM but this could take an extra pair of eyes regarding correctness of caching /cc @jiro4989 @xflywind

(eg given differences with https://github.com/jiro4989/setup-nim-action#devel---latest-usage, etc)

[juancarlospaco]

@alaviss merge?, close?.

[timotheecour]

@xflywind should we merge this ? worst case, we can always revert

[alaviss]

Putting the cache at the end seems completely useless to me, since they're only restored when this step run, which at this point everything else has already finished running.

[timotheecour]

• @alaviss can you make a followup PR with your proposed change?
• and is the example in https://github.com/jiro4989/setup-nim-action#devel---latest-usage also wrong?

[juancarlospaco]

I tried moving it at the top, it works, but also becomes kinda flaky:
https://github.com/nim-lang/fusion/runs/1640491026#step:10:51 Nim_and_C_compiler_disagree_on_target_architecture
It is faster than before even without cache. 🤷

[alaviss]

@timotheecour It is correct, but as you can see, cache come before the call to setup nim.

@juancarlospaco The cache key is wrong. It didn't factor in that:

• Multiple branches are employed
• Multiple OS architectures are employed

[alaviss]

Also, the cache key doesn't factor in changes in nightlies version between days, which means caching would only be performed the first time and not on subsequent updates, removing all benefits of the cache.

It's probably better to just get rid of it.

[timotheecour]

@jiro4989 I think the snippet you use in https://github.com/jiro4989/setup-nim-action#devel---latest-usage is likely to be copy pasted and misused as demonstrated here; I think it should come with a good explanation on how to use it properly, its caveats etc

[jiro4989]

Thanks. I wrote it.

jiro4989/setup-nim-action@4c32cad