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

fix #16206, nim r / nim -r recompiles if cwd changes #16349

Merged

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Dec 14, 2020

fix #16271

nim r / nim -r recompiles if cwd changes

I'm hitting same bug as original reporter of #16271, eg with nim r testament/testament args...; this PR fixes that in the simplest possible way, that is robust even if pessimistic (recompile on cwd change). Future PR's can add more logic to be robust and less pessimistic, see comment in PR.

future work

  • -d:nimBetterRunIgnoreCwd (see pr_fix_nimcache_nimrun_16271_followup_nimBetterRunIgnoreCwd wip PR)
  • robust approach described inside PR comment (it has other advantages which is caching more than 1 compilation, using a LRU cache to avoid blowing up disk usage)

@Clyybber
Copy link
Contributor

This means that doing

nim c file
cd ..
nim c folder/file

triggers a recompilation. This is too eager IMO.
Instead those flags that implicitly depend on the CWD should be checked to make sure the CWD is added to the respective path before checking for changes.

@timotheecour
Copy link
Member Author

timotheecour commented Dec 14, 2020

Instead those flags that implicitly depend on the CWD should be checked to make sure the CWD is added to the respective path before checking for changes.

I don't know of a reliable way to do that, so many flags can depend on CWD:

# examples:
nim r --lib:lib main
--path:.
--outdir:dir
-o:bin/main

# also:
--import
--include
--cincludes
--nimcache
--clibdir
--clib
--docRoot
--NimblePath
--excludePath

Furthermore, this would involve among other things understanding and parsing clang flags and linker flags, eg:

nim r --passl:-Wl,-rpath,lib --hotcodereloading:on /pathto/main.nim

or nim's own flags: --docCmd:"--lib:lib"

I also don't know which other flags (or error messages, depending on provided flags) implicitly depend on cwd, jsondoc ctags buildIndex genDepend, genDeps...

The robust approach I have in mind is described inside PR comment, and can be done later (it has other advantages which is caching more than 1 compilation, using a LRU cache to avoid blowing up disk usage).

That said, I'd be totally ok with an opt-in flag though, eg -d:nimBetterRunIgnoreCwd (named after existing -d:nimBetterRun, or a better name) which would use a best effort approach (starting with expanding input project file).

@timotheecour
Copy link
Member Author

timotheecour commented Dec 15, 2020

@Clyybber in light of #16354 i think we should merge this first, and once #16354 is fixed, introduce an opt-in -d:nimBetterRunIgnoreCwd which would use a best effort (optimistic) algorithm to decide whether to rebuild.

Copy link
Member

@ringabout ringabout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: #16531

@timotheecour timotheecour merged commit 854ff26 into nim-lang:devel Jan 2, 2021
@timotheecour timotheecour deleted the pr_fix_nimcache_nimrun_16271 branch January 2, 2021 09:34
mildred pushed a commit to mildred/Nim that referenced this pull request Jan 11, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
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.

Nimcache confusion compiling files with the same name in different directories
4 participants