Skip to content

Conversation

@timotheecour
Copy link
Member

@timotheecour timotheecour commented Feb 25, 2020

rationale

neither #13472 nor #13461 are correct wrt hintConf; this PR correctly handles nimConf, and honors all cfg/nims/cmdline flags that contains references to hintConf

nim c -r -f --hint:conf:on main.nim
Hint: used config file '/Users/timothee/git_clone/nim/Nim_prs/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/Nim_prs/config/config.nims' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/config.nims' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/config.nims' [Conf]
Hint: 53772 LOC; 0.390 sec; 58.574MiB peakmem; Debug build; proj: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello.nim; out: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello [SuccessX]
hello world
nim c -r -f --hint:conf:off main.nim
Hint: 53772 LOC; 0.433 sec; 58.578MiB peakmem; Debug build; proj: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello.nim; out: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello [SuccessX]
hello world
nim c -r -f main.nim

outputs with or without Conf hints depending on what's in cfg/nims after whole processing

output from #13461

  • it doesn't honor --hint:conf:on/off on cmdline
  • putting switch("hint", "conf:on") in ~/.config/nim/config.nims (or --hint:conf:off in ~/.config/nim/nim.cfg) is now not honored, and there's no way to hide those Conf hints anymore
  • output is buggy / out of order eg successX appears before some Conf hints:
Nim_prs/config/config.nims' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/config.nims' [Conf]
Hint: 53762 LOC; 0.294 sec; 57.578MiB peakmem; Debug build; proj: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello.nim; out: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello [SuccessX]
Hint: used config file '/Users/timothee/git_clone/nim/Nim_prs/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
hello world

output from #13461

  • it doesn't honor --hint:conf:on/off on cmdline
  • putting switch("hint", "conf:on") in ~/.config/nim/config.nims (or --hint:conf:off in ~/.config/nim/nim.cfg) is now not honored, and there's no way to hide those Conf hints anymore
  • Conf hint file order is also buggy
Hint: used config file '/Users/timothee/git_clone/nim/Nim_prs/config/config.nims' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/config.nims' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/Nim_prs/config/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/.config/nim/nim.cfg' [Conf]
Hint: used config file '/Users/timothee/git_clone/nim/timn/nim.cfg' [Conf]
Hint: 53771 LOC; 0.262 sec; 57.523MiB peakmem; Debug build; proj: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello.nim; out: /Users/timothee/git_clone/nim/timn/src/timn/examples/thello [SuccessX]
hello world

@timotheecour timotheecour force-pushed the pr_fix_9405_nims_cfg branch 2 times, most recently from 6bd56ea to 38eeb9e Compare February 25, 2020 03:37
This was referenced Feb 25, 2020
@timotheecour
Copy link
Member Author

PTAL

@timotheecour
Copy link
Member Author

timotheecour commented Feb 27, 2020

PTAL

please see here: https://github.com/nim-lang/Nim/pull/13488/files/ed82b9a482dd0a9b90547117ab8eb641feed1993..53d4ad816cbf5a50da74de7d6a3fb26437b3ed95 for the range of commits after @genotrance ' commit

I fixed a number of issues to have saner semantics wrt how notes are processed amongst defaults, verbosity, cmdline, config.

implementation notes

(in case the comments I added in code aren't enough)

  • i've removed enableNotes and disableNotes and introduced modifiedyNotes and cmdlineNotes; that's the simplest design I can think of that achieves correct semantics, and has 0 net added flags (+2, -2)

  • modifiedyNotes is set whenever a config/cmdline sets/unsets a note; this is used to make --verbosity:n work: the semantics are now: --verbosity:n sets a new default for notes; but whatever's in your config/cmdline will override this (setting/unsetting as needed); for example:
    --verbosity:3 --hint:processing:off previously didn't honor --hint:processing:off, now it does

  • cmdlineNotes is set whenever a cmdline sets/unsets a note, so that we "freeze" that note from further modification by the subsequent config logic; this is needed for example to tell whether --hint:config:on (or off) if passed on cmdline, so that it works even if config files contradict this; that's why in this PR I've removed the comment below:

graph.compileSystemModule() # TODO: see why this unsets hintConf in conf.notes

Without this, you'd get the errors I mentioned in top post (see output from #13461 and output from #13461)

@Clyybber
Copy link
Contributor

Clyybber commented Mar 6, 2020

Note: This should be backported

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

Successfully merging this pull request may close these issues.

global user config can override project specific config

4 participants