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

Running multiple macro runs doesn't work #2624

Closed
asterite opened this issue May 20, 2016 · 2 comments
Closed

Running multiple macro runs doesn't work #2624

asterite opened this issue May 20, 2016 · 2 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler

Comments

@asterite
Copy link
Member

# foo.cr
{% run "./bar" %}
{% run "./baz" %}
{% run "./bar" %}

# bar.cr
puts %(puts "hello")

# baz.cr
puts %(puts "bye")

Running crystal foo.cr results in:

--: /Users/asterite-manas/.cache/crystal/crystal-run-macro-run-_Users_asterite-manas_Projects_crystal_bar.cr.tmp: No such file or directory
Error in ./foo.cr:3: expanding macro

{% run "./bar" %}
^

in ./foo.cr:3: Error executing run: ./bar 

Got:

{% run "./bar" %}
   ^~~

The problem is that in 0.16.0 we changed the way .crystal works, and as part of its cleanup I decided to remove temporary files inside that directory. Temporary files are ones that result from old crystal file.cr. When a macro run is executed, it compiles that file and then executes it. It relies on the temporary executable to be there for next invocations, so that the file pointed by the macro run doesn't need to be compiled over and over. The problem is that a second macro run with a different file erases the previous temporary executable.

I'll fix this and release 0.17.3, as I think it affects a bunch of projects. At least slang and frost. I'll also include a bunch of fixes I put in that branch, together with this, so in 0.18.0 we can have correct Tuple.new and NamedTuple.new :-)

I still don't know what should the fix be. We could leave temporary files there forever, but that's not good. Maybe a better one is to delete all temporary files that a main compilation uses: the temporary file that results from the main compilation, if any, and temporary files from macro runs.

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels May 20, 2016
@ozra
Copy link
Contributor

ozra commented May 20, 2016

Storage is cheap, better to let the standard cache-cleaning mechanism do work and be configurable (only so and so much allowed in "~/.cache/crystal/" before wiping oldest).
Re-compiling could be further sped up if macros are unchanged and the compilation process is further optimized to reduce work by identifying as early as possible when there are no changes to a prog/unit/etc...
Perhaps even at the stage of evaluating run...

@jhass
Copy link
Member

jhass commented May 20, 2016

Can we perhaps somehow scope the target filename into the cache subdirectory for the current compilation?

@jhass jhass closed this as completed in f27e37e May 20, 2016
keplersj added a commit to keplersj/crystal that referenced this issue May 21, 2016
* master: (69 commits)
  Fixed: splat restriction and double splat restriction didn't match empty tuple/named-tuple
  Fixed crystal-lang#1203: allow using free var to match the size of a static array
  Updated Changelog
  Compiler: don't remove old directories when compiling macro run programs. Related to crystal-lang#2625
  Fixed crystal-lang#2624: Running multiple macro runs doesn't work
  Docker: Preinstall git in released docker image
  Update and fix TypeNode macro method docs
  Fixed wrong datatype in 0.17 branch
  Docs: use `*` and `**` before type argument for Tuple and NamedTuple
  Docs: use `*` and `**` before type argument for Tuple and NamedTuple
  Compiler: allow a double splat restriction on a double splat argument
  Compiler: allow a splat restriction on a splat argument
  Compiler: allow restriction in double splat
  Compiler: guess type from captured block without restriction
  Fixed crystal-lang#731: initialize's default arguments are incorrectly evaluated at the class scope
  Fixed crystal-lang#2619: wrong codegen for global variable assignment in type declaration
  Compiler: allow a double splat restriction on a double splat argument
  Compiler: allow a splat restriction on a splat argument
  Compiler: allow restriction in double splat
  Compiler: guess type from captured block without restriction
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler
Projects
None yet
Development

No branches or pull requests

3 participants