From 80b2484c3fb95cae09bce60cde3930a0df826cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 17 Sep 2024 21:17:40 +0200 Subject: [PATCH 1/5] Enable runners from `runs-on.com` for Aarch64 CI (#15007) https://runs-on.com is a service that provisions machines on AWS on demand to run workflow jobs. It triggers when a job is tagged as `runs-on` and you can configure what kind of machine you like this to run on. The machine specification could likely use some fine tuning (we can use other instance types, and theoretically 8GB should be sufficient). But that'll be a follow-up. For now we know that this works. We expect this to be more price-efficient setup than renting a fixed server or a CI runner service. This patch also includes an update to the latest base image. The old arm base images were using Crystal 1.0 and some outdated libraries which caused problems on the new runners (#15005). The build images are based on the docker images from [84codes/crystal](https://hub.docker.com/r/84codes/crystal). --- .github/workflows/aarch64.yml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/aarch64.yml b/.github/workflows/aarch64.yml index da252904fa37..85a8af2c8b37 100644 --- a/.github/workflows/aarch64.yml +++ b/.github/workflows/aarch64.yml @@ -8,13 +8,13 @@ concurrency: jobs: aarch64-musl-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -26,7 +26,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-musl-test-stdlib: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -38,12 +38,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: - args: make std_spec FLAGS=-Duse_pcre + args: make std_spec aarch64-musl-test-compiler: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -55,17 +55,17 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make primitives_spec compiler_spec FLAGS=-Dwithout_ffi aarch64-gnu-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -77,7 +77,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-gnu-test-stdlib: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -89,12 +89,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make std_spec aarch64-gnu-test-compiler: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -106,6 +106,6 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make primitives_spec compiler_spec From 47cd33b259ac46533f8898bdd70fbd15fa9ab306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 6 Sep 2024 08:19:38 +0200 Subject: [PATCH 2/5] Fix use global paths in macro bodies (#14965) Macros inject code into other scopes. Paths are resolved in the expanded scope and there can be namespace conflicts. This fixes non-global paths in macro bodies that expand into uncontrolled scopes where namespaces could clash. This is a fixup for #14282 (released in 1.12.0). --- src/crystal/pointer_linked_list.cr | 4 ++-- src/ecr/macros.cr | 2 +- src/intrinsics.cr | 34 +++++++++++++++--------------- src/json/serialization.cr | 6 +++--- src/number.cr | 8 +++---- src/object.cr | 12 +++++------ src/slice.cr | 6 +++--- src/spec/dsl.cr | 4 ++-- src/spec/helpers/iterate.cr | 8 +++---- src/static_array.cr | 2 +- src/syscall/aarch64-linux.cr | 2 +- src/syscall/arm-linux.cr | 2 +- src/syscall/i386-linux.cr | 2 +- src/syscall/x86_64-linux.cr | 2 +- src/yaml/serialization.cr | 14 ++++++------ 15 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/crystal/pointer_linked_list.cr b/src/crystal/pointer_linked_list.cr index 03109979d662..cde9b0b79ddc 100644 --- a/src/crystal/pointer_linked_list.cr +++ b/src/crystal/pointer_linked_list.cr @@ -7,8 +7,8 @@ struct Crystal::PointerLinkedList(T) module Node macro included - property previous : Pointer(self) = Pointer(self).null - property next : Pointer(self) = Pointer(self).null + property previous : ::Pointer(self) = ::Pointer(self).null + property next : ::Pointer(self) = ::Pointer(self).null end end diff --git a/src/ecr/macros.cr b/src/ecr/macros.cr index 92c02cc4284a..5e051232271b 100644 --- a/src/ecr/macros.cr +++ b/src/ecr/macros.cr @@ -34,7 +34,7 @@ module ECR # ``` macro def_to_s(filename) def to_s(__io__ : IO) : Nil - ECR.embed {{filename}}, "__io__" + ::ECR.embed {{filename}}, "__io__" end end diff --git a/src/intrinsics.cr b/src/intrinsics.cr index c5ae837d8931..7cdc462ce543 100644 --- a/src/intrinsics.cr +++ b/src/intrinsics.cr @@ -179,7 +179,7 @@ end module Intrinsics macro debugtrap - LibIntrinsics.debugtrap + ::LibIntrinsics.debugtrap end def self.pause @@ -191,15 +191,15 @@ module Intrinsics end macro memcpy(dest, src, len, is_volatile) - LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memmove(dest, src, len, is_volatile) - LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memset(dest, val, len, is_volatile) - LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) end def self.read_cycle_counter @@ -263,43 +263,43 @@ module Intrinsics end macro countleading8(src, zero_is_undef) - LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) end macro countleading16(src, zero_is_undef) - LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) end macro countleading32(src, zero_is_undef) - LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) end macro countleading64(src, zero_is_undef) - LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) end macro countleading128(src, zero_is_undef) - LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) end macro counttrailing8(src, zero_is_undef) - LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) end macro counttrailing16(src, zero_is_undef) - LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) end macro counttrailing32(src, zero_is_undef) - LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) end macro counttrailing64(src, zero_is_undef) - LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) end macro counttrailing128(src, zero_is_undef) - LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) end def self.fshl8(a, b, count) : UInt8 @@ -343,14 +343,14 @@ module Intrinsics end macro va_start(ap) - LibIntrinsics.va_start({{ap}}) + ::LibIntrinsics.va_start({{ap}}) end macro va_end(ap) - LibIntrinsics.va_end({{ap}}) + ::LibIntrinsics.va_end({{ap}}) end end macro debugger - Intrinsics.debugtrap + ::Intrinsics.debugtrap end diff --git a/src/json/serialization.cr b/src/json/serialization.cr index b1eb86d15082..15d948f02f40 100644 --- a/src/json/serialization.cr +++ b/src/json/serialization.cr @@ -164,7 +164,7 @@ module JSON private def self.new_from_json_pull_parser(pull : ::JSON::PullParser) instance = allocate instance.initialize(__pull_for_json_serializable: pull) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -422,8 +422,8 @@ module JSON # Try to find the discriminator while also getting the raw # string value of the parsed JSON, so then we can pass it # to the final type. - json = String.build do |io| - JSON.build(io) do |builder| + json = ::String.build do |io| + ::JSON.build(io) do |builder| builder.start_object pull.read_object do |key| if key == {{field.id.stringify}} diff --git a/src/number.cr b/src/number.cr index f7c82aa4cded..9d955c065df3 100644 --- a/src/number.cr +++ b/src/number.cr @@ -59,7 +59,7 @@ struct Number # :nodoc: macro expand_div(rhs_types, result_type) {% for rhs in rhs_types %} - @[AlwaysInline] + @[::AlwaysInline] def /(other : {{rhs}}) : {{result_type}} {{result_type}}.new(self) / {{result_type}}.new(other) end @@ -84,7 +84,7 @@ struct Number # [1, 2, 3, 4] of Int64 # : Array(Int64) # ``` macro [](*nums) - Array({{@type}}).build({{nums.size}}) do |%buffer| + ::Array({{@type}}).build({{nums.size}}) do |%buffer| {% for num, i in nums %} %buffer[{{i}}] = {{@type}}.new({{num}}) {% end %} @@ -113,7 +113,7 @@ struct Number # Slice[1_i64, 2_i64, 3_i64, 4_i64] # : Slice(Int64) # ``` macro slice(*nums, read_only = false) - %slice = Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) + %slice = ::Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) {% for num, i in nums %} %slice.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} @@ -139,7 +139,7 @@ struct Number # StaticArray[1_i64, 2_i64, 3_i64, 4_i64] # : StaticArray(Int64) # ``` macro static_array(*nums) - %array = uninitialized StaticArray({{@type}}, {{nums.size}}) + %array = uninitialized ::StaticArray({{@type}}, {{nums.size}}) {% for num, i in nums %} %array.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} diff --git a/src/object.cr b/src/object.cr index ba818ac2979e..800736687788 100644 --- a/src/object.cr +++ b/src/object.cr @@ -562,7 +562,7 @@ class Object def {{method_prefix}}\{{name.var.id}} : \{{name.type}} if (value = {{var_prefix}}\{{name.var.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") else value end @@ -574,7 +574,7 @@ class Object def {{method_prefix}}\{{name.id}} if (value = {{var_prefix}}\{{name.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") else value end @@ -1293,7 +1293,7 @@ class Object # wrapper.capitalize # => "Hello" # ``` macro delegate(*methods, to object) - {% if compare_versions(Crystal::VERSION, "1.12.0-dev") >= 0 %} + {% if compare_versions(::Crystal::VERSION, "1.12.0-dev") >= 0 %} {% eq_operators = %w(<= >= == != []= ===) %} {% for method in methods %} {% if method.id.ends_with?('=') && !eq_operators.includes?(method.id.stringify) %} @@ -1427,18 +1427,18 @@ class Object macro def_clone # Returns a copy of `self` with all instance variables cloned. def clone - \{% if @type < Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} + \{% if @type < ::Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} exec_recursive_clone do |hash| clone = \{{@type}}.allocate hash[object_id] = clone.object_id clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone end \{% else %} clone = \{{@type}}.allocate clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone \{% end %} end diff --git a/src/slice.cr b/src/slice.cr index 196a29a768dd..087679d37cb7 100644 --- a/src/slice.cr +++ b/src/slice.cr @@ -34,14 +34,14 @@ struct Slice(T) macro [](*args, read_only = false) # TODO: there should be a better way to check this, probably # asking if @type was instantiated or if T is defined - {% if @type.name != "Slice(T)" && T < Number %} + {% if @type.name != "Slice(T)" && T < ::Number %} {{T}}.slice({{args.splat(", ")}}read_only: {{read_only}}) {% else %} - %ptr = Pointer(typeof({{args.splat}})).malloc({{args.size}}) + %ptr = ::Pointer(typeof({{args.splat}})).malloc({{args.size}}) {% for arg, i in args %} %ptr[{{i}}] = {{arg}} {% end %} - Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) + ::Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) {% end %} end diff --git a/src/spec/dsl.cr b/src/spec/dsl.cr index 578076b86d69..d712aa59da4f 100644 --- a/src/spec/dsl.cr +++ b/src/spec/dsl.cr @@ -298,8 +298,8 @@ module Spec # If the "log" module is required it is configured to emit no entries by default. def log_setup defined?(::Log) do - if Log.responds_to?(:setup) - Log.setup_from_env(default_level: :none) + if ::Log.responds_to?(:setup) + ::Log.setup_from_env(default_level: :none) end end end diff --git a/src/spec/helpers/iterate.cr b/src/spec/helpers/iterate.cr index be302ebb49c2..7a70f83408ca 100644 --- a/src/spec/helpers/iterate.cr +++ b/src/spec/helpers/iterate.cr @@ -47,7 +47,7 @@ module Spec::Methods # See `.it_iterates` for details. macro assert_iterates_yielding(expected, method, *, infinite = false, tuple = false) %remaining = ({{expected}}).size - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) {{ method.id }} do |{% if tuple %}*{% end %}x| if %remaining == 0 if {{ infinite }} @@ -73,11 +73,11 @@ module Spec::Methods # # See `.it_iterates` for details. macro assert_iterates_iterator(expected, method, *, infinite = false) - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) %iter = {{ method.id }} ({{ expected }}).size.times do %v = %iter.next - if %v.is_a?(Iterator::Stop) + if %v.is_a?(::Iterator::Stop) # Compare the actual value directly. Since there are less # then expected values, the expectation will fail and raise. %ary.should eq({{ expected }}) @@ -86,7 +86,7 @@ module Spec::Methods %ary << %v end unless {{ infinite }} - %iter.next.should be_a(Iterator::Stop) + %iter.next.should be_a(::Iterator::Stop) end %ary.should eq({{ expected }}) diff --git a/src/static_array.cr b/src/static_array.cr index 2c09e21df166..3d00705bc21a 100644 --- a/src/static_array.cr +++ b/src/static_array.cr @@ -50,7 +50,7 @@ struct StaticArray(T, N) # * `Number.static_array` is a convenient alternative for designating a # specific numerical item type. macro [](*args) - %array = uninitialized StaticArray(typeof({{args.splat}}), {{args.size}}) + %array = uninitialized ::StaticArray(typeof({{args.splat}}), {{args.size}}) {% for arg, i in args %} %array.to_unsafe[{{i}}] = {{arg}} {% end %} diff --git a/src/syscall/aarch64-linux.cr b/src/syscall/aarch64-linux.cr index 5a61e8e7eed8..77b891fe2a7c 100644 --- a/src/syscall/aarch64-linux.cr +++ b/src/syscall/aarch64-linux.cr @@ -334,7 +334,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/arm-linux.cr b/src/syscall/arm-linux.cr index 97119fc4b3f3..da349dd45301 100644 --- a/src/syscall/arm-linux.cr +++ b/src/syscall/arm-linux.cr @@ -409,7 +409,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/i386-linux.cr b/src/syscall/i386-linux.cr index 843b2d1fd856..a0f94a51160a 100644 --- a/src/syscall/i386-linux.cr +++ b/src/syscall/i386-linux.cr @@ -445,7 +445,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/x86_64-linux.cr b/src/syscall/x86_64-linux.cr index 1f01c9226658..5a63b6ee2e1a 100644 --- a/src/syscall/x86_64-linux.cr +++ b/src/syscall/x86_64-linux.cr @@ -368,7 +368,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/yaml/serialization.cr b/src/yaml/serialization.cr index d5fae8dfe9c0..4a1521469dea 100644 --- a/src/yaml/serialization.cr +++ b/src/yaml/serialization.cr @@ -156,11 +156,11 @@ module YAML # Define a `new` directly in the included type, # so it overloads well with other possible initializes - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end - private def self.new_from_yaml_node(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + private def self.new_from_yaml_node(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, self) do |obj| return obj end @@ -170,7 +170,7 @@ module YAML ctx.record_anchor(node, instance) instance.initialize(__context_for_yaml_serializable: ctx, __node_for_yaml_serializable: node) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -178,7 +178,7 @@ module YAML # so it can compete with other possible initializes macro inherited - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end end @@ -409,17 +409,17 @@ module YAML {% mapping.raise "Mapping argument must be a HashLiteral or a NamedTupleLiteral, not #{mapping.class_name.id}" %} {% end %} - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, \{{@type}}) do |obj| return obj end - unless node.is_a?(YAML::Nodes::Mapping) + unless node.is_a?(::YAML::Nodes::Mapping) node.raise "Expected YAML mapping, not #{node.class}" end node.each do |key, value| - next unless key.is_a?(YAML::Nodes::Scalar) && value.is_a?(YAML::Nodes::Scalar) + next unless key.is_a?(::YAML::Nodes::Scalar) && value.is_a?(::YAML::Nodes::Scalar) next unless key.value == {{field.id.stringify}} discriminator_value = value.value From 0f257ef57bdf34f156dd7bb5bcb0624258eb07cb Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Mon, 16 Sep 2024 19:56:21 +0800 Subject: [PATCH 3/5] Fix `Process.exec` stream redirection on Windows (#14986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following should print the compiler help message to the file `foo.txt`: ```crystal File.open("foo.txt", "w") do |f| Process.exec("crystal", output: f) end ``` It used to work on Windows in Crystal 1.12, but is now broken since 1.13. This is because `LibC._wexecvp` only inherits file descriptors in the C runtime, not arbitrary Win32 file handles; since we stopped calling `LibC._open_osfhandle`, the C runtime knows nothing about any reopened standard streams in Win32. Thus the above merely prints the help message to the standard output. This PR creates the missing C file descriptors right before `LibC._wexecvp`. It also fixes a different regression of #14947 where reconfiguring `STDIN.blocking` always fails. Co-authored-by: Johannes Müller --- spec/std/process_spec.cr | 21 ++++++++++ src/crystal/system/win32/process.cr | 59 +++++++++++++++++++-------- src/lib_c/x86_64-windows-msvc/c/io.cr | 5 ++- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index f067d2f5c775..f1437656fffa 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -465,6 +465,27 @@ pending_interpreted describe: Process do {% end %} describe ".exec" do + it "redirects STDIN and STDOUT to files", tags: %w[slow] do + with_tempfile("crystal-exec-stdin", "crystal-exec-stdout") do |stdin_path, stdout_path| + File.write(stdin_path, "foobar") + + status, _, _ = compile_and_run_source <<-CRYSTAL + command = #{stdin_to_stdout_command[0].inspect} + args = #{stdin_to_stdout_command[1].to_a} of String + stdin_path = #{stdin_path.inspect} + stdout_path = #{stdout_path.inspect} + File.open(stdin_path) do |input| + File.open(stdout_path, "w") do |output| + Process.exec(command, args, input: input, output: output) + end + end + CRYSTAL + + status.success?.should be_true + File.read(stdout_path).chomp.should eq("foobar") + end + end + it "gets error from exec" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do Process.exec("foobarbaz") diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 05b2ea36584e..1817be9ee27f 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -326,9 +326,9 @@ struct Crystal::System::Process end private def self.try_replace(command_args, env, clear_env, input, output, error, chdir) - reopen_io(input, ORIGINAL_STDIN) - reopen_io(output, ORIGINAL_STDOUT) - reopen_io(error, ORIGINAL_STDERR) + old_input_fd = reopen_io(input, ORIGINAL_STDIN) + old_output_fd = reopen_io(output, ORIGINAL_STDOUT) + old_error_fd = reopen_io(error, ORIGINAL_STDERR) ENV.clear if clear_env env.try &.each do |key, val| @@ -351,11 +351,18 @@ struct Crystal::System::Process argv << Pointer(LibC::WCHAR).null LibC._wexecvp(command, argv) + + # exec failed; restore the original C runtime file descriptors + errno = Errno.value + LibC._dup2(old_input_fd, 0) + LibC._dup2(old_output_fd, 1) + LibC._dup2(old_error_fd, 2) + errno end def self.replace(command_args, env, clear_env, input, output, error, chdir) : NoReturn - try_replace(command_args, env, clear_env, input, output, error, chdir) - raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0]) + errno = try_replace(command_args, env, clear_env, input, output, error, chdir) + raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0], errno) end private def self.raise_exception_from_errno(command, errno = Errno.value) @@ -367,21 +374,41 @@ struct Crystal::System::Process end end + # Replaces the C standard streams' file descriptors, not Win32's, since + # `try_replace` uses the C `LibC._wexecvp` and only cares about the former. + # Returns a duplicate of the original file descriptor private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor) - src_io = to_real_fd(src_io) + unless src_io.system_blocking? + raise IO::Error.new("Non-blocking streams are not supported in `Process.exec`", target: src_io) + end - dst_io.reopen(src_io) - dst_io.blocking = true - dst_io.close_on_exec = false - end + src_fd = + case src_io + when STDIN then 0 + when STDOUT then 1 + when STDERR then 2 + else + LibC._open_osfhandle(src_io.windows_handle, 0) + end - private def self.to_real_fd(fd : IO::FileDescriptor) - case fd - when STDIN then ORIGINAL_STDIN - when STDOUT then ORIGINAL_STDOUT - when STDERR then ORIGINAL_STDERR - else fd + dst_fd = + case dst_io + when ORIGINAL_STDIN then 0 + when ORIGINAL_STDOUT then 1 + when ORIGINAL_STDERR then 2 + else + raise "BUG: Invalid destination IO" + end + + return src_fd if dst_fd == src_fd + + orig_src_fd = LibC._dup(src_fd) + + if LibC._dup2(src_fd, dst_fd) == -1 + raise IO::Error.from_errno("Failed to replace C file descriptor", target: dst_io) end + + orig_src_fd end def self.chroot(path) diff --git a/src/lib_c/x86_64-windows-msvc/c/io.cr b/src/lib_c/x86_64-windows-msvc/c/io.cr index 75da8c18e5b9..ccbaa15f2d1b 100644 --- a/src/lib_c/x86_64-windows-msvc/c/io.cr +++ b/src/lib_c/x86_64-windows-msvc/c/io.cr @@ -2,12 +2,13 @@ require "c/stdint" lib LibC fun _wexecvp(cmdname : WCHAR*, argv : WCHAR**) : IntPtrT + fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int + fun _dup(fd : Int) : Int + fun _dup2(fd1 : Int, fd2 : Int) : Int # unused - fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int fun _get_osfhandle(fd : Int) : IntPtrT fun _close(fd : Int) : Int - fun _dup2(fd1 : Int, fd2 : Int) : Int fun _isatty(fd : Int) : Int fun _write(fd : Int, buffer : UInt8*, count : UInt) : Int fun _read(fd : Int, buffer : UInt8*, count : UInt) : Int From ced75401dac79f9abfe77e2aa6181a62c1283107 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 25 Aug 2024 19:15:11 +0800 Subject: [PATCH 4/5] Fix `String#index` and `#rindex` for `Char::REPLACEMENT` (#14937) If the string consists only of ASCII characters and invalid UTF-8 byte sequences, all the latter should correspond to `Char::REPLACEMENT`, and so `#index` and `#rindex` should detect them, but this was broken since #14461. --- spec/std/string_spec.cr | 6 ++++++ src/string.cr | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/spec/std/string_spec.cr b/spec/std/string_spec.cr index 00310bfcbc47..9c4f3cfc5d12 100644 --- a/spec/std/string_spec.cr +++ b/spec/std/string_spec.cr @@ -945,6 +945,7 @@ describe "String" do it { "日本語".index('本').should eq(1) } it { "bar".index('あ').should be_nil } it { "あいう_えお".index('_').should eq(3) } + it { "xyz\xFFxyz".index('\u{FFFD}').should eq(3) } describe "with offset" do it { "foobarbaz".index('a', 5).should eq(7) } @@ -952,6 +953,8 @@ describe "String" do it { "foo".index('g', 1).should be_nil } it { "foo".index('g', -20).should be_nil } it { "日本語日本語".index('本', 2).should eq(4) } + it { "xyz\xFFxyz".index('\u{FFFD}', 2).should eq(3) } + it { "xyz\xFFxyz".index('\u{FFFD}', 4).should be_nil } # Check offset type it { "foobarbaz".index('a', 5_i64).should eq(7) } @@ -1094,6 +1097,7 @@ describe "String" do it { "foobar".rindex('g').should be_nil } it { "日本語日本語".rindex('本').should eq(4) } it { "あいう_えお".rindex('_').should eq(3) } + it { "xyz\xFFxyz".rindex('\u{FFFD}').should eq(3) } describe "with offset" do it { "bbbb".rindex('b', 2).should eq(2) } @@ -1106,6 +1110,8 @@ describe "String" do it { "faobar".rindex('a', 3).should eq(1) } it { "faobarbaz".rindex('a', -3).should eq(4) } it { "日本語日本語".rindex('本', 3).should eq(1) } + it { "xyz\xFFxyz".rindex('\u{FFFD}', 4).should eq(3) } + it { "xyz\xFFxyz".rindex('\u{FFFD}', 2).should be_nil } # Check offset type it { "bbbb".rindex('b', 2_i64).should eq(2) } diff --git a/src/string.cr b/src/string.cr index d3bc7d6998b2..ab107c454e8c 100644 --- a/src/string.cr +++ b/src/string.cr @@ -3335,11 +3335,21 @@ class String def index(search : Char, offset = 0) : Int32? # If it's ASCII we can delegate to slice if single_byte_optimizable? - # With `single_byte_optimizable?` there are only ASCII characters and invalid UTF-8 byte - # sequences and we can immediately reject any non-ASCII codepoint. - return unless search.ascii? + # With `single_byte_optimizable?` there are only ASCII characters and + # invalid UTF-8 byte sequences, and we can reject anything that is neither + # ASCII nor the replacement character. + case search + when .ascii? + return to_slice.fast_index(search.ord.to_u8!, offset) + when Char::REPLACEMENT + offset.upto(bytesize - 1) do |i| + if to_unsafe[i] >= 0x80 + return i.to_i + end + end + end - return to_slice.fast_index(search.ord.to_u8, offset) + return nil end offset += size if offset < 0 @@ -3455,11 +3465,21 @@ class String def rindex(search : Char, offset = size - 1) # If it's ASCII we can delegate to slice if single_byte_optimizable? - # With `single_byte_optimizable?` there are only ASCII characters and invalid UTF-8 byte - # sequences and we can immediately reject any non-ASCII codepoint. - return unless search.ascii? + # With `single_byte_optimizable?` there are only ASCII characters and + # invalid UTF-8 byte sequences, and we can reject anything that is neither + # ASCII nor the replacement character. + case search + when .ascii? + return to_slice.rindex(search.ord.to_u8!, offset) + when Char::REPLACEMENT + offset.downto(0) do |i| + if to_unsafe[i] >= 0x80 + return i.to_i + end + end + end - return to_slice.rindex(search.ord.to_u8, offset) + return nil end offset += size if offset < 0 From d14d04562125ee1f2c3985d756c5f0e4cd68a9c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 18 Sep 2024 15:43:39 +0200 Subject: [PATCH 5/5] Changelog for 1.13.3 (#14991) --- CHANGELOG.md | 24 ++++++++++++++++++++++++ shard.yml | 2 +- src/SOURCE_DATE_EPOCH | 2 +- src/VERSION | 2 +- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f97d0bedeb1b..341586a8fb95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +## [1.13.3] (2024-09-18) + +[1.13.3]: https://github.com/crystal-lang/crystal/releases/1.13.3 + +### Bugfixes + +#### stdlib + +- **[regression]** Fix use global paths in macro bodies ([#14965], thanks @straight-shoota) +- *(system)* **[regression]** Fix `Process.exec` stream redirection on Windows ([#14986], thanks @HertzDevil) +- *(text)* **[regression]** Fix `String#index` and `#rindex` for `Char::REPLACEMENT` ([#14937], thanks @HertzDevil) + +[#14965]: https://github.com/crystal-lang/crystal/pull/14965 +[#14986]: https://github.com/crystal-lang/crystal/pull/14986 +[#14937]: https://github.com/crystal-lang/crystal/pull/14937 + +### Infrastructure + +- Changelog for 1.13.3 ([#14991], thanks @straight-shoota) +- *(ci)* Enable runners from `runs-on.com` for Aarch64 CI ([#15007], thanks @straight-shoota) + +[#14991]: https://github.com/crystal-lang/crystal/pull/14991 +[#15007]: https://github.com/crystal-lang/crystal/pull/15007 + ## [1.13.2] (2024-08-20) [1.13.2]: https://github.com/crystal-lang/crystal/releases/1.13.2 diff --git a/shard.yml b/shard.yml index 0dd8c2abf3a1..6463a5681c65 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: crystal -version: 1.13.2 +version: 1.13.3 authors: - Crystal Core Team diff --git a/src/SOURCE_DATE_EPOCH b/src/SOURCE_DATE_EPOCH index 0ea6bd82d669..13a79f624e9d 100644 --- a/src/SOURCE_DATE_EPOCH +++ b/src/SOURCE_DATE_EPOCH @@ -1 +1 @@ -1724112000 +1726617600 diff --git a/src/VERSION b/src/VERSION index 61ce01b30118..01b7568230eb 100644 --- a/src/VERSION +++ b/src/VERSION @@ -1 +1 @@ -1.13.2 +1.13.3