ruby-core

Download

ruby-core@ml.ruby-lang.org

June 2023

2 participants
202 discussions

[ruby-core:113723] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by ioquatix (Samuel Williams) 01 Jun '23

01 Jun '23

Issue #19057 has been updated by ioquatix (Samuel Williams). Unfortunately this was reverted due to some extensions no longer compiling. - https://bugs.ruby-lang.org/issues/19704 I think it's expected that some dependencies using internal details of `rb_io_t` should be fixed. ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-103373 * Author: ioquatix (Samuel Williams) * Status: Closed * Priority: Normal * Assignee: ioquatix (Samuel Williams) ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:113720] [Ruby master Bug#19704] Unable to install readline-ext since 18e55fc1e1ec20e8f3166e3059e76c885fc9f8f2
by yahonda (Yasuo Honda) 01 Jun '23

01 Jun '23

Issue #19704 has been reported by yahonda (Yasuo Honda). ---------------------------------------- Bug #19704: Unable to install readline-ext since 18e55fc1e1ec20e8f3166e3059e76c885fc9f8f2 https://bugs.ruby-lang.org/issues/19704 * Author: yahonda (Yasuo Honda) * Status: Open * Priority: Normal * ruby -v: ruby 3.3.0dev (2023-05-30T01:02:40Z master 18e55fc1e1) [x86_64-linux] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Rails CI against Ruby 3.3.0dev fails to install readline-ext recenty. https://buildkite.com/rails/rails/builds/96780#01886e3f-eaef-405c-b58a-e90c… According to git bisect, it is triggered by 18e55fc1e1ec20e8f3166e3059e76c885fc9f8f2 ### Steps to reproduce Install ruby 3.3.0dev gem install readline-ext ### Expected behavior It should install readline-ext as the previous commit of Ruby 7ddcd0622f does. ``` $ ruby -v ruby 3.3.0dev (2023-05-29T21:24:22Z master 7ddcd0622f) [x86_64-linux] $ gem install readline-ext Building native extensions. This could take a while... Successfully installed readline-ext-0.1.5 Ignoring rbs-3.0.4 because its extensions are not built. Try: gem pristine rbs --version 3.0.4 Parsing documentation for readline-ext-0.1.5 Installing ri documentation for readline-ext-0.1.5 Done installing documentation for readline-ext after 0 seconds 1 gem installed $ ``` ### Actual result ```ruby $ ruby -v ruby 3.3.0dev (2023-05-30T01:02:40Z master 18e55fc1e1) [x86_64-linux] $ gem install readline-ext Building native extensions. This could take a while... ERROR: Error installing readline-ext: ERROR: Failed to build gem native extension. current directory: /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.3.0+0/gems/readline-ext-0.1.5/ext/readline /home/yahonda/.rbenv/versions/trunk/bin/ruby extconf.rb checking for tgetnum() in -lncurses... yes checking for readline/readline.h... yes checking for readline/history.h... yes checking for readline() in -lreadline... yes checking for rl_getc() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_getc_function() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_filename_completion_function() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_username_completion_function() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_completion_matches() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_refresh_line() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_deprep_term_function in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_completion_append_character in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_completion_quote_character in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_basic_word_break_characters in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_completer_word_break_characters in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_basic_quote_characters in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_completer_quote_characters in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_filename_quote_characters in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_attempted_completion_over in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_library_version in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_editing_mode in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_line_buffer in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_point in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_char_is_quoted_p in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_event_hook in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_catch_sigwinch in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_catch_signals in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_pre_input_hook in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_special_prefixes in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_cleanup_after_signal() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_free_line_state() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_clear_signals() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_set_screen_size() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_get_screen_size() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_vi_editing_mode() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_emacs_editing_mode() in stdio.h,readline/readline.h,readline/history.h... yes checking for replace_history_entry() in stdio.h,readline/readline.h,readline/history.h... yes checking for remove_history() in stdio.h,readline/readline.h,readline/history.h... yes checking for clear_history() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_redisplay() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_insert_text() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_delete_text() in stdio.h,readline/readline.h,readline/history.h... yes checking for rl_hook_func_t* in stdio.h,readline/readline.h,readline/history.h... yes creating Makefile current directory: /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.3.0+0/gems/readline-ext-0.1.5/ext/readline make DESTDIR\= sitearchdir\=./.gem.20230601-64629-eyixt9 sitelibdir\=./.gem.20230601-64629-eyixt9 clean current directory: /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.3.0+0/gems/readline-ext-0.1.5/ext/readline make DESTDIR\= sitearchdir\=./.gem.20230601-64629-eyixt9 sitelibdir\=./.gem.20230601-64629-eyixt9 compiling readline.c readline.c: In function ‘prepare_readline’: readline.c:386:16: error: invalid use of incomplete typedef ‘rb_io_t’ {aka ‘struct rb_io’} 386 | if (ifp->fd < 0) { | ^~ readline.c:395:16: error: invalid use of incomplete typedef ‘rb_io_t’ {aka ‘struct rb_io’} 395 | if (ofp->fd < 0) { | ^~ readline.c: In function ‘readline_s_set_input’: readline.c:565:32: error: invalid use of incomplete typedef ‘rb_io_t’ {aka ‘struct rb_io’} 565 | fd = rb_cloexec_dup(ifp->fd); | ^~ readline.c: In function ‘readline_s_set_output’: readline.c:601:32: error: invalid use of incomplete typedef ‘rb_io_t’ {aka ‘struct rb_io’} 601 | fd = rb_cloexec_dup(ofp->fd); | ^~ readline.c: At top level: cc1: note: unrecognized command-line option ‘-Wno-self-assign’ may have been intended to silence earlier diagnostics cc1: note: unrecognized command-line option ‘-Wno-parentheses-equality’ may have been intended to silence earlier diagnostics cc1: note: unrecognized command-line option ‘-Wno-constant-logical-operand’ may have been intended to silence earlier diagnostics make: *** [Makefile:248: readline.o] Error 1 make failed, exit code 2 Gem files will remain installed in /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.3.0+0/gems/readline-ext-0.1.5 for inspection. Results logged to /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.3.0+0/extensions/x86_64-linux/3.3.0+0-static/readline-ext-0.1.5/gem_make.out ``` -- https://bugs.ruby-lang.org/

2 2

Jump to page: