Skip to content

Commit

Permalink
Miscellaneous Windows-related fixes in the compiler (crystal-lang#9054)
Browse files Browse the repository at this point in the history
* Temporary file needs .exe extension to be executable
* Replacing '/' is not enough to sanitize a filename, use Path splitting and replace the drive ':' manually
* Split PATH by the correct delimiter
* Don't try to fork (n_threads=1)
* Don't try to set a directory's mtime on Windows, it errors
* Remove custom abs-path code that is already handled by expand_path
* Correct string interpolation in ECR
* Allow overriding `llvm-config --targets-built` because the host LLVM doesn't necessarily match target LLVM
* Skip signal code

Unrelated: handle non-exit non-signal exit statuses correctly on POSIX.
  • Loading branch information
oprypin authored and carlhoerberg committed Apr 29, 2020
1 parent 088af4e commit d4e615f
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 50 deletions.
4 changes: 2 additions & 2 deletions spec/spec_helper.cr
Expand Up @@ -115,7 +115,7 @@ end
def warnings_result(code, inject_primitives = true)
code = inject_primitives(code) if inject_primitives

output_filename = Crystal.tempfile("crystal-spec-output")
output_filename = Crystal.temp_executable("crystal-spec-output")

compiler = create_spec_compiler
compiler.warnings = Warnings::All
Expand Down Expand Up @@ -244,7 +244,7 @@ def run(code, filename = nil, inject_primitives = true, debug = Crystal::Debug::
ast.expressions[-1] = exps
code = ast.to_s

output_filename = Crystal.tempfile("crystal-spec-output")
output_filename = Crystal.temp_executable("crystal-spec-output")

compiler = create_spec_compiler
compiler.debug = debug
Expand Down
15 changes: 12 additions & 3 deletions src/compiler/crystal/codegen/cache_dir.cr
Expand Up @@ -31,9 +31,18 @@ module Crystal
def directory_for(filename : String)
dir = compute_dir

name = filename.gsub('/', '-')
while name.starts_with?('-')
name = name[1..-1]
filename = ::Path[filename]
name = String.build do |io|
filename.each_part do |part|
if io.empty?
if part == "#{filename.anchor}"
part = "#{filename.drive}"[..0]
end
else
io << '-'
end
io << part
end
end
output_dir = File.join(dir, name)
Dir.mkdir_p(output_dir)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/codegen/link.cr
Expand Up @@ -81,7 +81,7 @@ module Crystal
end

class_getter paths : Array(String) do
default_path.split(':', remove_empty: true)
default_path.split(Process::PATH_DELIMITER, remove_empty: true)
end
end

Expand Down
30 changes: 17 additions & 13 deletions src/compiler/crystal/command.cr
Expand Up @@ -196,7 +196,7 @@ class Crystal::Command
return
end

output_filename = Crystal.tempfile(config.output_filename)
output_filename = Crystal.temp_executable(config.output_filename)

result = config.compile output_filename

Expand Down Expand Up @@ -230,9 +230,11 @@ class Crystal::Command
begin
elapsed = Time.measure do
Process.run(output_filename, args: run_args, input: Process::Redirect::Inherit, output: Process::Redirect::Inherit, error: Process::Redirect::Inherit) do |process|
# Ignore the signal so we don't exit the running process
# (the running process can still handle this signal)
::Signal::INT.ignore # do
{% unless flag?(:win32) %}
# Ignore the signal so we don't exit the running process
# (the running process can still handle this signal)
::Signal::INT.ignore # do
{% end %}
end
end
{$?, elapsed}
Expand All @@ -252,22 +254,24 @@ class Crystal::Command
puts "Execute: #{elapsed_time}"
end

if status.normal_exit?
case status
when .normal_exit?
exit error_on_exit ? 1 : status.exit_code
else
case status.exit_signal
when ::Signal::KILL
when .signal_exit?
case signal = status.exit_signal
when .kill?
STDERR.puts "Program was killed"
when ::Signal::SEGV
when .segv?
STDERR.puts "Program exited because of a segmentation fault (11)"
when ::Signal::INT
when .int?
# OK, bubbled from the sub-program
else
STDERR.puts "Program received and didn't handle signal #{status.exit_signal} (#{status.exit_signal.value})"
STDERR.puts "Program received and didn't handle signal #{signal} (#{signal.value})"
end

exit 1
else
STDERR.puts "Program exited abnormally, the cause is unknown"
end
exit 1
end

record CompilerConfig,
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/command/eval.cr
Expand Up @@ -24,7 +24,7 @@ class Crystal::Command

sources = [Compiler::Source.new("eval", program_source)]

output_filename = Crystal.tempfile "eval"
output_filename = Crystal.temp_executable "eval"

result = compiler.compile sources, output_filename
execute output_filename, program_args, compiler
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/command/spec.cr
Expand Up @@ -69,7 +69,7 @@ class Crystal::Command
source = target_filenames.map { |filename| %(require "./#{filename}") }.join('\n')
sources = [Compiler::Source.new(source_filename, source)]

output_filename = Crystal.tempfile "spec"
output_filename = Crystal.temp_executable "spec"

result = compiler.compile sources, output_filename
report_warnings result
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/compiler.cr
Expand Up @@ -73,7 +73,7 @@ module Crystal
property? no_codegen = false

# Maximum number of LLVM modules that are compiled in parallel
property n_threads : Int32 = {% if flag?(:preview_mt) %} 1 {% else %} 8 {% end %}
property n_threads : Int32 = {% if flag?(:preview_mt) || flag?(:win32) %} 1 {% else %} 8 {% end %}

# Default prelude file to use. This ends up adding a
# `require "prelude"` (or whatever name is set here) to
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/crystal/crystal_path.cr
Expand Up @@ -13,7 +13,7 @@ module Crystal
@crystal_path : Array(String)

def initialize(path = CrystalPath.default_path, codegen_target = Config.default_target)
@crystal_path = path.split(':').reject &.empty?
@crystal_path = path.split(Process::PATH_DELIMITER).reject &.empty?
add_target_path(codegen_target)
end

Expand Down Expand Up @@ -144,7 +144,6 @@ module Crystal
end

private def make_relative_unless_absolute(filename)
filename = "#{Dir.current}/#{filename}" unless filename.starts_with?('/')
File.expand_path(filename)
end

Expand Down
23 changes: 6 additions & 17 deletions src/compiler/crystal/macros/macros.cr
Expand Up @@ -15,19 +15,6 @@ class Crystal::Program
record CompiledMacroRun, filename : String, elapsed : Time::Span, reused : Bool
property compiled_macros_cache = {} of String => CompiledMacroRun

# Returns a new temporary file, which tries to be stored in the
# cache directory associated to a program. This file is then added
# to `tempfiles` so they can eventually be deleted.
def new_tempfile(basename)
filename = if cache_dir = @cache_dir
File.join(cache_dir, basename)
else
Crystal.tempfile(basename)
end
tempfiles << filename
filename
end

def expand_macro(a_macro : Macro, call : Call, scope : Type, path_lookup : Type? = nil, a_def : Def? = nil)
interpreter = MacroInterpreter.new self, scope, path_lookup || scope, a_macro, call, a_def, in_macro: true
a_macro.body.accept interpreter
Expand Down Expand Up @@ -94,10 +81,12 @@ class Crystal::Program
recorded_requires_path = File.join(program_dir, "recorded_requires")
requires_path = File.join(program_dir, "requires")

# First, update times for the program dir, so it remains in the cache longer
# (this is specially useful if a macro run program is used by multiple programs)
now = Time.utc
File.utime(now, now, program_dir)
{% unless flag?(:win32) %}
# First, update times for the program dir, so it remains in the cache longer
# (this is specially useful if a macro run program is used by multiple programs)
now = Time.utc
File.utime(now, now, program_dir)
{% end %}

if can_reuse_previous_compilation?(filename, executable_path, recorded_requires_path, requires_path)
elapsed_time = Time.monotonic - time
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/tools/init.cr
Expand Up @@ -281,7 +281,7 @@ module Crystal

macro template(name, template_path, destination_path)
class {{name.id}} < View
ECR.def_to_s "{{TEMPLATE_DIR.id}}/{{template_path.id}}"
ECR.def_to_s {{"#{TEMPLATE_DIR.id}/#{template_path.id}"}}

def path
{{destination_path}}
Expand Down
6 changes: 1 addition & 5 deletions src/compiler/crystal/tools/playground/server.cr
Expand Up @@ -56,7 +56,7 @@ module Crystal::Playground
return
end

output_filename = tempfile "play-#{@session_key}-#{tag}"
output_filename = Crystal.temp_executable "play-#{@session_key}-#{tag}"
compiler = Compiler.new
compiler.color = false
begin
Expand Down Expand Up @@ -156,10 +156,6 @@ module Crystal::Playground
end
end

private def tempfile(basename)
Crystal.tempfile(basename)
end

private def stop_process
if process = @process
Log.info { "Code execution killed (session=#{@session_key}, filename=#{@running_process_filename})." }
Expand Down
16 changes: 14 additions & 2 deletions src/compiler/crystal/util.cr
@@ -1,10 +1,14 @@
require "path"

module Crystal
def self.relative_filename(filename)
return filename unless filename.is_a?(String)

if base_file = filename.lchop? Dir.current
if file_prefix = base_file.lchop? '/'
return file_prefix
::Path::SEPARATORS.each do |sep|
if file_prefix = base_file.lchop? sep
return file_prefix
end
end
return base_file
end
Expand All @@ -21,6 +25,14 @@ module Crystal
CacheDir.instance.join("crystal-run-#{basename}.tmp")
end

def self.temp_executable(basename)
name = tempfile(basename)
{% if flag?(:win32) %}
name += ".exe"
{% end %}
name
end

def self.with_line_numbers(
source : String | Array(String),
highlight_line_number = nil,
Expand Down
4 changes: 3 additions & 1 deletion src/llvm/lib_llvm.cr
Expand Up @@ -13,7 +13,9 @@ end
{% end %}
lib LibLLVM
VERSION = {{`#{LibLLVM::LLVM_CONFIG} --version`.chomp.stringify}}
BUILT_TARGETS = {{ `#{LibLLVM::LLVM_CONFIG} --targets-built`.strip.downcase.split(' ').map(&.id.symbolize) }}
BUILT_TARGETS = {{ (
env("LLVM_TARGETS") || `#{LibLLVM::LLVM_CONFIG} --targets-built`
).strip.downcase.split(' ').map(&.id.symbolize) }}
end
{% end %}

Expand Down

0 comments on commit d4e615f

Please sign in to comment.