Feature #13570
closedUsing mkmf for ruby/spec C API specs
Description
Hello all,
I am thinking to use mkmf to compile the C API specs.
https://github.com/ruby/ruby/blob/trunk/spec/rubyspec/optional/capi/spec_helper.rb
is getting pretty complex and hard to maintain.
I have a few questions:
- Does mkmf works well on Windows?
- What is a good way to compile a single .c file with mkmf to a given library file in another directory?
I tried this but I am not sure it's correct:
def compile_extension(name)
objdir = object_path
ext = "#{name}_spec"
lib = "#{objdir}/#{ext}.#{RbConfig::CONFIG['DLEXT']}"
require 'mkmf' # TODO: probably best to use a subprocess to avoid polluting the namespace
Dir.chdir(objdir) do
$srcs = ["#{extension_path}/#{ext}.c"]
$objs = ["#{extension_path}/#{ext}.o"] # should probably be in objdir but that does not seem to work
create_makefile(ext)
system "make"
end
lib
end
Alternatively, we can copy the needed files to a temporary directory, build there and copy the shared library back.
It's a bit more work but not a big deal either.
Files
Updated by MSP-Greg (Greg L) over 7 years ago
Does mkmf works well on Windows?
I'm not a c type. With MinGW builds, I've got 89 mkmf.log files in the build dir, and test-all results are consistent and reasonable low. I'm guessing that means yes.
FYI, I ran test-spec with -j on 58760, and it matched (or very closely matched) results without -j. test-all has the same results, but assertions and skips are different.
Updated by Eregon (Benoit Daloze) over 7 years ago
- Status changed from Open to Closed
I made the change in https://github.com/ruby/spec/commit/84ea66ef61424ef87d03658cbc140d4b1af17c22
which was imported in r58939.
Updated by Eregon (Benoit Daloze) over 7 years ago
- File spec_helper.rb spec_helper.rb added
I attach here the current version of the spec_helper.rb doing the compilation of extensions.
It seems to now work correctly for out-of-source builds,
and with an extra hack uses the right ruby in the Makefile
(RbConfig.ruby in ./rbconfig.rb is just incorrect for built but not installed ruby).
I found very confusing the effect that $extmf can have,
I wished MRI used a simpler mechanism to detect if ruby was installed or not.
Is it so important nowadays to be able to test without "make install"?
Could I ask some help for making this run correctly on Windows?
I would like if possible something not too hacky.
The old way to compile extensions was un-maintainable due to its complexity.
Updated by Eregon (Benoit Daloze) over 7 years ago
- Status changed from Closed to Assigned
- Assignee set to windows
Updated by MSP-Greg (Greg L) over 7 years ago
Benoit,
Sorry, never set this to 'watch'. I'll try a build with MinGW.
FWIW, I just got one of the spec c files to compile with only the following --
require 'mkmf'
Dir.chdir('src/build-x86_64/spec/rubyspec/optional/capi/temp') { |d|
append_ldflags('-L /e/GitHub/ruby-loco/src/build-x86_64')
create_makefile("bignum_spec")
puts `make`
require_relative 'src/build-x86_64/spec/rubyspec/optional/capi/temp/bignum_spec.so'
puts CApiBignumSpecs.instance_methods(false)
}
Output:
E:\GitHub\ruby-loco>ruby make_test.rb
checking for whether -L /e/GitHub/ruby-loco/src/build-x86_64 is accepted as LDFLAGS... yes
creating Makefile
generating bignum_spec-x64-mingw32.def
compiling bignum_spec.c
linking shared-object bignum_spec.so
rb_big_pack_array
rb_big_pack_length
rb_big2dbl
rb_dbl2big
rb_big2ll
rb_big2long
rb_big2str
rb_big2ulong
rb_big_cmp
rb_big_pack
For the test, I only copied bignum_spec.c
to the temp folder, along with rubyspec.h
. I'm sure there's a setting to pick that up in mkmf.rb
, along with several more that I'm probably missing. Again, I'm not a c type...
Updated by MSP-Greg (Greg L) over 7 years ago
Benoit,
After screwing around with quite a bit of make code, I found the (simple) issue. The code in Object.cp does not properly copy binary files on Windows.
Hence, since we know the src
and dest
are on the same partition/drive, I used File.rename
in spec_helper.rb
. Below is a diff/patch file:
--- spec_helper_orig.rb Mon May 29 13:34:22 2017
+++ spec_helper.rb Wed May 31 15:47:17 2017
@@ -18,11 +18,7 @@
def compile_extension(name)
debug = false
- run_mkmf_in_process = false
-
- if RUBY_NAME == 'truffleruby'
- run_mkmf_in_process = true
- end
+ run_mkmf_in_process = RUBY_NAME == 'truffleruby'
ext = "#{name}_spec"
lib = "#{object_path}/#{ext}.#{RbConfig::CONFIG['DLEXT']}"
@@ -61,7 +57,7 @@
raise "make failed:\n#{output}" unless $?.success?
$stderr.puts output if debug
- cp File.basename(lib), lib
+ File.rename File.basename(lib), lib
end
ensure
rm_r tmpdir unless debug
Tests run with -j
both via make test-spec
and running mspec
. Results of both are at Ruby MinGW Test Results.
Thanks for the work.
Updated by hsbt (Hiroshi SHIBATA) over 7 years ago
- Related to Bug #13611: MinGW spec/rubyspec/optional/capi compile/link issues added
Updated by Eregon (Benoit Daloze) over 7 years ago
MSP-Greg (Greg L) wrote:
Benoit,
After screwing around with quite a bit of make code, I found the (simple) issue. The code in Object.cp does not properly copy binary files on Windows.
Oh, wow, thanks and nice find!
MSpec's #cp should use "rb"/"wb" modes obviously.
Would it work with that?
Hence, since we know the
src
anddest
are on the same partition/drive, I usedFile.rename
inspec_helper.rb
. Below is a diff/patch file:
I am not sure that's always the case, but thanks for the patch, I will try it.
Tests run with
-j
both viamake test-spec
and runningmspec
. Results of both are at Ruby MinGW Test Results.
Great!
Updated by MSP-Greg (Greg L) over 7 years ago
I checked on MinGW, and there is not an issue with File.rename
with src
and dest
being on different drives. Where I got that from, I don't know...
Hence, on MinGW/Windows, rename will work in all situations.
MSpec's #cp should use "rb"/"wb" modes obviously.
Would it work with that?
Yes and haven't tried. Seems odd to copy a file when a rename will do...
Updated by Eregon (Benoit Daloze) over 7 years ago
MSP-Greg (Greg L) wrote:
I checked on MinGW, and there is not an issue with
File.rename
withsrc
anddest
being on different drives. Where I got that from, I don't know...
It's the semantics of the rename(2) syscall on Linux, which throws EXDEV if oldpath and newpath are not on the same mounted filesystem.
And indeed, on Linux:
> File.write "foo","abc"
> File.rename "foo", "/tmp/bla"
Errno::EXDEV: Invalid cross-device link @ rb_file_s_rename - (foo, /tmp/bla)
So we would need some special handling to move files across filesystems on non-Windows, or use FileUtils.mv
MSpec's #cp should use "rb"/"wb" modes obviously.
Would it work with that?Yes and haven't tried. Seems odd to copy a file when a rename will do...
Indeed, moving sounds nicer in principle.
The reason I like the copy is to help debugging, the .so is still in tmpdir used for compilation along other files.
But it's a small thing, looking for the moved file is fine as well.
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
Eregon (Benoit Daloze) wrote:
MSpec's #cp should use "rb"/"wb" modes obviously.
Why not IO.copy_stream
?
Updated by Eregon (Benoit Daloze) over 7 years ago
Sounds good, I did not know IO.copy_stream can be used with 2 path Strings as well. It's a bit of a strange name for "cp".
But I think I slightly prefer the current implementation, it uses only well-known basic functionality such as File.open+read/write.
I would guess early Ruby implementations don't implement IO.copy_stream.
Updated by Eregon (Benoit Daloze) over 7 years ago
- File spec_helper.rb spec_helper.rb added
Here is a version of the spec_helper.rb working on MRI 2.2 to trunk on Linux (and I would expect macOS as well).
@greg (Grégory Duchatelet) and @nobu (Nobuyoshi Nakada):
Could you test it on Windows?
Then should I commit it?
Or do we have a way to test CI (Travis + rubyci.org) before committing?
Updated by MSP-Greg (Greg L) over 7 years ago
I'll try a MinGW build shortly.
Re IO.copy_stream
, I believe it has existed since Ruby 1.9.3, which should be far enough back for current test code. Also, I've only paid attention to the files copied for capi, but all are small enough that I'd prefer one function call to the call in fs.rb that creates two blocks in ruby, then loops (in ruby) 300+ times for the copy.
I patched both fs.rb and spec_helper.rb with IO.copy_stream
, and both ran successfully.
I'll start the build.
Updated by MSP-Greg (Greg L) over 7 years ago
Ran a MinGW build on ruby 2.5.0dev 2017-06-02 58998
. Ran tests three ways:
-
make test-spec
- pkg/install bin folder in path -
make test-spec
- pkg/install bin folder not in path -
mspec
- pkg/install bin folder in path
All three ran with expected results. Although I'd prefer that IO.copy_stream
be used, neither fs.rb
or spec_helper.rb
were patched to use it.
Hence, good job and thanks...
Updated by MSP-Greg (Greg L) over 7 years ago
Eregon (Benoit Daloze) wrote:
Or do we have a way to test CI (Travis + rubyci.org) before committing?
As you know, Travis will run on a PR.
A few people seem to state that trunk builds and tests fine on mswin, but Appveyor doesn't run test-all or test-spec/test-rubyspec. I'd be interested to know why...
Updated by usa (Usaku NAKAMURA) over 7 years ago
MSP-Greg (Greg L) wrote:
A few people seem to state that trunk builds and tests fine on mswin, but Appveyor doesn't run test-all or test-spec/test-rubyspec. I'd be interested to know why...
Our hero nobu is testing running test-all and test-spec on appveyor.
I've heard that there was a little problem, but I believe that he will merge it into trunk in near future.
Updated by nobu (Nobuyoshi Nakada) over 7 years ago
test-spec runs fine on AppVeyor, test/ruby/test_rubyoptions.rb has a failure when locale and filesystem encodings differ.
Another problem is that it consumes 15 min, vs 8 min without tests.
Updated by MSP-Greg (Greg L) over 7 years ago
nobu (Nobuyoshi Nakada) wrote:
test-spec runs fine on AppVeyor, test/ruby/test_rubyoptions.rb has a failure when locale and filesystem encodings differ.
Another problem is that it consumes 15 min, vs 8 min without tests.
Thank you for working on that (in addition to everything else you do).
MinGW has the same failure, as discussed previously. In my test results log, failures jumped from 5 to 6. That's when I decided to remove the chcp and RUBYOPT changes from my build code.
Re my MinGW test-all, I guess my goals were/are:
- It completes
- Consistent failures/errors
- Minimize skips
- Consistent tests/assertions
- WHY DOES THIS TAKE SO LONG / parallel test
I haven't gotten to 5...
Updated by Eregon (Benoit Daloze) over 7 years ago
I tested and adapted the spec_helper.rb so it now also works on mswin (https://github.com/ruby/ruby/pull/1649) and mingw (on the Travis of ruby/spec).
Therefore I close this with r59093.
Please fix if new failures appear when building with mkmf.
Updated by Eregon (Benoit Daloze) over 7 years ago
- Status changed from Assigned to Closed
Updated by Eregon (Benoit Daloze) almost 6 years ago
@nobu (Nobuyoshi Nakada) cherry-picked the IO.copy_stream patch in r66147, and I now upstreamed it to ruby/mspec.
I mostly forgot about it, and it's easy enough for early Ruby implementations to shim IO.copy_stream.