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

Experimental Prism feature listed as runtime dependency #282

Closed
mhashizume opened this issue Feb 29, 2024 · 11 comments · Fixed by #285
Closed

Experimental Prism feature listed as runtime dependency #282

mhashizume opened this issue Feb 29, 2024 · 11 comments · Fixed by #285

Comments

@mhashizume
Copy link

Support for Prism was added in rubocop-ast 1.31.0 and marked as "experimental" in the changelog. However, Prism was also added as a runtime dependency in the Gemspec.

It seems contradictory to require a library for an experimental feature, particularly when it's only available in the latest Ruby release.

We're starting to see some failures in our CI pipelines because of this new requirement.

Would you be able to remove Prism as a runtime dependency? Thank you!

@Routable
Copy link

We are also experiencing this issue, with the same behavior as reported above. Downgrading has been our solution, but this really should be addressed.

@bbatsov
Copy link
Contributor

bbatsov commented Feb 29, 2024

Yeah, it seems we messed up. @koic I think we should move this to the Gemfile instead unless you have other alternatives in mind.

@marcandre
Copy link
Contributor

Right, I think #283 is all that is required?

@koic
Copy link
Member

koic commented Mar 1, 2024

This was marked as experimental with regard to the validity of what should be stable published interface API.

It seems necessary to understand the details of the issue before making a decision to remove it from the default dependencies.

RuboCop AST 1.31 requires Ruby 2.7+, and Prism works with Ruby 2.7+. This means they should be compatible in terms of requirements, and if there are any issues, it would be worth reporting to Prism:
https://github.com/ruby/prism

We're starting to see some failures in our CI pipelines because of this new requirement.

@mhashizume @Routable Can you provide specifics about what errors are occurring?

cc @kddnewton

@marcandre
Copy link
Contributor

Agreed on reporting issue, but I also feel that we should not say that rubocop-ast depends on prism to work.

@bbatsov
Copy link
Contributor

bbatsov commented Mar 1, 2024

Yeah, I was under the impression that's the actual issue - now we force the installation of Prism even when it's not used. It seems to me the users will have to install Prism separately for the time being, if they want to use it.

@KNejad
Copy link

KNejad commented Mar 1, 2024

@koic the issue we are experiencing is that our GitHub CI based on an Alpine image is failing since the development tools need to be installed.

It can be fixed easily by adding apk add build-base before the gem install line, however this would add an extra step to install libraries which could slow down our CI. In the meantime we will probably pin to a previous version.

Here's the output log in case it's of use (we use standardrb which depends on rubocop-ast):

ERROR:  Error installing standardrb:
	ERROR: Failed to build gem native extension.
    current directory: /usr/local/bundle/gems/prism-0.24.0/ext/prism
/usr/local/bin/ruby extconf.rb
checking for prism.h in /usr/local/bundle/gems/prism-0.24.0/include... *** extconf.rb failed ***
Could not create Makefile due to some reason, probably lack of necessary
libraries and/or headers.  Check the mkmf.log file for more details.  You may
need configuration options.
Provided configuration options:
	--with-opt-dir
	--without-opt-dir
	--with-opt-include=${opt-dir}/include
	--without-opt-include
	--with-opt-lib=${opt-dir}/lib
	--without-opt-lib
	--with-make-prog
	--without-make-prog
	--srcdir=.
	--curdir
	--ruby=/usr/local/bin/$(RUBY_BASE_NAME)
/usr/local/lib/ruby/3.3.0/mkmf.rb:4[8](<JOB URL>)0:in `try_do': The compiler failed to generate an executable file. (RuntimeError)
You have to install development tools first.
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:606:in `block in try_compile'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:555:in `with_werror'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:606:in `try_compile'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:1204:in `block in find_header'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:[9](<JOB URL>)83:in `block in checking_for'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:344:in `block (2 levels) in postpone'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:314:in `open'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:344:in `block in postpone'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:314:in `open'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:340:in `postpone'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:982:in `checking_for'
	from /usr/local/lib/ruby/3.3.0/mkmf.rb:[12](<JOB URL>)03:in `find_header'
	from extconf.rb:62:in `<main>'
To see why this extension failed to compile, please check the mkmf.log which can be found here:
  /usr/local/bundle/extensions/x86_64-linux-musl/3.3.0/prism-0.[24](<JOB URL>).0/mkmf.log

koic added a commit to koic/rubocop-ast that referenced this issue Mar 1, 2024
Fixes rubocop#282.

This PR removes Prism from runtime dependency.

If it is decided that Prism will not be a runtime dependency for the time being,
error message and documentation will be used to communicate the dependency on Prism to users.

Making it a default runtime dependency will be avoided until at least rubocop#282 installation error with Prism is resolved.
@koic
Copy link
Member

koic commented Mar 1, 2024

@KNejad Thank you for the details. Let's hold off on making it a default runtime dependency until at least this installation error with Prism is resolved.

If possible, can you share that error on Prism's issue tracker?
https://github.com/ruby/prism/issues

koic added a commit to koic/rubocop-ast that referenced this issue Mar 1, 2024
Fixes rubocop#282.

This PR removes Prism from runtime dependency.

If it is decided that Prism will not be a runtime dependency for the time being,
error message and documentation will be used to communicate the dependency on Prism to users.

Making it a default runtime dependency will be avoided until at least rubocop#282 installation error with Prism is resolved.
bbatsov pushed a commit that referenced this issue Mar 1, 2024
Fixes #282.

This PR removes Prism from runtime dependency.

If it is decided that Prism will not be a runtime dependency for the time being,
error message and documentation will be used to communicate the dependency on Prism to users.

Making it a default runtime dependency will be avoided until at least #282 installation error with Prism is resolved.
@koic
Copy link
Member

koic commented Mar 1, 2024

rubocop-ast 1.31.1 has been released.

@VitaliySerov
Copy link

Let's hold off on making it a default runtime dependency until at least this installation error with Prism is resolved.
If possible, can you share that error on Prism's issue tracker?

Seems this is not a bug in prism, the reason of this is that prism requires native extensions to be installed, not sure if they have any plan to remove native extensions

So probably it should be held not until prism changes something, but until rubocop releases v2.0 with respect to semantic version and breakable changes

@KNejad
Copy link

KNejad commented Mar 1, 2024

Thanks for sorting this @koic!

I agree with @VitaliySerov, I don't believe this is an issue with prism. It's just they have a system dependency which isn't installed by default on some containers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants