-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
properly fixes #13758 so that import std/macros
stays legal
#14291
Conversation
cebc0ee
to
e1e7dfc
Compare
import std/macros
stays legal
Doesn’t the FreeBSD CI script want the same change, just to make things equal?
…On Sun, 10 May 2020, at 12:05, Timothee Cour wrote:
* properly fixes #13758 <#13758>
* #13762 <#13762> was a bad fix
that fixed some symptoms instead of root cause:
underlying issue remained (eg in #14285
<#14285> I couldn't `import
std/browsers`). The root cause is that `getAppFilename` was buggy on
openBSD and was fixed in devel (since #12390
<#12390> by @euantorano
<https://github.com/euantorano>) but not in csources; so it keeps
bugging during bootstrap unless some precautions are taken (this PR).
`* build nim from csources
* nim c koch
=> it loads config/nim.cfg
=> it processes `path="$lib/pure"`
=> calls loadConfigs => setDefaultLibpath => setDefaultLibpath =>
getAppDir => getAppFilename => getApplHeuristic
=> `getApplHeuristic` is buggy so returns returned wrong results (lib =
"")
=> $lib/pure" is resolved to pure instead of /pathto/Nim/lib/pure
=> import std/macros gives: Error: cannot open file: std/macros
`
note that #12390 <#12390> fixed
(although bugs still remain) `getAppFilename` on openbsd, but the exact
same problem will affect any new platform we add since
`getApplHeuristic` still has all the bugs that were fixed for openbsd
by #12390 <#12390>
CI failures unrelated
(a lot of these are because gitlab is giving `503: We are upgrading our
database! ` see
https://gitlab.com/define-private-public/HTML5-Canvas-Nim and
https://status.gitlab.com/)
timotheecour#113 (comment)
<timotheecour#113 (comment)>
timotheecour#113 (comment)
<timotheecour#113 (comment)>
timotheecour#113 (comment)
<timotheecour#113 (comment)>
links
* refs timotheecour#166
<timotheecour#166>: findExe and
getAppFilename are full of bugs
D20200327T021847
note
sooner or later, we'll have to update csources (which I was enabling
via #13854 <#13854>) or we'll keep
getting bogged down by issues that are fixed in devel (and even in
supported releases) but not fixed in the aging v0.20.0 csources. The
costs of updating csources is small, and much smaller than the cost of
maintaining bootstrap-compatibility with v020.0
You can view, comment on, or merge this pull request online at:
#14291
Commit Summary
* properly fix #12389
File Changes
* *M* .builds/openbsd.yml
<https://github.com/nim-lang/Nim/pull/14291/files#diff-acf16258a1015502d21272836fa90fd7> (2)
* *M* build_all.sh
<https://github.com/nim-lang/Nim/pull/14291/files#diff-4a42809a0e4d80710fa84a9f7ea26bcf> (3)
* *M* compiler/nim.nim
<https://github.com/nim-lang/Nim/pull/14291/files#diff-bd073c215df203e1bb81e25400f0e2af> (2)
* *M* koch.nim
<https://github.com/nim-lang/Nim/pull/14291/files#diff-cbed6f667eb622190ddf224587036ea7> (12)
* *M* lib/pure/typetraits.nim
<https://github.com/nim-lang/Nim/pull/14291/files#diff-c0cc2e46425f05da1b3bcb3aa72dc475> (3)
Patch Links:
* https://github.com/nim-lang/Nim/pull/14291.patch
* https://github.com/nim-lang/Nim/pull/14291.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14291>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFW24PP4ATZ4JDQMJRLEV3RQ2C7TANCNFSM4M5DPTNA>.
|
no need, it wasn't broken for freebsd in v0.20.0 (nor were other platforms tested in CI). if/when csources2 becomes a thing, passing |
Ok, I just saw the change to |
that's because build_all.sh is meant to be usable by all posix (including openbsd), whereas .builds/freebsd.yml is... just for freebsd and doesn't need it. But IMO the clean way to do this is code reuse (even if in bash) by doing it for example like I was doing it in https://github.com/nim-lang/Nim/pull/13854/files#diff-4a42809a0e4d80710fa84a9f7ea26bcf so that each CI can just call a shell function instead of inlining the code; it's not just about "nim c koch" it's about all the rest of the logic duplicated in all the CI's instead of being handled in just 1 place. That would avoid your concern too. |
e1e7dfc
to
06628ad
Compare
It's not just our CIs that have to be patched. Others cannot bootstrap from csources either with these changes. |
06628ad
to
65da0db
Compare
PTAL, fixed differently |
Seems relevant: d:\a\1\s\lib\core\macros.nim(568, 31) Error: 'untyped' is only allowed in templates and macros |
30f24c9
to
542d116
Compare
542d116
to
9513921
Compare
@Araq green now, finally (it almost turned into an argument in favor of bumping up csources2, but "unfortunately" didn't) |
…nim-lang#14291) * properly fix nim-lang#12389 * use --lib:lib in koch.nim.cfg instead * third time is the charm
…ootstrap + bsd * refs #16282 (comment) * sounds very similar to #14291
…ootstrap + bsd * refs #16282 (comment) * sounds very similar to #14291
…ap + bsd (#17902) * [WIP] bring back std/ prefix within compiler and ensure it works in bootstrap + bsd * refs #16282 (comment) * sounds very similar to #14291 * more: vmops * update tools/ci_generate.nim * auto-generate freebsd.yml as well, to avoid duplication with openbsd.yml * cleanup * undo temporary CI removal
…ap + bsd (nim-lang#17902) * [WIP] bring back std/ prefix within compiler and ensure it works in bootstrap + bsd * refs nim-lang#16282 (comment) * sounds very similar to nim-lang#14291 * more: vmops * update tools/ci_generate.nim * auto-generate freebsd.yml as well, to avoid duplication with openbsd.yml * cleanup * undo temporary CI removal
$PATH
#13758import macros
rather thanimport std/macros
#13762 was a bad fix that fixed some symptoms instead of root cause:underlying issue remained (eg in #14285 I couldn't
import std/browsers
). The root cause isgetAppFilename
bugs on openBSD that are fixed in devel but not in csources; so it keeps bugging during bootstrap unless some precautions are taken (this PR):note that #12390 (thanks @euantorano) fixed
getAppFilename
for openbsd (although it still has bugs even for openbsd), but the exact same problem will affect any new platform we add sincegetApplHeuristic
still has all the bugs that were fixed for openbsd.CI failures unrelated
(a lot of these are because gitlab is giving
503: We are upgrading our database!
see https://gitlab.com/define-private-public/HTML5-Canvas-Nim and https://status.gitlab.com/) timotheecour#113 (comment) timotheecour#113 (comment) timotheecour#113 (comment)note
the bugs get compounded by several factors:
"" / path
returnspath
instead of raising; this is cause of a large number of hard to track bugs and is a design issue that should be fixed (eg doc2 outputs in current work dir #6583, which just got re-opened as I explained in doc2 outputs in current work dir #6583 (comment); or here withconf.libpath = prefix / RelativeDir"lib"
)sooner or later, we'll have to update csources/csources2 (which I was enabling via [superseded] [WIP] fix #11457 csources2 revision is now version-controlled so we can keep updating boostrap nim version #13854) or we'll keep getting bogged down by ghost issues that are fixed in devel (and even in supported releases) but not fixed in the aging v0.20.0 csources. The costs of bumping csources2 is small, and much smaller than the cost of maintaining bootstrap-compatibility with v020.0.
getAppFilename
andfindExe
are still completely broken on devel findExe and getAppFilename are full of bugs #14292D20200327T021847