Simplify Link annotation handling

spec/compiler/semantic/lib_spec.cr

@@ -345,7 +345,7 @@ describe "Semantic: lib" do
       lib LibFoo
       end
       ),
-      "unknown link argument: 'boo' (valid arguments are 'lib', 'ldflags', 'static' and 'framework')"
+      "unknown link argument: 'boo' (valid arguments are 'lib', 'ldflags', 'static', 'pkg_config' and 'framework')"
   end
 
   it "errors if lib already specified with positional argument" do
@@ -376,6 +376,24 @@ describe "Semantic: lib" do
       )) { int32 }
   end
 
+  it "warns if @[Link(static: true)] is specified" do
+    assert_warning %(
+      @[Link("foo", static: true)]
+      lib Foo
+      end
+      ),
+      "warning in line 3\nWarning: specifying static linking for individual libraries is deprecated"
+  end
+
+  it "warns if Link annotations use positional arguments" do
+    assert_warning %(
+      @[Link("foo", "bar")]
+      lib Foo
+      end
+      ),
+      "warning in line 3\nWarning: using non-named arguments for Link annotations is deprecated"
+  end
+
   it "allows invoking lib call without obj inside lib" do
     assert_type(%(
       lib LibFoo

src/annotations.cr

@@ -27,7 +27,7 @@ end
 annotation Flags
 end
 
-# A `lib` can be marked with `@[Link(lib : String, ldflags : String, static : Bool, framework : String)]`
+# A `lib` can be marked with `@[Link(lib : String, *, ldflags : String, framework : String, pkg_config : String)]`
 # to declare the library that should be linked when compiling the program.
 #
 # At least one of the *lib*, *ldflags*, *framework* arguments needs to be specified.
@@ -38,17 +38,17 @@ end
 # 1. will lookup `pcre` using `pkg-config`, if not found
 # 2. will pass `-lpcre` to the linker.
 #
-# `@[Link("pcre", static: true)]` will favor static libraries over shared libraries.
-# 1. will lookup `libpcre.a` in `CRYSTAL_LIBRARY_PATH`, if not found
-# 2. will lookup `pcre` using `pkg-config --static`, if not found,
-# 3. will lookup `libpcre.a` in `/usr/lib`, `/usr/local/lib`
+# `@[Link("pcre", pkg_config: "libpcre")]` will lookup for a shared library.
+# 1. will lookup `libpcre` using `pkg-config`, if not found
+# 2. will lookup `pcre` using `pkg-config`, if not found
+# 3. will pass `-lpcre` to the linker.
 #
 # `@[Link(framework: "Cocoa")]` will pass `-framework Cocoa` to the linker.
 #
 # When an `-l` option is passed to the linker, it will lookup the libraries in
-# paths passed with the `-L` option. `CRYSTAL_LIBRARY_PATH`, `/usr/lib`,
-# and `/usr/local/lib` are added by default. Custom paths can be passed
-# using `ldflags`: `@[Link(ldflags: "-Lvendor/bin")]`.
+# paths passed with the `-L` option. Any paths in `CRYSTAL_LIBRARY_PATH` are
+# added by default. Custom paths can be passed using `ldflags`:
+# `@[Link(ldflags: "-Lvendor/bin")]`.
 annotation Link
 end
 

src/compiler/crystal/codegen/link.cr

@@ -1,10 +1,11 @@
 module Crystal
   struct LinkAnnotation
     getter lib : String?
+    getter pkg_config : String?
     getter ldflags : String?
     getter framework : String?
 
-    def initialize(@lib = nil, @ldflags = nil, @static = false, @framework = nil)
+    def initialize(@lib = nil, @pkg_config = @lib, @ldflags = nil, @static = false, @framework = nil)
     end
 
     def static?
@@ -22,6 +23,7 @@ module Crystal
       lib_name = nil
       lib_ldflags = nil
       lib_static = false
+      lib_pkg_config = nil
       lib_framework = nil
       count = 0
 
@@ -66,12 +68,15 @@ module Crystal
           named_arg.raise "'framework' link argument already specified" if count > 3
           named_arg.raise "'framework' link argument must be a String" unless value.is_a?(StringLiteral)
           lib_framework = value.value
+        when "pkg_config"
+          named_arg.raise "'pkg_config' link argument must be a String" unless value.is_a?(StringLiteral)
+          lib_pkg_config = value.value
         else
-          named_arg.raise "unknown link argument: '#{named_arg.name}' (valid arguments are 'lib', 'ldflags', 'static' and 'framework')"
+          named_arg.raise "unknown link argument: '#{named_arg.name}' (valid arguments are 'lib', 'ldflags', 'static', 'pkg_config' and 'framework')"
         end
       end
 
-      new(lib_name, lib_ldflags, lib_static, lib_framework)
+      new(lib_name, lib_pkg_config, lib_ldflags, lib_static, lib_framework)
     end
   end
 
@@ -109,83 +114,68 @@ module Crystal
     end
 
     private def lib_flags_posix
-      library_path = ENV["LIBRARY_PATH"]?.try(&.split(':', remove_empty: true)) ||
-                     ["/usr/lib", "/usr/local/lib"]
-      has_pkg_config = nil
+      flags = [] of String
+      static_build = has_flag?("static")
 
-      String.build do |flags|
-        link_annotations.reverse_each do |ann|
-          if ldflags = ann.ldflags
-            flags << ' ' << ldflags
-          end
+      # Instruct the linker to link statically if the user asks
+      flags << "-static" if static_build
 
-          if libname = ann.lib
-            if has_pkg_config.nil?
-              has_pkg_config = Process.run("pkg-config", ["-h"]).success?
-            end
-
-            static = has_flag?("static") || ann.static?
-
-            if static && (static_lib = find_static_lib(libname, CrystalLibraryPath.paths))
-              flags << ' ' << static_lib
-            elsif has_pkg_config && (libflags = pkg_config_flags(libname, static, library_path))
-              flags << ' ' << libflags
-            elsif static && (static_lib = find_static_lib(libname, library_path))
-              flags << ' ' << static_lib
-            else
-              flags << " -l" << libname
-            end
-          end
+      # Add CRYSTAL_LIBRARY_PATH locations, so the linker preferentially
+      # searches user-given library paths.
+      CrystalLibraryPath.paths.each do |path|
+        flags << "'-L#{path}'"
+      end
 
-          if framework = ann.framework
-            flags << " -framework " << framework
-          end
+      link_annotations.reverse_each do |ann|
+        if ldflags = ann.ldflags
+          flags << ldflags
         end
 
-        # Append the CRYSTAL_LIBRARY_PATH values as -L flags.
-        CrystalLibraryPath.paths.each do |path|
-          flags << " -L#{path}"
+        # First, check pkg-config for the pkg-config module name if provided, then
+        # check pkg-config with the lib name, then fall back to -lname
+        if (pkg_config_name = ann.pkg_config) && (flag = pkg_config(pkg_config_name, static_build))
+          flags << flag
+        elsif (lib_name = ann.lib) && (flag = pkg_config(lib_name, static_build))
+          flags << flag
+        elsif (lib_name = ann.lib)
+          flags << "-l#{lib_name}"
         end
-        # Append the default paths as -L flags in case the linker doesn't know
-        # about them (eg: FreeBSD won't search /usr/local/lib by default):
-        library_path.each do |path|
-          flags << " -L#{path}"
+
+        if framework = ann.framework
+          flags << "-framework" << framework
         end
       end
-    end
 
-    def link_annotations
-      annotations = [] of LinkAnnotation
-      add_link_annotations @types, annotations
-      annotations
+      flags.join(" ")
     end
 
-    private def pkg_config_flags(libname, static, library_path)
-      if system("pkg-config #{libname}")
-        if static
-          flags = [] of String
-          `pkg-config #{libname} --libs --static`.split.each do |cfg|
-            if cfg.starts_with?("-L")
-              library_path << cfg[2..-1]
-            elsif cfg.starts_with?("-l")
-              flags << (find_static_lib(cfg[2..-1], library_path) || cfg)
-            else
-              flags << cfg
-            end
-          end
-          flags.join ' '
-        else
-          `pkg-config #{libname} --libs`.chomp
-        end
+    PKG_CONFIG_PATH = Process.find_executable("pkg-config")
+
+    # Returns the result of running `pkg-config mod` but returns nil if
+    # pkg-config is not installed, or the module does not exist.
+    private def pkg_config(mod, static = false) : String?
+      return unless pkg_config_path = PKG_CONFIG_PATH
+      return unless Process.run(pkg_config_path, {mod}).success?
+
+      args = ["--libs"]
+      args << "--static" if static
+      args << mod
+
+      process = Process.new(pkg_config_path, args, input: :close, output: :pipe, error: :inherit)
+      flags = process.output.gets_to_end.chomp
+      status = process.wait
+      if status.success?
+        flags
+      else
+        nil
       end
     end
 
-    private def find_static_lib(libname, library_path)
-      library_path.each do |libdir|
-        static_lib = "#{libdir}/lib#{libname}.a"
-        return static_lib if File.exists?(static_lib)
-      end
-      nil
+    # Returns every @[Link] annotation in the program parsed as `LinkAnnotation`
+    def link_annotations
+      annotations = [] of LinkAnnotation
+      add_link_annotations @types, annotations
+      annotations
     end
 
     private def add_link_annotations(types, annotations)

src/compiler/crystal/compiler.cr

@@ -359,7 +359,6 @@ module Crystal
 
         link_flags = @link_flags || ""
         link_flags += " -rdynamic"
-        link_flags += " -static" if static?
 
         { %(#{cc} "${@}" -o '#{output_filename}' #{link_flags} #{program.lib_flags}), object_names }
       end

src/compiler/crystal/semantic/top_level_visitor.cr

@@ -464,7 +464,17 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor
     process_annotations(annotations) do |annotation_type, ann|
       case annotation_type
       when @program.link_annotation
-        type.add_link_annotation(LinkAnnotation.from(ann))
+        link_annotation = LinkAnnotation.from(ann)
+
+        if link_annotation.static?
+          @program.report_warning(ann, "specifying static linking for individual libraries is deprecated")
+        end
+
+        if ann.args.size > 1
+          @program.report_warning(ann, "using non-named arguments for Link annotations is deprecated")
+        end
+
+        type.add_link_annotation(link_annotation)
       when @program.call_convention_annotation
         type.call_convention = parse_call_convention(ann, type.call_convention)
       else
@@ -967,25 +977,6 @@ class Crystal::TopLevelVisitor < Crystal::SemanticVisitor
     false
   end
 
-  def process_lib_annotations
-    link_annotations = nil
-    call_convention = nil
-
-    process_annotations do |annotation_type, ann|
-      case annotation_type
-      when @program.link
-        link_annotations ||= [] of LinkAnnotation
-        link_annotations << LinkAnnotation.from(ann)
-      when @program.call_convention
-        call_convention = parse_call_convention(ann, call_convention)
-      end
-    end
-
-    @annotations = nil
-
-    {link_annotations, call_convention}
-  end
-
   def include_in(current_type, node, kind)
     node_name = node.name
 

src/gc/boehm.cr

@@ -9,7 +9,7 @@
 {% if flag?(:freebsd) || flag?(:dragonfly) %}
   @[Link("gc-threaded")]
 {% else %}
-  @[Link("gc", static: true)]
+  @[Link("gc")]
 {% end %}
 
 lib LibGC