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

Fix libyaml download failure rescue under miniruby #562

Merged
merged 1 commit into from
Jun 14, 2022

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented May 26, 2022

I tried to build Ruby on a system without libyaml today and realized
that my attempt from #557 doesn't
fix the error in #552. I still got
the same LoadError from digest.

Since LoadError is not a StandardError, a plain rescue doesn't catch
it. Catch LoadError explicitly instead and reduce the scope of the
begin block.

I tested this change in a Ruby build on macOS without libyaml installed
and confirmed that make continues with a warning instead of aborting:

*** Following extensions are not compiled:
psych:
        Could not be configured. It will not be installed.
        ...

I tried to build Ruby on a system without libyaml today and realized
that my attempt from ruby#557 doesn't
fix the error in ruby#552. I still got
the same LoadError from digest.

Since `LoadError` is not a StandardError, a plain `rescue` doesn't catch
it. Catch `LoadError` explicitly instead and reduce the scope of the
`begin` block.

I tested this change in a Ruby build on macOS without libyaml installed
and confirmed that `make` continues with a warning instead of aborting:

    *** Following extensions are not compiled:
    psych:
            Could not be configured. It will not be installed.
            ...
@XrXr
Copy link
Member Author

XrXr commented May 26, 2022

For only graceful failures on the Ruby side, we could apply:

diff --git a/ext/extmk.rb b/ext/extmk.rb
index a440af27fc..fd6518fa6e 100755
--- a/ext/extmk.rb
+++ b/ext/extmk.rb
@@ -224,7 +224,7 @@ def extmake(target, basedir = 'ext', maybestatic = true)
        end
       rescue SystemExit
        # ignore
-      rescue => error
+      rescue StandardError, ScriptError => error
         ok = false
       ensure
        rm_f "conftest*"

This PR gives better error message in addition to graceful failures. We may want to change extmk.rb for similar issues in the future, though.

@XrXr
Copy link
Member Author

XrXr commented Jun 14, 2022

I'm going to merge this since it should fix https://bugs.ruby-lang.org/issues/18790 for people that don't have libyaml installed on their system. Feel free to revert/ make changes if you notice something off.

@XrXr XrXr merged commit 251289b into ruby:master Jun 14, 2022
@XrXr XrXr deleted the graceful-no-libyml branch June 14, 2022 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant