Nim icon indicating copy to clipboard operation
Nim copied to clipboard

`--moduleoverride:mod1:mod2:prefix`: generalized `patchFile` to allow non-global effect; works on cmdline / config

Open timotheecour opened this issue 2 years ago • 6 comments

This PR improves and generalizes (and in fact subsumes) patchFile as follows:

  • --moduleoverride can be used in cmdline (and therefore, in user config too)
  • it uses the canonical module names introduced in https://github.com/nim-lang/Nim/pull/16999 to display module names in docs in an un-ambiguous way (also used in https://github.com/nim-lang/Nim/pull/17746 in processing:canonical)

the most important aspect of --moduleoverride though is that it allows applying the module override in a non-global way, using module prefixes, eg:

--moduleoverride:std/sequtils:std/sequtilsv0 # applies globally
--moduleoverride:std/sequtils:std/sequtilsv0:pkg/mypkg  # override only applies within pkg/mypkg
--moduleoverride:std/sequtils:std/sequtilsv0:pkg/mypkg/sub # only within pkg/mypkg/sub

prefix is an optional comma delimited set of prefixes, eg: std/strutils,pkg/mypkg,pkg/cligen/bar

it can also contain absolute paths (but not relative paths; a std/ or system/ or pkg/ prefix with a canonical name must be used if the path is not absolute)

example use case 1: breaking change mitigation

this can be used to override a module for just a package, without affecting the behavior of other packages. For eg, when upgrading nim to a new version, if some package foo requires a pre-existing behavior in some stdlib module (eg std/sequtils), you can override std/sequtils as needed and make the override apply only within package foo, unlike a flag like -d:nimLegacyFoo which has global effect:

nim --moduleoverride:std/sequtils:pkg/foo/sequtils_override:pkg/foo /pathto/main.nim

example use case 2: custom behavior for a dependency

this is useful for experimentation or if you want to change behavior of a 3rd party package foo in a way that doesn't affect other dependencies that also depend on foo.

note

the overridden module can co-exist with the non-overridden module within a project, and this ends up being 2 separate modules (PSym).

use --processing:filenames to see what modules are being imported (shows import stacks since https://github.com/nim-lang/Nim/pull/18372)

note: rationale for canonical modules names

the way patchFile refers to module names has inherent ambiguities, eg:

  • duplicate module names within a package eg foo/bar/baz vs foo/baz are confused, both as foo_baz
  • foo_bar/baz vs foo/bar_baz are confused, both as foo_bar_baz

furthermore, it doesn't correspond to how you'd import a module.

future work

  • [ ] --symoverride:mod1.sym:sym2:prefix: variant which allows to override a fully qualified symbol mod1.sym by mod1.sym2 within the context of modules matching prefix (eg system.delete => system.deleteV0 to get back old behavior within the context of some pkg/foo only). --moduleoverride is more general (because you can override a whole module as you need) but --symoverride can be easier to use in some cases especially in cases where a behavior for an API was changed and you want the old behavior for a package without affecting other packages.

timotheecour avatar Jul 15 '21 08:07 timotheecour

Interesting, it always looks strange that you can do this for NimScript but not for other targets. 🙂:+1:

juancarlospaco avatar Jul 15 '21 11:07 juancarlospaco

I think the needs this mechanism is addressing should be discussed in an RFC first. The compiler command line is already quite complicated, and I think some additional effort should be made to ensure that the amount of any additional complexity, if it is needed, is as small as possible.

Varriount avatar Jul 16 '21 08:07 Varriount

I think the needs this mechanism is addressing should be discussed in an RFC first.

=> filed https://github.com/nim-lang/RFCs/issues/400

The compiler command line is already quite complicated, and I think some additional effort should be made to ensure that the amount of any additional complexity, if it is needed, is as small as possible.

simple things should be simple, complex things should be possible.

--moduleoverride:mod1:mod2:prefix is not something you'd need nor encounter as a beginner, but when the need for it comes, you will find it useful (I certainly found patchFile useful). I can't think of a more succint way to describe the override than with the proposed flag.

timotheecour avatar Jul 16 '21 19:07 timotheecour

simple things should be simple, complex things should be possible.

Complexity breeds complexity.

Araq avatar Jul 20 '21 19:07 Araq

example use case 1: breaking change mitigation

I don't see this being a use case really - ie the onus of maintaining compatibility is on the std lib in the case of Nim itself - other depenedencies that can be independently upgraded don't need this kind of facility generally

arnetheduck avatar Aug 09 '21 08:08 arnetheduck

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

stale[bot] avatar Aug 10 '22 04:08 stale[bot]