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

At least one gemspec has incorrect dependency data as an array of arrays #2329

Open
deivid-rodriguez opened this issue Apr 24, 2020 · 8 comments

Comments

@deivid-rodriguez
Copy link
Member

The gem in question is ajp-rails version 0.0.0, which has the dependencies property in its gemspec set to an array of arrays.

This makes the gem uninstallable:

$ gem install ajp-rails -v '0.0.0'
ERROR:  While executing gem ... (NoMethodError)
    undefined method `type' for ["ruby-ajp", ">= 0.2.0"]:Array

This is the wrong specification:

gem specification ajp-rails -v '0.0.0' --remote

--- !ruby/object:Gem::Specification
name: ajp-rails
version: !ruby/object:Gem::Version
  version: 0.0.0
platform: ruby
authors:
- Yugui
autorequire: 
bindir: bin
cert_chain: []
date: 2006-01-23 00:00:00.000000000 Z
dependencies:
- - ruby-ajp
  - ">= 0.2.0"
- - rails
  - ">= 0.14"
description: ''
email: [email protected]
executables: []
extensions: []
extra_rdoc_files: []
files: []
homepage: 
licenses: []
metadata: 
post_install_message: 
rdoc_options: []
require_paths:
- lib
required_ruby_version: !ruby/object:Gem::Requirement
  requirements:
  - - ">="
    - !ruby/object:Gem::Version
      version: 1.8.3
required_rubygems_version: 
requirements: []
rubygems_version: 3.2.0.pre1
signing_key: 
specification_version: 1
summary: Ruby on Rails Runner, which uses AJP(Apache JServ Protocol) to cooperate
  with a HTTPd, instead of CGI or FastCGI
test_files: []

Which was corrected in 0.0.1

gem specification ajp-rails -v '0.0.1' --remote

--- !ruby/object:Gem::Specification
name: ajp-rails
version: !ruby/object:Gem::Version
  version: 0.0.1
platform: ruby
authors:
- Yugui
autorequire: 
bindir: bin
cert_chain: []
date: 2006-01-23 00:00:00.000000000 Z
dependencies:
- !ruby/object:Gem::Dependency
  name: ruby-ajp
  version_requirements: !ruby/object:Gem::Requirement
    requirements:
    - - ">="
      - !ruby/object:Gem::Version
        version: 0.2.0
  version_requirement: 
- !ruby/object:Gem::Dependency
  name: rails
  version_requirements: !ruby/object:Gem::Requirement
    requirements:
    - - ">="
      - !ruby/object:Gem::Version
        version: '0.14'
  version_requirement: 
description: ''
email: [email protected]
executables: []
extensions: []
extra_rdoc_files: []
files: []
homepage: 
licenses: []
metadata: 
post_install_message: 
rdoc_options: []
require_paths:
- lib
required_ruby_version: !ruby/object:Gem::Requirement
  requirements:
  - - ">="
    - !ruby/object:Gem::Version
      version: 1.8.3
required_rubygems_version: 
requirements: []
rubygems_version: 3.2.0.pre1
signing_key: 
specification_version: 1
summary: Ruby on Rails Runner, which uses AJP(Apache JServ Protocol) to cooperate
  with a HTTPd, instead of CGI or FastCGI
test_files: []

I presume there's a very reduced number of gemspecs with this problem, I assume they were created when a very old rubygems version. It'd be nice to get the data sanitized in this regard.

Thanks!

@simi
Copy link
Member

simi commented Apr 25, 2020

@deivid-rodriguez I did some analyze recently on rubygems.org DB dump finding out more of those inconsistencies (during some performance improvements investigation). Would it be welcomed to prepare some "normalize" script for those data?

@deivid-rodriguez
Copy link
Member Author

I would personally appreciate efforts on keeping all data normalized, yeah! 👍

In this particular case, we might want to yank the gems where this problem is detected, since the gems were not actually installable anyways.

@sonalkr132
Copy link
Member

I would argue we don't need to spend any effort here. AFAIU, it is not a requirement that every gem uploaded to rg.org has to be installable. Consider, https://rubygems.org/gems/ims-lti/versions/1.0.1, ruby-deug is grayed out because no such gem exists on our server and the version is un-installable.

It is nice that gem has some validations, but there will always be cases where users can push gems which won't not installable. We should consider if issue exists because some bug in rg.org or the owner.

@sonalkr132
Copy link
Member

I am putting this on owner because the gemspec has incorrect format. I understand it is most likely that this broken gem exists because of rubyforge to gemcutter migration.

@deivid-rodriguez
Copy link
Member Author

deivid-rodriguez commented Apr 25, 2020

We ended up adding code in bundler due to this issue. Bundler developers agreed at the time that it would still be best to fix this in the rubygems.org side by yanking the gem. Nowadays you can't really build valid gems with this format, and there are probably also validations in the server side so once the issue is fixed once, it shouldn't come up again.

My idea is to eventually get rid of the code we have in bundler to deal with these pathological cases by fixing things in the right place.

Relevant issues in bundler are https://github.com/rubygems/bundler/search?q=ajp-rails&type=Issues. In rubygems/bundler#5819 custom code in bundler was added to deal with this error, although I have hesitations it was a good thing to do because it masks the issue under a regular gem installation error.

@sonalkr132
Copy link
Member

fixing things in the right place.

I am not so sure about yanking a gem because it is uninstallable, it would be inconsistent with our policy of allowing other uninstallable gems on the site.
We could do it if you feel removing handling for this in bundler would be safer because of it. IMHO, showing appropriate message should be good enough. Right place to fix/report this issue is the owner of the gem.

@deivid-rodriguez
Copy link
Member Author

deivid-rodriguez commented Apr 25, 2020

I'm not really sure where the gem lives 🤷‍♂️. In any case, the gemspec was fixed in version 0.0.1, so there's no issue to report to the owner.

The problem is:

  • A data problem where rubygems.org is serving gems that none of the major clients, rubygems and bundler, can deal with.
  • A bundler problem (as illustrated by the list of issues I linked to) where it sometimes used to fetch this gemspec, even if not being involved in the target lockfile at all.

The combination of both issues would make bundler crash when it shouldn't.

I believe the bundler problem might be fixed, since we haven't got any reports for it in like three years.

Regarding the rubygems.org issue, I wanted to fix it since it somewhat justifies the removal of the code in bundler to handle this pathological case (rubygems/bundler#5819), which is useful for me at the moment to move forward some other refactorings I'm attempting.

Anyways, this is not a big deal, and I can still remove the code I want to remove without the data being fixed, so I'll just close it!

@deivid-rodriguez
Copy link
Member Author

I'm reopening this because both @indirect and @segiddins agreed at rubygems/bundler#5819 (comment) that it'd be nice to get these gems yanked, and also @simi suggested to prepare a script to normalize the data.

I think we could revisit this and that could also potentially allow to revert rubygems/bundler@89aeafd and simplify client code a bit too.

If you think it's still not worth it, we can close again.

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

No branches or pull requests

3 participants