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

Improve array rb documentation #565

Closed
wants to merge 509 commits into from
Closed

Conversation

alexcrocha
Copy link

No description provided.

jhawthorn and others added 30 commits December 3, 2024 10:03
In some locations we were using shared->lock and in others
&shared->lock, and we were leaking the allocated memory.
The evaluation order of C arguments is unspecified.
`RSTRING_LEN(str)` would fails if the conversion to a String by
`StringValuePtr` is not done yet.

Coverity Scan found this issue.
The evaluation order of C arguments is unspecified.
`RSTRING_LEN(value)` would fail if the conversion to a String by
`StringValuePtr(value)` is not done yet.

Coverity Scan found this issue.

ruby/psych@d1e6bf323a
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.27.5 to 3.27.6.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@f09c1c0...aa57810)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
In this context, `th` must not be NULL
Get rid of repeated exec XRUBY recursively.
Coverity Scan alerts `for(i, j=0;...)` as a misuse of comma expression.
Since ruby 2.3, a file opened with `File::SHARE_DELETE` and
`File::BINARY` can be renamed or removed.

ruby/logger@7b6146fee6
The functions dereference `*dead_entry` without a NULL check
Refactor to use real test cases rather than mock.

Add relative path tests wich `Dir.chdir`.

rubygems/rubygems@ed556a0a53
Copy from gc/default/default.c and revert the part of 51bd816.
We can use the BUILDING_SHARED_GC flag to check if we're building gc_impl.h
as a shared GC or building the default GC.
amomchilov and others added 24 commits December 12, 2024 17:55
* Clarify “Building Ruby” docs

* Fix test examples to work from `build` dir

* Clarify “Testing Ruby” examples with real examples

All the commands should run correctly by default, without the user needing to modify them. This builds confidence that the relative paths are working correct from within the `build` directory.

Also, let’s use a consistent example throughout, for greater clarity.

* Improve examples to use `-v` flag in-context

This shows the correct way to combine `-v` with another parameter, e.g. a specific file to test.

* Other readability improvements

* Clarify `make` implementation support
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.27.7 to 3.27.9.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@babb554...df409f7)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
It is not "in bytes" for wide char literal.
* Make it loose coupling between RubyGems and RDoc

\### Problems

There are following problems because of tight coupling between RubyGems and RDoc.

1. If there are braking changes in RDoc, RubyGems is also broken.
2. When we maintain RDoc, we have to change RubyGems.

The reason why they are happened is that RubyGems creates documents about a gem with installing it.

Note that RubyGems uses functions of RDoc to create documents.
Specifically,

- Creating documents is executed by `rubygems/lib/rubygems/rdoc.rb`.
- `::RDoc::RubygemsHook` which is defined by RDoc is called by the file.

\### Solution

RubyGems has the plugin system.

If a gem includes `rubygems_plugin.rb`, RubyGems loads it.
RubyGems executes a process defined in it while installing gems, uninstalling gems or other events.

We can use the system to solve the problems.

The root cause is RubyGems directly references the class of RDoc.

We can remove the root cause by making RDoc RubyGems plugin.

Alternatively `rubygems_plugin.rb` creates documents about gems.

\### FAQ

Q1. Do we need to change codes of RubyGems?

A.

No, we don't.

This change keeps compatibility of API used from RubyGems.

Q2. Is it better to delete existing codes related to RDoc in RubyGems?

No, it isn't.

If we change codes of RubyGems,
we can't keep a compatibility.

Example:

If we delete codes that uses `RDoc::RubygemsHook` in `rubygems/lib/rubygems/rdoc.rb`,
documentations are not created with old RDoc.

Q3. When can we delete `rubygems/lib/rubygems/rdoc.rb`?

A.

We can delete it when all users use RDoc including `rubygems_plugin`.

Next ruby version is 3.4.
If it includes the RDoc including `rubygems_plugin`,
we can delete `rubygems/lib/rubygems/rdoc.rb` after ruby 3.3 is EOL.

Q4. Is it a breaking change that Rubygems creates documents with
rubygems_plugin not RDoc::RubygemsHook?

A.

No, it isn't.

If we simply implement this approach,
we move the implementation from `rdoc/lib/rdoc/rubygems_hook.rb` to
`rubygems_plugin.rb`.

This way can be breaking change.

It seems to be fine that we just need to delete `rdoc/rubygems_hook.rb` but it doesn't work.
It generates multiple documents.

`rubygems/lib/rubygems/rdoc.rb` has the following code.

```
begin
  require "rdoc/rubygems_hook"
  # ...
rescue LoadError
end
```

This code ignores RDoc related processes when `rdoc/rubygems_hook` can't be required.
But, this 'require' is not failed.

This is because Ruby installs Rdoc as a default gem.

So, Rdoc installed as a default gem generates documents and one installed as a normal gem does it too.

If you think that this behavior is accectable,
we can just delete `rdoc/rubygems_hook.rb`.

What do you think about this approach?

In this change, we take another approach to solve the problem that creates multiple documents.

If `Gem.done_installing(&Gem::RDoc.method(:generation_hook))` in `rubygems/rdoc.rb` doesn't create documents,
we can solve the problem.

We have some options.

* We change `rubygems/rdoc.rb` and then don't execute `Gem.done_installing`.
  (This is a change for RubyGems.)
* We change `rdoc/rubygems_hook.rb` and then make `generation_hook` a no-op method.
  (This is a change for RDoc.)

We choose the latter to avoid changing for RubyGems.

\### Test

\#### Preparation

Install Rdoc which including our changes by executing `rake install`.

❯ rake install

We confirmed that Rdoc which including our changes was installed.

❯ gem list | grep rdoc
rdoc (6.6.0, default: 6.4.0)

\#### Check point

We tested to check compatibility.

How to chack the compatibility?

We tested creating same documents by our RDoc and old RDoc with latest RubyGems.

We used following versions to test.

```
❯ ruby -v
ruby 3.1.0p0 (2021-12-25 revision fb4df44) [arm64-darwin22]

❯ gem list | grep rdoc
rdoc (default: 6.4.0)

❯ ruby -I rubygems/lib rubygems/exe/gem --version
3.5.14
```

Here is a result of test with old RDoc.
We can see that the document is created correctlly with `Parsing...` and `Done installing...`.

```
❯ ruby -I rubygems/lib rubygems/exe/gem install pkg-config
Successfully installed pkg-config-1.5.6
Parsing documentation for pkg-config-1.5.6
Done installing documentation for pkg-config after 0 seconds
1 gem installed
```

Here is a result of test with our RDoc.
We can see that the document is created correctlly with `Parsing...` and `Done installing...`.

```
❯ ruby -I rubygems/lib rubygems/exe/gem install pkg-config
Successfully installed pkg-config-1.5.6
Parsing documentation for pkg-config-1.5.6
Done installing documentation for pkg-config after 0 seconds
1 gem installed
```

As you can see we got the same results, our RDoc keeps compatibility.

* rename a test file

* Revert "rename a test file"

This reverts commit 70a144bf3fb8f2cc653972e858b5fed3747765d7.

* revert a test class name

* exclude `TestRDocRubyGemsHook` at job of ruby-core

* When `rubygems_plugin.rb` is not found, `test_rdoc_rubygems_hook.rb` is skipped.

* remove unnecessary whitespace

* add comment

* Add support for the case that RDoc is installed as a default gem

* Fix problems

Co-authored-by: mterada1228 <49284339+mterada1228@users.noreply.github.com>

* Simplify

* removed unused blank lines and revert test

* for rerun tests

* add comment for rubygems_plugin.rb

---------

Co-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
* Use `launchable record session` command to split test sessions based on test suites.
* Use BTESTS env instead of RUBY_TESTOPTS for passing CLI option to `make btest`.
* Group Launchable logs
* Add several flavors to identify the relationship between test session and GH workflow.
(forgot to amend...)
…NAME has been changed

Use Process.argv0 instead of $PROGRAM_NAME because $PROGRAM_NAME is
liable to be changed but Process.argv0 is not.

rubygems/rubygems@43b747dc9e
[Bug #20950]

ifunc proc has the ep allocated in the cfunc_proc_t which is the data of
the TypedData object. If an ifunc proc is duplicated, the ep points to
the ep of the source object. If the source object is freed, then the ep
of the duplicated object now points to a freed memory region. If we try
to use the ep we could crash.

For example, the following script crashes:

    p = { a: 1 }.to_proc
    100.times do
      p = p.dup
      GC.start
      p.call
    rescue ArgumentError
    end

This commit changes ifunc proc to also duplicate the ep when it is duplicated.
A good amount of call sites always pass nil as block argument, but the
nil doesn't show up in the context. Put a runtime guard for those
cases to handle it. Particular relevant for the `ruby-lsp` benchmark in
`yjit-bench`. Up to a 2% speedup across headline benchmarks.

Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
Co-authored-by: Randy Stauner <randy.stauner@shopify.com>
array.rb Outdated Show resolved Hide resolved
@alexcrocha alexcrocha force-pushed the improve-array-rb-documentation branch from 949fa22 to 6c9cd51 Compare December 13, 2024 17:51
@alexcrocha alexcrocha closed this Dec 13, 2024
@XrXr XrXr deleted the improve-array-rb-documentation branch December 13, 2024 19:31
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 this pull request may close these issues.