move -d:nimRawSetjmp to nim.cfg

[narimiran]

Refs https://github.com/status-im/nimbus-build-system/issues/44

[Menduist]

I'm not super convinced by this PR Realistically, we'll have more flags in this file:

-d:chronosStrictException
--styleCheck:usages
--styleCheck:hint

& what already exists in config.nims (which I'm not sure is translatable to cfg anyway) But because of --skipParentCfg, we'll have to duplicate theses flags on nim.cfg and tests/nim.cfg, which is just asking for trouble

imo, we should either get rid of skipParentCfg, or switch back to nims to allow including ../config.nims from tests/config.nims

[narimiran]

I'm not super convinced by this PR

Neither am I :)

But because of --skipParentCfg, we'll have to duplicate theses flags on nim.cfg and tests/nim.cfg, which is just asking for trouble

I agree.

imo, we should either get rid of skipParentCfg, or switch back to nims to allow including ../config.nims from tests/config.nims

There was a discussion on Discord about this whole thing (including a Nim bug this discovered), but the only conclusion was: we need to come up with something more robust, but for now it is not clear what exactly that solution would be.

[narimiran]

It's been more than a month. AFAIK, we still didn't come up with some better solution and/or dealt with --skipParentCfg.

More than a half other libraries (where I've made these changes) have already merged those PRs. Can we merge this?

[Menduist]

Switched to config.nims, let me know what you think

[narimiran]

Switched to config.nims, let me know what you think

config.nims was the original idea, but later it was decided that nim.cfg might be more suitable.

Both are fine by me (i.e. I don't mind your change), but maybe @arnetheduck would like to have this done consistently to all the projects.

[Menduist]

Yeah, we need a config.nims either way (for nimble lock file, the pin mechanism & style check?), so I'd rather have only a config.nims than both nims & cfg

The current version of this PR is the cleanest, imo

[arnetheduck]

config.nims is imo a strategic mistake that will keep creating problems and complexity for tooling, but indeed, since the codebase is already polluted with it, it might be the lesser evil - the proper fix here would be to make nbs compatible with lock files and not introduce the conditional path files which would allow avoiding doing unnecessary work such as instantiating a VM just to parse a config, then address deficiencies in nimble itself to make cfg files extensible (for fringe cases that actually benefit from scriptability) but that's a longer road to walk - cc @zah @yyoncho

[yyoncho]

the proper fix here would be to make nbs compatible with lock files and not introduce the conditional path files

@arnetheduck check https://github.com/status-im/nimbus-build-system/pull/48 (Just in case you haven't seen it). I might be missing a context, but why making NBS compatible with lock files given the fact that nimble is already a tool supporting locking files?

[arnetheduck]

I might be missing a context, but why making NBS compatible with lock files given the fact that nimble is already a tool supporting locking files?

2 reasons:

• we'll want a "slow" transition from nbs to nimble, gradually replacing its functionality (cross-compiling, integration with complex C libraries like libbacktrace and build systems like autoconf/cmake etc) and these will need significant work - in an ideal world, "users" of nbs could continue using make during a transition period and it would call nimble instead of doing whatever it's doing now, with equivalent functionality (such as parallelizing multiple builds with LTO, managing flags and options correctly, understanding dependencies to do minimal rebuilds etc) as it gains capabilities
• we want to get rid of the ugly hack where we check for a paths file and conditionally add nonimblepath - tooling should "understand" the structure and basic configuration of a clean checkout of a repo and its dependencies and configuration without having to execute any code / scripts like config.nims, via declarative mechanisms that don't have security issues and can be reasoned about / changed by tooling, and a clean checkout should "just work" for tooling like dependency analysis, nimsuggest etc. the paths file itself is structurally a compromise and a problem because it introduces an additional source of truth as well, which needs to be kept in sync - eventually, we'll probably want to see if there is any way to get rid of it, but that requires redesigning nim and nimble to work better together without the overlapping responsibilities that they today have (like noNimblePath - the compiler should not have to "know" about nimble ideally)

[yyoncho]

@arnetheduck seems like we can call nimble to do that lock file integration. A bonus will be that once https://github.com/nim-lang/nimble/pull/1017 is in we might let it manage nim as well.

[zah]

we'll want a "slow" transition from nbs to nimble, gradually replacing its functionality (cross-compiling, integration with complex C libraries like libbacktrace and build systems like autoconf/cmake etc) and these will need significant work - in an ideal world, "users" of nbs could continue using make during a transition period and it would call nimble instead of doing whatever it's doing now, with equivalent functionality (such as parallelizing multiple builds with LTO, managing flags and options correctly, understanding dependencies to do minimal rebuilds etc) as it gains capabilities

All of the libraries that require external build tools (e.g. libbacktrace) already have a Nimble-compatible setup. This was done long time ago by necessity because the CI builds of all libraries are relying on nimble to install their dependencies.

The users are free to make a full switch to the new environment. The transition period requires the use of some additional tools that are used to translate the changes that happened in one of the environments to the pinning mechanism used in the other. See here for more details:

https://github.com/status-im/nim-workspace#sync-vendor-revisions-to-workspace https://github.com/status-im/nim-workspace#sync-workspace-revisions-to-vendor

Ideally, it would be the developer doing the dependency bump who translates the change in both formats, but this is unlikely to happen in practice, so anyone can update an out-of-sync lockfile (or out of sync vendor dir in the future).

[zah]

eventually, we'll probably want to see if there is any way to get rid of it, but that requires redesigning nim and nimble to work better together without the overlapping responsibilities that they today have (like noNimblePath - the compiler should not have to "know" about nimble ideally)

There are contradicting statements here. The problems you outlined can be solved by teaching Nim about lockfiles and how to locate package paths. The current design is the best we can do following the philosophy "the compiler should not know anything about nimble".

[arnetheduck]

The problems you outlined can be solved by teaching Nim about lockfiles and how to locate package paths.

the idea would be that --noNimblePath is the default (in a distant future), ie have it know less about nimble - from there, there are a few design choices to make, which come with tradeoffs (off the top of my head):

• pass everything on command line: you pass a --path-file:my-generated-file.paths
  • this is the cleanest option because it is explicit and thus easy to understand, reason about and troubleshoot
  • this flag can be added to nim.cfg if wanted - as long as nim.cfg is declarative, it's trivial for tooling to reason about this as well, and avoids cyclic dependencies between the nim VM and its own configuration
• make nim always read a "well-known" path file (nim.paths), similar to how it reads nim.cfg and have nimble generate this from the lock file
  • this is a more magic solution, because of how it adds yet another implicit location that affects the build outcome
  • still, if indeed it's called nim.paths, maybe it's simple enough compared to nim.cfg etc
• make nim understand lock files themselves
  • this adds a lot of knowledge to nim that would have to be repeated in tooling that is irrelevant for the problems that these tools need, making it harder to implement and troubleshoot - at this point, a bigger redesign is in order

regardless of which solution is chosen, there's a lot of spaghetti in the current setup, mainly revolving around dependency tracking, flags, caches, what to rebuild, what is an application and so on - both nim and nimble for example have the notion of a "project" but the way they view it is not the same - nim bases it on whatever file is supplied to the command line, with some really weird magic to work backwards from there to a "project folder" and later (for example in its import pkg/ handling - in nimble, it's more clear-cut thanks to the explicit .nimble file declaring things, but it only loosely overlaps with what nim thinks - there is no first-class support for packages in Nim, so work on both nim (specifically its module and package system) is needed in addition to nimble work to make these things "smooth".

[zah]

The proposed ideas bring only minor benefits IMO. You make nimble incompatible with older versions of Nim, just so you can stop using nims files (which shouldn't be considered a problem in the first place).

The user would still face the problem that before the paths file is generated (or if it's outdated), nim and nimsuggest won't work as expected. For me, this is the interesting problem that we could try to solve in the future.

[arnetheduck]

The user would still face the problem that before the paths file is generated (or if it's outdated), nim and nimsuggest won't work as expected. For me, this is the interesting problem that we could try to solve in the future.

The current situation is that nim and nimsuggest appear to work iff the user has the right junk in their nimble folder - this appearance of working is the first issue to solve because it is a hard problem to diagnose. Once --noNimblePath is the default, it is easy to diagnose what's wrong with a setup ("did you run nimble setup?") and provding a good UX becomes trivial as a consequence.

config.nims remains a point of contention, but more than that, it introduces unwanted complexity, which in turn leads to poor UX: because tooling cannot reason about its contents (only the outcome of executing it) and because of this, tooling will lose important capabilities (a magic editable section in the script is not a solution: it's a hack to get to an 80% solution). This complexity results in the user having to troubleshoot their setup more than is needed based on what we know about the problem: it's a rube-goldberg machine for the problem of setting up paths (there is no need for flexibility here: it's not like if exists("nimble.paths") is a desired point of configuratbility).

re backwards compatibility, it's questionable whether it's worth maintaining - the global nimble path system is broken by design, and it's easy to teach a transition - ie removing it would create easy-to-diagnose problems that the users can solve.

The benefit is that a significant source of confusion is removed from the build in general, which makes the problem an order of magnitude more tractable over time. If need be, this can be the knowledge about lock files that nim has: if a lock file is present, all "old" stuff relating to nimble gets disabled, and it ends there.

[Menduist]

it might be the lesser evil

Is this an approval :) ?

[arnetheduck]

Is this an approval :) ?

it's not a disapproval - just like I wouldn't approve of anyone jumping off a cliff without appropriate gear, but can't stop those really determined to do so

[Menduist]

Again, the missing feature from .cfg is the ability to import the root config from the tests one

The way, I see it, we have a few ways forward:

• Merge as is (--skipParentCfg + nims because we need import)
• Remove --skipParentCfg, switch to .cfg (can cause issues when building from NBS)
• Keep --skipParentCfg, switch to .cfg, duplicate the file in the tests folder (imo the worst option)
• Merge as is, open RFC to add import capability to .cfg, and switch to it once that's done