Skip to content

Commit

Permalink
Instead of looking at the ARG_MAX size let the exception occur and ca…
Browse files Browse the repository at this point in the history
…tch it.

This circumvents guessing the usable size of ARG_MAX and will also work on systems where it's not possible to query 'getconf ARG_MAX'. I feel much better about this as it leaves the original flow intact and will only split the work into two if exactly Errno::E2BIG is raised. This should work across Unix/Win/MacOS.
  • Loading branch information
weibel committed Nov 18, 2018
1 parent 4db8864 commit ba4e9b1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 50 deletions.
29 changes: 11 additions & 18 deletions lib/slather/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,18 @@ def pathnames_per_binary(binary_path)
def create_coverage_files_for_binary(binary_path, pathnames_per_binary)
coverage_files = []

# Chunk pathnames_per_binary into segments that do not exceed the OS ARG_MAX size for calls to llvm-cov
# The size is padded with 10% to account for added spaces etc. in the final argument
if pathnames_per_binary.join.size * 1.1 < self.class.max_os_argument_size
begin
coverage_files.concat(create_coverage_files(binary_path, pathnames_per_binary))
elsif pathnames_per_binary.count >=2
# If pathnames_per_binary is too big it's split in two halfs which are then processed independently
left,right = pathnames_per_binary.each_slice( (pathnames_per_binary.size/2.0).round ).to_a
coverage_files.concat(create_coverage_files_for_binary(binary_path, left))
coverage_files.concat(create_coverage_files_for_binary(binary_path, right))
else
raise StandardError, "Errno::E2BIG: Argument list too long. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414"
rescue Errno::E2BIG => e
# pathnames_per_binary is too big for the OS to handle so it's split in two halfs which are processed independently
if pathnames_per_binary.count > 1
left, right = pathnames_per_binary.each_slice( (pathnames_per_binary.size/2.0).round ).to_a
coverage_files.concat(create_coverage_files_for_binary(binary_path, left))
coverage_files.concat(create_coverage_files_for_binary(binary_path, right))
else
# pathnames_per_binary contains one element which is too big for the OS to handle.
raise e, "#{e}. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414", e.backtrace
end
end

coverage_files
Expand Down Expand Up @@ -297,14 +298,6 @@ def self.yml
@yml ||= File.exist?(yml_filename) ? YAML.load_file(yml_filename) : {}
end

def self.get_arg_max
@get_arg_max ||= `getconf ARG_MAX`.to_i
end

def self.max_os_argument_size
self.get_arg_max > 0 ? self.get_arg_max : 26214
end

def configure
begin
configure_scheme
Expand Down
42 changes: 10 additions & 32 deletions spec/slather/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,6 @@
end
end

describe "::max_os_argument_size" do
let(:getconf_project) do
Slather::Project.open(FIXTURES_PROJECT_PATH)
end

before(:each) do
getconf_project.instance_variable_set("@get_arg_max", nil)
end

context "getconf ARG_MAX returns a value" do
it "should return a value" do
allow(getconf_project.class).to receive(:get_arg_max).and_return(2000)
expect(getconf_project.class.max_os_argument_size).to eq(2000)
end
end

context "getconf ARG_MAX does not return a value" do
it "should return a value with a sensible default" do
allow(getconf_project.class).to receive(:get_arg_max).and_return(0)
expect(getconf_project.class.max_os_argument_size).to eq(26214)
end
end
end

describe "#coverage_files" do
class SpecCoverageFile < Slather::CoverageFile
end
Expand Down Expand Up @@ -134,22 +110,24 @@ class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
allow(Dir).to receive(:[]).with("#{fixtures_project.build_directory}/**/Coverage.profdata").and_return(["/some/path/Coverage.profdata"])
allow(fixtures_project).to receive(:binary_file).and_return(["Fixtures"])
allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(llvm_cov_export_output)
allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return(profdata_llvm_cov_output)
allow(fixtures_project).to receive(:coverage_file_class).and_return(SpecXcode7CoverageFile)
allow(fixtures_project).to receive(:ignore_list).and_return([])
end

it "Should raise an error when the OS level argument size is too small to work with" do
allow(fixtures_project.class).to receive(:max_os_argument_size).and_return(10)
expect { fixtures_project.send(:profdata_coverage_files) }.to raise_error(StandardError, "Errno::E2BIG: Argument list too long. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414")
it "Should catch Errno::E2BIG and re-raise if the input is too large to split into multiple chunks" do
allow(fixtures_project).to receive(:unsafe_profdata_llvm_cov_output).and_raise(Errno::E2BIG)
expect { fixtures_project.send(:profdata_coverage_files) }.to raise_error(Errno::E2BIG, "Argument list too long. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414")
end

it "Should return Coverage.profdata file objects when the OS level argument size is smaller than the input size" do
allow(fixtures_project.class).to receive(:max_os_argument_size).and_return(llvm_cov_export_output.size/2)
allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return(profdata_llvm_cov_output << profdata_llvm_cov_output)
it "Should catch Errno::E2BIG and return Coverage.profdata file objects when the work can be split into two" do
allow(fixtures_project).to receive(:unsafe_profdata_llvm_cov_output).once {
# raise once and then insert the stub
allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return(profdata_llvm_cov_output)
raise Errno::E2BIG
}
profdata_coverage_files = fixtures_project.send(:profdata_coverage_files)
profdata_coverage_files.each { |cf| expect(cf.kind_of?(SpecXcode7CoverageFile)).to be_truthy }
expect(profdata_coverage_files.map { |cf| cf.source_file_pathname.basename.to_s }).to eq(["Fixtures.swift"])
expect(profdata_coverage_files.map { |cf| cf.source_file_pathname.basename.to_s }).to eq(["Fixtures.swift", "Fixtures.swift"])
end
end

Expand Down

0 comments on commit ba4e9b1

Please sign in to comment.