Ticket #256 (closed enhancement: fixed)

Opened 14 months ago

Last modified 13 months ago

Ruby guidelines - %gem_install macro

Reported by: vondruch Owned by: spot
Priority: minor Milestone:
Component: Guideline Draft Version:
Keywords: Cc: bkabrda
Blocked By: Blocking:

Description

Would you please review new version of Ruby packaging guidelines [1]? The only change is introduction of a %gem_install macro. This [2] should be the changes describing usage of the macro. The macro simplifies the .spec file a bit and allows us to globally modify certain aspects of gem installation (generation in defaults of generation of documentation is the particular reason) without modification of every .spec file. It current implementation can be found here [3].

In the future, I'd like to work toward more macros as such, covering 'gem build', 'gem spec' etc, but I am running out of the time for F19.

Thank you

[1] https://fedoraproject.org/wiki/PackagingDrafts/Ruby [2] https://fedoraproject.org/w/index.php?title=PackagingDrafts%2FRuby&diff=323959&oldid=323941 [3] http://pkgs.fedoraproject.org/cgit/ruby.git/tree/ruby.spec?h=ruby-2.0#n450

Change History

comment:1 Changed 14 months ago by bkabrda

  • Cc bkabrda added

comment:2 Changed 14 months ago by toshio

#info the update to Ruby Guideline to use %gem_install passes with a note that renaming it to something that reflects that this is a build step, not install step would be desirable. (+1: 6, 0: 1, -1: 0)

Little explanation of the last part. There was a little of debate so we decided to suggest but not require the macro name to be changed. The problem with %gem_install as a macro name is that:

  • rpm has %build and %install sections. It seems odd to run %gem_install in the %build section. When you could see the whole command, you could see the local, temporary directory and figure out what was going on. So perhaps a better name for the macro would not include "install" in the name. For instance "%build_gem" or similar.

On the other side of the debate:

  • "gem install" is the command being run behind the scenes by the macro. We have lots of precedent for naming macros after the command that they invoke (For instance %configure).

We agreed that we were fine with the guideline and would just mention this for the ruby sig to decide whether to change the name or not.

Please let me know what you decide and then I'll write up the change in the Guidelines.

comment:3 Changed 14 months ago by vondruch

Well I had the same feeling about the name and I agree it is a bit unfortunate. However, the %build_gem has the same issues, since we are using 'gem build' command in %prep section. Therefore, we decided to name the macro according the underlying command.

I see that there were some concerns about --install-dir so just want to clarify that there is no use case ATM. Nevertheless, the gem is unpacked into %{_builddir}/%{gem_name}-%{version} and into the same directory is the gem later installed. I am afraid, that it might possibly lead in some cases to errors during gem rebuild phase, so in that case, redefining --install-dir might help and you would not need to drop to plain 'gem install' commands. I am fine if you will leave out the -d parameter from guidelines, but I'll keep it in macro anyway.

comment:4 Changed 14 months ago by toshio

Not quite sure I understand that last paragraph... Do you have an example of what you'd redefine install dir too?

comment:5 Changed 14 months ago by vondruch

1) If the gem, which is unpacked contains its .gemspec file, the 'gem spec' command will rewrite this file. If you want to preserve it, you might either modify the install dir of the gem or ouput path of the 'gem spec' while the former is more secure IMO.

2) Some legacy gems are doing 'gem install' in their %install section. There you have to redefine the --install-dir to install into %{buildroot}, while you are in %{builddir}. Alternatively, you could change the current directory.

comment:6 Changed 14 months ago by toshio

If I'm understanding correctly, in (1) you're asking to use %gem_install to install into the buildroot instead of into the builddir. I think that should be disallowed in the guidelines.

For (2) it seems like the same thing except that you're positing that there's packages which haven't updated to doing things the correct way. When they are updated, instead of updating to do things the correct way, they are simply being touched to add this macro instead. I kinda think that should be disallowed as well....

comment:7 Changed 14 months ago by vondruch

No, in (1) I am asking to install into subdirectory of %{builddir}. However, it should be macroized in future, so it is not big deal.

Regarding (2), there is always one thing which is written in guidelines and second think what maintainers do. Guidelines can be enforced for newly reviewed packages, not for the old one (unless someone will do the work). If I am doing updates of packages for Ruby 2.0, I am doing as small changes as I can, if it is not my package. If it should be %gem_install macro in %install section, so be it. The rest is up to the package maintainers.

comment:8 Changed 14 months ago by toshio

My point on (2) is that %gem_install macro is not necessary. So if you're really doing as small a change as possible, don't convert to the %gem_install macro if it's in %install.

comment:9 Changed 14 months ago by vondruch

As I explained in initial description, the macro was introduced to cover changes in default flags for generating documentation, so for the next time, just rebuild would be enough. Otherwise, you can always fallback to plain "gem install" command.

comment:10 Changed 14 months ago by toshio

I don't know if we're in agreement and things are just getting lost in translation. So I'll make a proposal here that will hopefully clarify the guidelines and what I'm just trying to ensure:

in the %build section: https://fedoraproject.org/wiki/PackagingDrafts/Ruby#.25build

{{admon/note
The %gem_install macro must not be used to install into the <code>%{buildroot}</code> }}

comment:11 Changed 13 months ago by vondruch

Toshio, can we please move this forward? I.e. can be the guidelines updated? This was approved month ago. People are fixing their packages for Ruby 2.0.0 but they are not using the %gem_install macro, since it is not in official guidelines. This causing more harm then if there will be some note or not.

We are flaming here about unimportant -d command instead of focusing on important stuff. If the -d is so controversial for you, please drop it from the guidelines. If somebody will be capable enough to find it, (s)he will probably use it for good.

Thank you.

comment:12 Changed 13 months ago by toshio

I haven't seen any flames from you yet, hopefully you haven't perceived any of the things I've said as flames either. If you have, I apologize as that hasn't been my intent. I think there's just language confusion over the interpretation of the guideline. I've explicitly asked that the other FPC members vote on the addition of the note in comment:10 to clarify the language and unstick the writeup of this guideline.

+1 to addition of the note in comment:10

comment:13 Changed 13 months ago by limb

+1 from me as well.

comment:14 Changed 13 months ago by rathann

+1 from me.

comment:15 Changed 13 months ago by rdieter

+1

comment:16 Changed 13 months ago by tibbs

+1 to the comment:10 addition.

comment:17 Changed 13 months ago by spot

  • Status changed from new to assigned
  • Owner set to spot

+1.

This amendment (in comment #10) is approved. (+1:6, 0:0, -1:0)

comment:18 Changed 13 months ago by toshio

guidelines updated.

Announcement text:

The ruby gems guidelines have been updated to make use of the %gem_install macro that is available in Fedora 19 and beyond.

comment:19 Changed 13 months ago by spot

  • Status changed from assigned to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.