Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using <internal: for core library methods defined in Ruby #6430

Closed
eregon opened this issue Oct 8, 2020 · 4 comments · Fixed by #7958
Closed

Using <internal: for core library methods defined in Ruby #6430

eregon opened this issue Oct 8, 2020 · 4 comments · Fixed by #7958
Milestone

Comments

@eregon
Copy link
Member

eregon commented Oct 8, 2020

Core library methods' source_location and in backtraces defined with Ruby code look like this:

  • <internal:kernel>:89 on CRuby
  • <internal:core> core/kernel.rb:533 on TruffleRuby
  • uri:classloader:/jruby/kernel/enumerator.rb:243 on JRuby

I would suggest to use a <internal: prefix since this is becoming the de facto standard.
That will also provide a consistent way to filter out core library methods defined in Ruby when desirable.

Also note that both CRuby 3 and TruffleRuby ignore entries starting with <internal: for Kernel#warn(msg, uplevel: n) (details in rubygems/rubygems#3987 (comment)).

@headius
Copy link
Member

headius commented Oct 9, 2020

I am reluctant to use something as meaningless as "internal" since it can't be resolved to a real path by any code. The path we provide with "uri:classloader:" is actually the path that gets required; it indicates clearly where the source is located (as a classloader resource) and the full path to that file within the classloader.

We will consider an appropriate way to add the "internal" identifier without losing the actual classloader path information.

@headius headius added this to the JRuby 9.3.0.0 milestone Oct 9, 2020
@eregon
Copy link
Member Author

eregon commented Oct 9, 2020

What about something like:
<internal:uri:classloader:/jruby/kernel/enumerator.rb:243> or
<internal:core> uri:classloader:/jruby/kernel/enumerator.rb:243?
The only important part for this issue is that it starts with <internal:.

It doesn't seem good to ask 3rd-party libraries which sometimes need to filter out core methods (e.g. for warnings, or for more relevant backtraces for test frameworks) to have various different regexps to skip those, and <internal: seems an easy prefix to standardize on (here is a particularly telling example).

External processes can't understand uri:classloader anyway, and internally File/IO methods could recognize a variant of the prefix starting with <internal:.

@headius
Copy link
Member

headius commented Dec 11, 2020

Linking with #5764 since that rewrites most of the load logic.

@eregon
Copy link
Member Author

eregon commented Dec 12, 2020

For more context, see

@headius headius modified the milestones: JRuby 9.3.0.0, JRuby 9.3.1.0 Jul 1, 2021
@headius headius modified the milestones: JRuby 9.3.1.0, JRuby 9.4.0.0 Oct 8, 2021
@headius headius modified the milestones: JRuby 9.4.0.0, JRuby 9.4.1.0 Nov 16, 2022
@enebo enebo modified the milestones: JRuby 9.4.1.0, JRuby 9.4.2.0 Jan 31, 2023
@headius headius modified the milestones: JRuby 9.4.2.0, JRuby 9.4.3.0 Mar 1, 2023
@headius headius modified the milestones: JRuby 9.4.3.0, JRuby 9.4.4.0 Jun 6, 2023
headius added a commit to headius/jruby that referenced this issue Oct 10, 2023
This allows tools to omit any of our internal Ruby-based kernel
sources when processing backtraces and source_locations. The
filtering here is only done in source_location and backtrace
generation currently, with some extra logic during backtrace
mining to skip internal frames for e.g. Kernel#warn.

I only did this filtering at the highest levels, when the filename
is actually about to be used and returned to a Ruby consumer, to
avoid any internal usage of these filenames breaking due to the
new prefix.

Fixes jruby#6430
headius added a commit to headius/jruby that referenced this issue Oct 10, 2023
The Java version did little more than yield to the given block,
and the new specs provided for Kernel#warn skipping internal files
expects that Kernel#tap will come from an internal file.

See jruby#6430
headius added a commit to headius/jruby that referenced this issue Oct 10, 2023
The Java version did little more than yield to the given block,
and the new specs provided for Kernel#warn skipping internal files
expects that Kernel#tap will come from an internal file.

There's a workaround here for behavior that changed in CRuby,
whereby yielding to a lambda should trigger arity errors rather
than bypassing arity checks.

See https://bugs.ruby-lang.org/issues/12705

See jruby#6430 for the "internal" source issue that led to moving this
into Ruby.
headius added a commit to headius/jruby that referenced this issue Oct 11, 2023
The Java version did little more than yield to the given block,
and the new specs provided for Kernel#warn skipping internal files
expects that Kernel#tap will come from an internal file.

There's a workaround here for behavior that changed in CRuby,
whereby yielding to a lambda should trigger arity errors rather
than bypassing arity checks.

See https://bugs.ruby-lang.org/issues/12705

See jruby#6430 for the "internal" source issue that led to moving this
into Ruby.
@headius headius modified the milestones: JRuby 9.4.4.0, JRuby 9.4.5.0 Oct 11, 2023
@headius headius modified the milestones: JRuby 9.4.5.0, JRuby 9.4.6.0 Oct 31, 2023
headius added a commit to headius/jruby that referenced this issue Nov 8, 2023
This allows tools to omit any of our internal Ruby-based kernel
sources when processing backtraces and source_locations. The
filtering here is only done in source_location and backtrace
generation currently, with some extra logic during backtrace
mining to skip internal frames for e.g. Kernel#warn.

I only did this filtering at the highest levels, when the filename
is actually about to be used and returned to a Ruby consumer, to
avoid any internal usage of these filenames breaking due to the
new prefix.

Fixes jruby#6430
headius added a commit to headius/jruby that referenced this issue Nov 8, 2023
The Java version did little more than yield to the given block,
and the new specs provided for Kernel#warn skipping internal files
expects that Kernel#tap will come from an internal file.

There's a workaround here for behavior that changed in CRuby,
whereby yielding to a lambda should trigger arity errors rather
than bypassing arity checks.

See https://bugs.ruby-lang.org/issues/12705

See jruby#6430 for the "internal" source issue that led to moving this
into Ruby.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants