Remove CRYSTAL_LIBRARY_RPATH and delay-load helper (#14598)

UPGRADING.md

@@ -12,6 +12,13 @@ We're only listing the most relevant changes here that could have a relevant imp
 The [changelog](./CHANGELOG.md) contains more information about all changes in
 a specific release.
 
+## Crystal 1.13
+
+* `CRYSTAL_LIBRARY_RPATH` and the `preview_win32_delay_load` feature flag have
+  been removed. Individual DLLs can be explicitly delay-loaded with the MSVC
+  toolchain by using `/DELAYLOAD` as a linker flag. Similarly RPATH can be added
+  with GCC or Clang toolchains by adding `-Wl,-rpath`.
+
 ## Crystal 1.9
 
 * The implementation of the comparison operator `#<=>` between `Big*` (`BigDecimal`,

spec/std/spec_helper.cr

@@ -86,10 +86,9 @@ def compile_file(source_file, *, bin_name = "executable_file", flags = %w(), fil
     args = ["build"] + flags + ["-o", executable_file, source_file]
     output = IO::Memory.new
     status = Process.run(compiler, args, env: {
-      "CRYSTAL_PATH"          => Crystal::PATH,
-      "CRYSTAL_LIBRARY_PATH"  => Crystal::LIBRARY_PATH,
-      "CRYSTAL_LIBRARY_RPATH" => Crystal::LIBRARY_RPATH,
-      "CRYSTAL_CACHE_DIR"     => Crystal::CACHE_DIR,
+      "CRYSTAL_PATH"         => Crystal::PATH,
+      "CRYSTAL_LIBRARY_PATH" => Crystal::LIBRARY_PATH,
+      "CRYSTAL_CACHE_DIR"    => Crystal::CACHE_DIR,
     }, output: output, error: output)
 
     unless status.success?

src/compiler/crystal/codegen/link.cr

@@ -114,59 +114,6 @@ module Crystal
       default_paths.join(Process::PATH_DELIMITER)
     end
 
-    def self.default_rpath : String
-      # do not call `CrystalPath.expand_paths`, as `$ORIGIN` inside this env
-      # variable is always expanded at run time
-      ENV.fetch("CRYSTAL_LIBRARY_RPATH", "")
-    end
-
-    # Adds the compiler itself's RPATH to the environment for the duration of
-    # the block. `$ORIGIN` in the compiler's RPATH is expanded immediately, but
-    # `$ORIGIN`s in the existing environment variable are not expanded. For
-    # example, on Linux:
-    #
-    # - CRYSTAL_LIBRARY_RPATH of the compiler: `$ORIGIN/so`
-    # - Current $CRYSTAL_LIBRARY_RPATH: `/home/foo:$ORIGIN/mylibs`
-    # - Compiler's full path: `/opt/crystal`
-    # - Generated executable's Crystal::LIBRARY_RPATH: `/home/foo:$ORIGIN/mylibs:/opt/so`
-    #
-    # On Windows we additionally append the compiler's parent directory to the
-    # list, as if by appending `$ORIGIN` to the compiler's RPATH. This directory
-    # is effectively the first search entry on any Windows executable. Example:
-    #
-    # - CRYSTAL_LIBRARY_RPATH of the compiler: `$ORIGIN\dll`
-    # - Current %CRYSTAL_LIBRARY_RPATH%: `C:\bar;$ORIGIN\mylibs`
-    # - Compiler's full path: `C:\foo\crystal.exe`
-    # - Generated executable's Crystal::LIBRARY_RPATH: `C:\bar;$ORIGIN\mylibs;C:\foo\dll;C:\foo`
-    #
-    # Combining RPATHs multiple times has no effect; the `CRYSTAL_LIBRARY_RPATH`
-    # environment variable at compiler startup is used, not really the "current"
-    # one. This can happen when running a program that also uses macro `run`s.
-    def self.add_compiler_rpath(&)
-      executable_path = Process.executable_path
-      compiler_origin = File.dirname(executable_path) if executable_path
-
-      current_rpaths = ORIGINAL_CRYSTAL_LIBRARY_RPATH.try &.split(Process::PATH_DELIMITER, remove_empty: true)
-      compiler_rpaths = Crystal::LIBRARY_RPATH.split(Process::PATH_DELIMITER, remove_empty: true)
-      CrystalPath.expand_paths(compiler_rpaths, compiler_origin)
-
-      rpaths = compiler_rpaths
-      rpaths.concat(current_rpaths) if current_rpaths
-      {% if flag?(:win32) %}
-        rpaths << compiler_origin if compiler_origin
-      {% end %}
-
-      old_env = ENV["CRYSTAL_LIBRARY_RPATH"]?
-      ENV["CRYSTAL_LIBRARY_RPATH"] = rpaths.join(Process::PATH_DELIMITER)
-      begin
-        yield
-      ensure
-        ENV["CRYSTAL_LIBRARY_RPATH"] = old_env
-      end
-    end
-
-    private ORIGINAL_CRYSTAL_LIBRARY_RPATH = ENV["CRYSTAL_LIBRARY_RPATH"]?
-
     class_getter paths : Array(String) do
       default_paths
     end

src/compiler/crystal/command.cr

@@ -349,15 +349,14 @@ class Crystal::Command
     hierarchy_exp : String?,
     cursor_location : String?,
     output_format : String,
-    combine_rpath : Bool,
     includes : Array(String),
     excludes : Array(String),
     verbose : Bool,
     check : Bool,
     tallies : Bool do
     def compile(output_filename = self.output_filename)
       compiler.emit_base_filename = emit_base_filename || output_filename.rchop(File.extname(output_filename))
-      compiler.compile sources, output_filename, combine_rpath: combine_rpath
+      compiler.compile sources, output_filename
     end
 
     def top_level_semantic
@@ -632,10 +631,9 @@ class Crystal::Command
       emit_base_filename = ::Path[sources.first.filename].stem
     end
 
-    combine_rpath = run && !compiler.no_codegen?
     @config = CompilerConfig.new compiler, sources, output_filename, emit_base_filename,
       arguments, specified_output, hierarchy_exp, cursor_location, output_format.not_nil!,
-      combine_rpath, includes, excludes, verbose, check, tallies
+      includes, excludes, verbose, check, tallies
   end
 
   private def gather_sources(filenames)

src/compiler/crystal/command/env.cr

@@ -18,12 +18,11 @@ class Crystal::Command
     end
 
     vars = {
-      "CRYSTAL_CACHE_DIR"     => CacheDir.instance.dir,
-      "CRYSTAL_PATH"          => CrystalPath.default_path,
-      "CRYSTAL_VERSION"       => Config.version || "",
-      "CRYSTAL_LIBRARY_PATH"  => CrystalLibraryPath.default_path,
-      "CRYSTAL_LIBRARY_RPATH" => CrystalLibraryPath.default_rpath,
-      "CRYSTAL_OPTS"          => ENV.fetch("CRYSTAL_OPTS", ""),
+      "CRYSTAL_CACHE_DIR"    => CacheDir.instance.dir,
+      "CRYSTAL_PATH"         => CrystalPath.default_path,
+      "CRYSTAL_VERSION"      => Config.version || "",
+      "CRYSTAL_LIBRARY_PATH" => CrystalLibraryPath.default_path,
+      "CRYSTAL_OPTS"         => ENV.fetch("CRYSTAL_OPTS", ""),
     }
 
     if var_names.empty?

src/compiler/crystal/command/eval.cr

@@ -26,7 +26,7 @@ class Crystal::Command
 
     output_filename = Crystal.temp_executable "eval"
 
-    compiler.compile sources, output_filename, combine_rpath: true
+    compiler.compile sources, output_filename
     execute output_filename, program_args, compiler
   end
 end

src/compiler/crystal/command/spec.cr

@@ -96,7 +96,7 @@ class Crystal::Command
     output_filename = Crystal.temp_executable "spec"
 
     ENV["CRYSTAL_SPEC_COMPILER_BIN"] ||= Process.executable_path
-    compiler.compile sources, output_filename, combine_rpath: true
+    compiler.compile sources, output_filename
     report_warnings
     execute output_filename, options, compiler, error_on_exit: warnings_fail_on_exit?
   end

src/compiler/crystal/compiler.cr

@@ -198,21 +198,12 @@ module Crystal
     # Compiles the given *source*, with *output_filename* as the name
     # of the generated executable.
     #
-    # If *combine_rpath* is true, add the compiler itself's RPATH to the
-    # generated executable via `CrystalLibraryPath.add_compiler_rpath`. This is
-    # used by the `run` / `eval` / `spec` commands as well as the macro `run`
-    # (via `Crystal::Program#macro_compile`), and never during cross-compiling.
-    #
     # Raises `Crystal::CodeError` if there's an error in the
     # source code.
     #
     # Raises `InvalidByteSequenceError` if the source code is not
     # valid UTF-8.
-    def compile(source : Source | Array(Source), output_filename : String, *, combine_rpath : Bool = false) : Result
-      if combine_rpath
-        return CrystalLibraryPath.add_compiler_rpath { compile(source, output_filename, combine_rpath: false) }
-      end
-
+    def compile(source : Source | Array(Source), output_filename : String) : Result
       source = [source] unless source.is_a?(Array)
       program = new_program(source)
       node = parse program, source
@@ -443,22 +434,6 @@ module Crystal
             end
 
             link_args = search_result.remaining_args.concat(search_result.library_paths).map { |arg| Process.quote_windows(arg) }
-
-            if program.has_flag?("preview_win32_delay_load")
-              # "LINK : warning LNK4199: /DELAYLOAD:foo.dll ignored; no imports found from foo.dll"
-              # it is harmless to skip this error because not all import libraries are always used, much
-              # less the individual DLLs they refer to
-              link_args << "/IGNORE:4199"
-
-              dlls = Set(String).new
-              search_result.library_paths.each do |library_path|
-                Crystal::System::LibraryArchive.imported_dlls(library_path).each do |dll|
-                  dlls << dll.downcase
-                end
-              end
-              dlls.delete "kernel32.dll"
-              dlls.each { |dll| link_args << "/DELAYLOAD:#{dll}" }
-            end
           end
         {% end %}
 

src/compiler/crystal/loader/msvc.cr

@@ -172,7 +172,7 @@ class Crystal::Loader
   end
 
   private def open_library(path : String)
-    # TODO: respect Crystal::LIBRARY_RPATH (#13490), or `@[Link(dll:)]`'s search order
+    # TODO: respect `@[Link(dll:)]`'s search order
     LibC.LoadLibraryExW(System.to_wstr(path), nil, 0)
   end
 

src/compiler/crystal/macros/macros.cr

@@ -93,7 +93,7 @@ class Crystal::Program
       return CompiledMacroRun.new(executable_path, elapsed_time, true)
     end
 
-    result = host_compiler.compile Compiler::Source.new(filename, source), executable_path, combine_rpath: true
+    result = host_compiler.compile Compiler::Source.new(filename, source), executable_path
 
     # Write the new files from which 'filename' depends into the cache dir
     # (here we store how to obtain these files, because a require might use

src/compiler/crystal/program.cr

@@ -311,11 +311,6 @@ module Crystal
 
         The value is defined by the environment variables `CRYSTAL_LIBRARY_PATH`.
         MD
-      define_crystal_string_constant "LIBRARY_RPATH", Crystal::CrystalLibraryPath.default_rpath, <<-MD
-        Colon-separated paths where the loader searches for dynamic libraries at runtime.
-
-        The value is defined by the environment variables `CRYSTAL_LIBRARY_RPATH`.
-        MD
       define_crystal_string_constant "VERSION", Crystal::Config.version, <<-MD
         The version of the Crystal compiler.
         MD

src/compiler/crystal/tools/doc/generator.cr

@@ -227,7 +227,7 @@ class Crystal::Doc::Generator
 
     {"BUILD_COMMIT", "BUILD_DATE", "CACHE_DIR", "DEFAULT_PATH",
      "DESCRIPTION", "PATH", "VERSION", "LLVM_VERSION",
-     "LIBRARY_PATH", "LIBRARY_RPATH", "HOST_TRIPLE", "TARGET_TRIPLE"}.each do |name|
+     "LIBRARY_PATH", "HOST_TRIPLE", "TARGET_TRIPLE"}.each do |name|
       return true if type == crystal_type.types[name]?
     end
 

src/crystal/main.cr

@@ -1,17 +1,5 @@
 require "process/executable_path" # Process::PATH_DELIMITER
 
-module Crystal
-  {% unless Crystal.has_constant?("LIBRARY_RPATH") %}
-    LIBRARY_RPATH = {{ env("CRYSTAL_LIBRARY_RPATH") || "" }}
-  {% end %}
-end
-
-{% if flag?(:unix) && !flag?(:darwin) %}
-  {% unless Crystal::LIBRARY_RPATH.empty? %}
-    # TODO: is there a better way to quote this?
-    @[Link(ldflags: {{ "'-Wl,-rpath,#{Crystal::LIBRARY_RPATH.id}'" }})]
-  {% end %}
-{% end %}
 lib LibCrystalMain
   @[Raises]
   fun __crystal_main(argc : Int32, argv : UInt8**)
@@ -143,7 +131,6 @@ end
 
 {% if flag?(:win32) %}
   require "./system/win32/wmain"
-  require "./system/win32/delay_load"
 {% end %}
 
 {% if flag?(:wasi) %}