On Wed, Sep 15, 2021 at 12:25 PM Vít Ondruch <vondruch(a)redhat.com> wrote:
Dne 15. 09. 21 v 11:45 Pavel Valena napsal(a):
On Wed, Sep 15, 2021 at 10:49 AM Vít Ondruch <vondruch(a)redhat.com> wrote:
>
https://src.fedoraproject.org/rpms/ruby/blob/rawhide/f/macros.rubygems#_57
>
> Maybe this ^^ could default to empty array instead of `nil`?
>
Yes, but that would just ignore the error. `nil` is fine with me.
> OTOH, it might be worth of raising error for this case instead of just
> being NOOP, because this helps to detect changes in gem and remove the
> macro when it is not needed anymore.
>
Yes, that's what I'm proposing. It's just that the current error message
is not explanative (it's not obvious what's broken).
```
if dep
abort("#{name} dependency already exists") unless dependencyt
```
You meant s/dependencyt/requirements/, right? And the error message could
be probably more explanatory, e.g. it could include something like "without
explicitly defined requirements" ...
Yes, sorry, `requirements`. Ok, I'll prepare a PR.
Pavel
V.
Like this?
Pavel
>
> Vít
>
>
> Dne 14. 09. 21 v 16:21 Pavel Valena napsal(a):
>
> Hello,
>
> I've encountered an issue while doing an update to vagrant-libvirt 0.5.3.
>
> ```
> DEBUG: + echo 'gemspec_file =
'\''../vagrant-libvirt-0.5.3.gemspec'\''
> DEBUG:
> DEBUG: name = '\''rexml'\''
> DEBUG: requirements = nil
> DEBUG:
> DEBUG: type = :runtime
> DEBUG:
> DEBUG: spec = Gem::Specification.load(gemspec_file)
> DEBUG: abort("#{gemspec_file} is not accessible.") unless spec
> DEBUG:
> DEBUG: dep = spec.dependencies.detect { |d| d.type == type && d.name
> == name }
> DEBUG: if dep
> DEBUG: dep.requirement.concat requirements
> DEBUG: else
> DEBUG: spec.public_send "add_#{type}_dependency", name, requirements
> DEBUG: end
> DEBUG: File.write gemspec_file, spec.to_ruby'
> DEBUG: + ruby
> DEBUG: /usr/share/rubygems/rubygems/requirement.rb:146:in `concat':
> undefined method `flatten' for nil:NilClass (NoMethodError)
> DEBUG: from -:13:in `<main>'
> ```
>
> This is caused by (I've already removed it; and it works now):
> ```
> -# Rexml needs to be required since Ruby 3.0.
> -#
https://github.com/vagrant-libvirt/vagrant-libvirt/pull/1277
> -%gemspec_add_dep -g rexml -s ../%{vagrant_plugin_name}-%{version}.gemspec
> ```
> As this would be an empty change (rexml was already added), and
> `requirements` is `nil`, this is not expected use.
>
> Should we enhance the macros to cover this case with a proper error
> message?
>
> Regards,
> Pavel
>
> Additional notes:
>
>
https://github.com/rubygems/rubygems/blob/3387dbf66d3721f1a25ea60d6bab8fd...
>
https://src.fedoraproject.org/rpms/vagrant-libvirt/pull-request/7
>
>
>