Skip to content
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

RFC: --path:prefix:path to resolve import prefix/suffix #291

Open
timotheecour opened this issue Nov 25, 2020 · 4 comments
Open

RFC: --path:prefix:path to resolve import prefix/suffix #291

timotheecour opened this issue Nov 25, 2020 · 4 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Nov 25, 2020

this RFC improves on --path:path as follows:

import cligen # ok, works
import examples/cols # bug: this is exposed to everyone that can `import cligen`
  • (EDIT) enables Frictionless Nimble directory structures nimble#653 (Frictionless Nimble directory structures) in a sane way that doesn't pollute scope / break namespacing
  • makes it easy (for debugging, customization or other purposes) to selectively override either a specific module or package / sub-package etc.

proposal

  • --path:path is extended to support this new syntax: --path:prefix:path, which updates a key-value table prefixes: Table[string,string] mapping prefix to path.

  • when import path is parsed, we now resolve the module as follows:

proc resolveModule(path: string): string =
  # use the --path:prefix:path flags
  for prefix, pathAbs in conf.prefixes:
    if os.isRelativeTo(path, prefix):
      return pathAbs / path.relativePath(prefix)
  # use the --path:path flags
  return resolveModuleClassic(path)

notes (consequences of above rules)

note that --path:prefix:path operates as a key-value table, unlike --path:path, which guarantees that only 1 dir is searched for for a given prefix and avoids inconsistencies. eg:

nim r --path:compiler:/pathto/Nim1/compiler --path:compiler:/pathto/Nim2/compiler main

# main.nim
import compiler/ast # => resolves to /pathto/Nim2/compiler/ast
import compiler/newmodule # module not found error even if /pathto/Nim1/compiler/newmodule.nim exists

note that you can override a child prefix, which is an important feature (eg for debugging or customization) eg:

nim r --path:compiler:/pathto/Nim1/compiler --path:compiler/ast:/pathto/Nim1/compiler/ast_modif.nim main

import compiler/ast # => resolves to /pathto/Nim1/compiler/ast_modif.nim

It works for packages too:
nim r --path:compiler:/pathto/Nim1/compiler --path:compiler/plugins:/pathto/Nim_temp/compiler/plugins main

import compiler/ast # => resolves to /pathto/Nim1/compiler/ast.nim
import compiler/plugins/itersgen.nim # => resolves to /pathto/Nim_temp/compiler/plugins/itersgen.nim

benefits

  • no breaking change
  • src in nimble packages can now be omitted without any downside of scope pollution
  • tests/compiler/nim.cfg can now contain
--path:compiler:"../../compiler" # so we can `import compiler/foo` in this dir
instead of:
--path:"../../" # so we can `import compiler/foo` in this dir

and avoids scope pollution (ie bringing everything in $nim/ in import scope (eg import tools/deps) even though we were just interested in compiler/* )

  • makes it easy to swap off a particular module or (sub) package
  • paths are now explicit since we should what (sub)packages/modules we expect in a given path
  • we could also give a warning when providing a --path:prefix:path where path isn't found (this would not make as much sense with --path:path syntax)

links

@Araq
Copy link
Member

Araq commented Nov 25, 2020

This is a bit similar to my import $var / file ideas (which didn't make it into Nim) but has the downside that it's command-line only. I would like to see {.path ...} as pragmas, maybe with the restriction that only the main Nim file can use it. In general I want to get away both from Nim's config system and also from command-line only features.

@timotheecour
Copy link
Member Author

timotheecour commented Nov 25, 2020

has the downside that it's command-line only

I also want {.path.}, it simplifies some patterns. Note that {.path: "compiler:/pathto/compiler".} can be supported in addition to --path:prefix:path (and would call same logic) with the semantics that for a relative path, it resolves as usual to currentSourcePath. But that can be done later.

Note that nimble shells out to nim so {.path.} wouldn't be sufficient.

This is a bit similar to my import $var / file ideas

it subsumes it, if you allow a special token @ (or $) in import paths:

--path:@var:pathto/var
import @var # for a package
import @var/sub # for a module

and could simplify/generalize existing hard-coded logic for interpolated paths eg import "$lib/../tools/nimweb"

But the main point of the RFC is it works without modification to existing source files, eg import compiler/foo can be re-targetted via --path:compiler:path

maybe with the restriction that only the main Nim file can use

I don't think that's needed, this would restrict legitimate use case.

extension: versioning

  • this could also be useful for versioning eg in case of a package depending (via diamond dependencies) on 2 different versions of some package:
--path:cligen@1:pathto/cligen-1.0 --path:cligen@1.2:pathto/cligen-1.2
import cligen@1/foo
import cligen@1.2/foo

@timotheecour
Copy link
Member Author

  • this can also be used to improve messages with listFullPaths:off, instead of current algo which uses a mix of absolute or relative paths depending on number of .., we can now show:
    /pathto/compiler/foo.nim => compiler/foo.nim when uses passes --path:compiler:/pathto/compiler
    which gives good context as to which package error messages came from

@timotheecour
Copy link
Member Author

timotheecour commented Dec 24, 2020

  • also allows drop-in replacement for stdlib modules, eg see this proposal to replace the c-based linenoise wrapper with nimble packages: update linenoise Nim#16451 (comment) via --path:std/linenoise:pkg/noise/linenoisecompat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants