Implement rich RPM dependencies #124

ekohl · 2023-11-24T17:59:14Z

This implements rich RPM dependencies, which are available with RPM 4.14+ (introduced in Fedora 271). This means only a single line for a dependency is used. That doesn't play well with the .with_requires.commented_out approach that was implemented. This instead moves the loop back to the template.

This implements rich RPM dependencies, which are available with RPM 4.14+ (introduced in Fedora 27[1]). This means only a single line for a dependency is used. That doesn't play well with the .with_requires.commented_out approach that was implemented. This instead moves the loop back to the template. [1]: https://fedoraproject.org/wiki/Changes/RPM-4.14

ekohl · 2023-11-24T19:12:28Z

Bleh, Ruby 2.6 had a different sorting compared to the rest which breaks the tests.

voxik · 2023-11-27T14:13:55Z

templates/fedora-27-rawhide.spec.erb

-  ["rdoc", "rake", "bundler"].include? d.name
-end.virtualize.with_requires.comment_out.to_rpm -%>
+<% for req in development_dependencies.reject { |d| ["rdoc", "rake", "bundler"].include? d.name }.virtualize -%>
+# BuildRequires: <%= req.to_rich_rpm %>


Unless I am missing something, I'd prefer to keep using the end.virtualize.with_requires.comment_out and just append the to_rich_rpm

I meant to reply https://github.com/fedora-ruby/gem2rpm/pull/124/files#r1406987179 here.

voxik · 2023-11-27T14:16:19Z

lib/gem2rpm/rpm_dependency.rb

@@ -47,5 +47,11 @@ def to_rpm
      end
      rpm_dependencies.join("\n")
    end
+
+    # Returns string with entry suitable for RPM .spec file with RPM 4.14+.
+    def to_rich_rpm


In principle, I don't have anything against this. However, what is the merit of separate method in comparison to adding some options to to_rpm? Maybe the options could also cover my proposal with dropping the version altogether (but now I remember that having the versions around is nice for awareness, so I'd like to keep them printed into the output).

Actually, second option could be to have filter, similar to how the virtualize.with_requires.comment_out filter chain works. Actually that would be probably the best option if it is doable

I was under the impression that virtualize.with_requires results in ['Requires: rubygem(mygem) >= 1.0', 'Requires: rubygem(mygem) < 2'] and if you join that, it doesn't work. I considered something like virtualize.to_rich_rpm.with_requires.commented_out but then .to_rich_rpm doesn't return a string anymore.

Actually, second option could be to have filter, similar to how the virtualize.with_requires.comment_out filter chain works. Actually that would be probably the best option if it is doable

I'm not sure what you exactly mean by this. Do you mean supporting a block?

This demonstrates how the filter chain works:

$ cat test.spec.erb <% p development_dependencies p development_dependencies.virtualize p development_dependencies.virtualize.with_requires p development_dependencies.virtualize.with_requires.comment_out p development_dependencies.virtualize.with_requires.comment_out.to_rpm -%> $ bin/gem2rpm --template test.spec.erb test/artifacts/testing_gem/testing_gem-1.0.0.gem #<Gem2Rpm::RpmDependencyList:0x00007f10ee78b738 @items=[<Gem::Dependency type=:development name="test_development" requirements="~> 1.0, >= 1.0.0">]> #<Gem2Rpm::RpmDependencyList:0x00007f10e0fa8dc0 @items=[<Gem::Dependency type=:development name="rubygem(test_development)" requirements="~> 1.0, >= 1.0.0">]> #<Gem2Rpm::RpmDependencyList:0x00007f10e0fa88c0 @items=[<Gem::Dependency type=:development name="BuildRequires: rubygem(test_development)" requirements="~> 1.0, >= 1.0.0">]> #<Gem2Rpm::RpmDependencyList:0x00007f10e0fa82f8 @items=[<Gem::Dependency type=:development name="# BuildRequires: rubygem(test_development)" requirements="~> 1.0, >= 1.0.0">]> "# BuildRequires: rubygem(test_development) >= 1.0\n# BuildRequires: rubygem(test_development) < 2\n# BuildRequires: rubygem(test_development) >= 1.0.0\n"

As you can see, there still single Gem::Dependency object and the filters modifies the internal name field, keeping the requirement field in its original form. IOW it is still array of Gem::Version object.

Let me change the template a bit:

$ cat test.spec.erb <% development_dependencies.virtualize.with_requires.comment_out.each {|d| p d.requirement } -%> $ bin/gem2rpm --template test.spec.erb test/artifacts/testing_gem/testing_gem-1.0.0.gem [">= 1.0", "< 2", ">= 1.0.0"]

IOW, at this stage, it is still possible to output single line or three lines, as needed. From here I continued in this direction:

$ cat test.spec.erb <%= development_dependencies.virtualize.with_requires.comment_out.map do |d| [d.name, "(#{d.requirement.join(' with ')})"].join end.join("\n") -%> $ bin/gem2rpm --template test.spec.erb test/artifacts/testing_gem/testing_gem-1.0.0.gem # BuildRequires: rubygem(test_development)(>= 1.0 with < 2 with >= 1.0.0)

This however unveils, that my filter idea won't work as I hoped 🤣🤦🏻‍♂️ But after all, this might be the way:

$ cat test.spec.erb <%= dd = development_dependencies.virtualize.map do |d| Gem::Dependency.new("(#{d.requirement.map {|r| [d.name, r].join(' ')}.join(' with ')})", d.type) end dd = Gem2Rpm::RpmDependencyList.new dd dd.with_requires.comment_out.to_rpm -%> $ bin/gem2rpm --template test.spec.erb test/artifacts/testing_gem/testing_gem-1.0.0.gem # BuildRequires: (rubygem(test_development) >= 1.0 with rubygem(test_development) < 2 with rubygem(test_development) >= 1.0.0)

This could be further improved if the RpmDependencyList implemented the map method or of course if there was different filter, such as .rich_dependencies hiding this.

This however unveils, that my filter idea won't work as I hoped

This is exactly what I was referring to.

I'll say that I actually like the loop in the ERB template: it makes it easier to understand what's going on. Implementing map (or even more parts of Enumerable) on RpmDependencyList does sound like a good idea.

Implementing the map or additional filter provides just an option. It does not limit use of more low level constructs. After all, we certainly had more bare loops in the templates previously.

voxik · 2023-11-27T14:27:54Z

Just to summarize my inline comments, I think that the implementation of rich filters could end up like this:

$ git diff
diff --git a/templates/fedora-27-rawhide.spec.erb b/templates/fedora-27-rawhide.spec.erb
index 7ad641b..d7fbbbb 100644
--- a/templates/fedora-27-rawhide.spec.erb
+++ b/templates/fedora-27-rawhide.spec.erb
@@ -24,7 +24,7 @@ BuildRequires: gcc
 <% end -%>
 <%= development_dependencies.reject do |d|
   ["rdoc", "rake", "bundler"].include? d.name
-end.virtualize.with_requires.comment_out.to_rpm -%>
+end.virtualize.with_requires.comment_out.rich_dependencies.to_rpm -%>
 <% if spec.extensions.empty? -%>
 BuildArch: noarch
 <% end -%>

voxik · 2023-11-27T14:30:56Z

+end.virtualize.with_requires.comment_out.rich_dependencies.to_rpm -%>

And I won't mind if you come up with better name then rich_dependencies 🙈

voxik · 2023-11-27T14:32:24Z

test/templates/test_fedora.rb

@@ -36,7 +36,7 @@ def test_exclude_extension_directory
  end

  def test_build_requires
-    assert_match(/^# BuildRequires: rubygem\(test_development\) >= 1\.0\.0$/, @out_string)
+    assert_includes(@out_string, "\n# BuildRequires: (rubygem(test_development) >= 1.0 with rubygem(test_development) < 2 with rubygem(test_development) >= 1.0.0)\n")


Just to double check, is RPM able to cope with this?

https://fedoraproject.org/wiki/Changes/RPM-4.14#User_Experience lists Requires: (foo >= 1.0 with foo < 2.0) as an example. I haven't tried more 2 conditions, so that would be a good testcase. That said, there is this code in /usr/lib/rpm/rubygems.req (rubygems-devel-3.4.10-180.fc38.noarch)

# Compose dependency together with its requirements in RPM rich dependency # string. def self.compose_dependency_string(name, requirements) dependency_strings = requirements.map { |requirement| name + requirement } dependency_string = dependency_strings.join(' with ') dependency_string.prepend('(').concat(')') if dependency_strings.length > 1 dependency_string end

That doesn't make any special conditions and does pretty much the same thing.

I can't tell I trust myself :D

ekohl mentioned this pull request Nov 24, 2023

Implement rich RPM dependencies #123

Draft

ekohl force-pushed the rich-rpms-simplified branch from 128f58f to 51b5119 Compare November 24, 2023 18:13

ekohl mentioned this pull request Nov 24, 2023

Implement rich dependencies in gem2rpm templates theforeman/foreman-packaging#10025

Merged

ekohl added 2 commits November 24, 2023 19:31

Simplify test from regex to simple includes assertion

b3bc787

ekohl force-pushed the rich-rpms-simplified branch from 51b5119 to 5d40122 Compare November 24, 2023 18:33

voxik reviewed Nov 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement rich RPM dependencies #124

Implement rich RPM dependencies #124

ekohl commented Nov 24, 2023

ekohl commented Nov 24, 2023

voxik Nov 27, 2023

ekohl Nov 28, 2023

voxik Nov 27, 2023

voxik Nov 27, 2023

ekohl Nov 28, 2023

voxik Nov 28, 2023

ekohl Nov 28, 2023

voxik Nov 28, 2023

voxik commented Nov 27, 2023 •

edited

Loading

voxik commented Nov 27, 2023

voxik Nov 27, 2023

ekohl Nov 28, 2023

voxik Nov 28, 2023

Implement rich RPM dependencies #124

Are you sure you want to change the base?

Implement rich RPM dependencies #124

Conversation

ekohl commented Nov 24, 2023

ekohl commented Nov 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voxik commented Nov 27, 2023 • edited Loading

voxik commented Nov 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voxik commented Nov 27, 2023 •

edited

Loading