[RPM] Add rubygem-gssapi and rubygem-net-ssh-krb #1665

adamruzicka · 2017-05-31T11:00:59Z

For plugin updates, please indicate which repos this should be built into:

Nightly
1.15
1.14
1.13

See Foreman's plugin maintainer documentation for more information.

The rubygem-gssapi spec was taken and modified from Fedora

dLobatog

@adamruzicka The packages are added to comps and tito.props for both plugins and non-plugins, is there any reason for that? It looks like these only belong on the plugins comps/props.

Which plugin is using this?

adamruzicka · 2017-06-15T11:32:42Z

The packages are added to comps and tito.props for both plugins and non-plugins, is there any reason for that?

I must have messed that up somehow...

It will be used by rubygem-foreman_remote_execution_core after theforeman/foreman_remote_execution#250 gets in

ehelms · 2017-07-12T16:14:54Z

Looks like this needs a rebase @adamruzicka

adamruzicka · 2017-07-13T06:43:35Z

@ehelms rebased

adamruzicka · 2017-08-15T11:27:56Z

@ehelms &| @dLobatog could any of you please take a look at this again?

ehelms · 2017-08-24T20:33:59Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+
+%files doc
+%doc %{gem_docdir}
+%{gem_instdir}/Gemfile


Not sure this makes sense in the -doc package

ehelms · 2017-08-24T20:34:04Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+%doc %{gem_docdir}
+%{gem_instdir}/Gemfile
+%doc %{gem_instdir}/README.md
+%{gem_instdir}/Rakefile


Not sure this makes sense in the -doc package

ehelms · 2017-08-24T20:34:34Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+%{gem_instdir}/Gemfile
+%doc %{gem_instdir}/README.md
+%{gem_instdir}/Rakefile
+%{gem_instdir}/spec


This is a test directory and can/should be excluded.

ehelms · 2017-08-24T20:35:14Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+mkdir -p %{buildroot}%{smart_proxy_dynflow_bundlerd_dir}
+cat <<EOF > %{buildroot}%{smart_proxy_dynflow_bundlerd_dir}/%{gem_name}.rb
+gem '%{gem_name}'
+EOF


This part seems a bit strange tieing this so closely to smart_proxy_dynflow. This seems like something that maybe it should be doing and let this be a stand alone gem?

So you suggest generating this file using the installer? It would definitely be cleaner

ehelms · 2017-08-24T20:35:18Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+%exclude %{gem_instdir}/net-ssh-kerberos.gemspec
+%exclude %{gem_cache}
+%{gem_spec}
+%{smart_proxy_dynflow_bundlerd_dir}/%{gem_name}.rb


This part seems a bit strange tieing this so closely to smart_proxy_dynflow. This seems like something that maybe it should be doing and let this be a stand alone gem?

ehelms · 2017-08-24T20:35:58Z

rubygem-gssapi/rubygem-gssapi.spec

+%{gem_libdir}
+%exclude %{gem_cache}
+%{gem_spec}
+%doc %{gem_instdir}/COPYING


This is in the -doc package

ehelms · 2017-08-24T20:36:10Z

rubygem-gssapi/rubygem-gssapi.spec

+%doc %{gem_instdir}/COPYING
+%doc %{gem_instdir}/Changelog.md
+%doc %{gem_instdir}/Gemfile
+%{gem_instdir}/VERSION


Seems like something that should be in the base %files section

ehelms · 2017-08-24T20:36:24Z

rubygem-gssapi/rubygem-gssapi.spec

+%doc %{gem_instdir}/Gemfile
+%{gem_instdir}/VERSION
+%{gem_instdir}/examples
+%{gem_instdir}/test


I'd omit this given they are tests.

ehelms · 2017-08-24T20:36:40Z

rubygem-gssapi/rubygem-gssapi.spec

+%{gem_instdir}/VERSION
+%{gem_instdir}/examples
+%{gem_instdir}/test
+%{gem_instdir}/Rakefile


Should this be in the %files section? This does not seem like documentation.

ehelms · 2017-08-24T20:37:01Z

rubygem-gssapi/rubygem-gssapi.spec

+
+Name: %{?scl_prefix}rubygem-%{gem_name}
+Version: 1.2.0
+Release: 4%{?dist}


Start release at 1

Because this was taken from fedora I wasn't sure if we want to start the numbering over or not. Will do

ehelms · 2017-08-24T20:37:19Z

Sorry I didn't catch those comments earlier.

ehelms · 2017-08-25T13:28:33Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+mkdir -p %{buildroot}%{gem_dir}
+cp -pa .%{gem_dir}/* \
+        %{buildroot}%{gem_dir}/
+mkdir -p %{buildroot}%{smart_proxy_dynflow_bundlerd_dir}


Still coupled to smart_proxy_dynflow

Sorry, missed that one

ehelms · 2017-08-25T19:27:54Z

rubygem-net-ssh-krb/rubygem-net-ssh-krb.spec

+%exclude %{gem_instdir}/net-ssh-kerberos.gemspec
+%exclude %{gem_cache}
+%{gem_spec}
+


Looks like you need to add %exclude %{gem_instdir}/spec

error: Installed (but unpackaged) file(s) found: /opt/theforeman/tfm/root/usr/share/gems/gems/net-ssh-krb-0.4.0/Gemfile /opt/theforeman/tfm/root/usr/share/gems/gems/net-ssh-krb-0.4.0/Rakefile /opt/theforeman/tfm/root/usr/share/gems/gems/net-ssh-krb-0.4.0/spec/integration_spec.rb /opt/theforeman/tfm/root/usr/share/gems/gems/net-ssh-krb-0.4.0/spec/spec_helper.rb

Also looks like Gemfile and Rakefile need to be included or excluded.

ehelms · 2017-08-25T19:29:20Z

rubygem-gssapi/rubygem-gssapi.spec

+%{gem_spec}
+%{gem_instdir}/Rakefile
+%{gem_instdir}/VERSION
+


Looks like you need to add %exclude %{gem_instdir}/spec

ehelms · 2017-08-30T13:50:46Z

LGTM -- @dLobatog you had outstanding change requests here

iNecas · 2017-09-06T06:47:07Z

Don't we need this rather in scl, rather than non-scl, as the smart_proxy_dynflow_core is in scl as well.

adamruzicka · 2017-09-06T16:05:18Z

@iNecas but it is in scl, or am I missing something?

EDIT: Oh, do you mean it should be named like tfm-rubygem-gssapi in the comps?

iNecas · 2017-09-06T16:35:48Z

That's it

iNecas · 2017-09-06T20:07:32Z

That's better :)

adamruzicka · 2017-09-11T14:02:38Z

@dLobatog anything else to change here?

dLobatog

Looks good to me, @adamruzicka @iNecas is this meant for 1.16 too?

Also what are these dependencies for?

dLobatog · 2017-09-25T15:37:17Z

Built for nightly for the moment

iNecas · 2017-09-25T18:40:21Z

1.16 (we're even targeting 1.15 with rex 1.3.4)

ekohl · 2017-09-28T17:17:52Z

rubygem-gssapi/rubygem-gssapi.spec

+Source0: https://rubygems.org/gems/%{gem_name}-%{version}.gem
+Requires: %{?scl_prefix_ruby}ruby(release)
+Requires: %{?scl_prefix_ruby}ruby(rubygems)
+Requires: %{?scl_prefix_ruby}rubygem(ffi) >= 1.0.1


It looks like this package is actually from the rh-ror42 SCL rather than rh-ruby22 breaking repoclosure.

I believe #1835 should fix it.

theforeman-bot added Needs testing Not yet reviewed labels May 31, 2017

adamruzicka mentioned this pull request May 31, 2017

Fixes #11936 - Allow authenticating with kerberos theforeman/foreman_remote_execution#250

Merged

domcleal added the Blocked on build label May 31, 2017

theforeman-bot removed the Not yet reviewed label Jun 7, 2017

dLobatog suggested changes Jun 7, 2017

View reviewed changes

theforeman-bot added the Waiting on contributor label Jun 7, 2017

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jun 15, 2017

ehelms added the Waiting on contributor label Jul 12, 2017

adamruzicka force-pushed the rpm/net-ssh-krb branch from 36dc386 to 0641a43 Compare July 13, 2017 06:43

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jul 13, 2017

adamruzicka added 3 commits August 14, 2017 15:29

Add rubygem-gssapi package

db81171

Add rubygem-net-ssh-krb package

a80f997

Fix comps and props

981aa4f

adamruzicka force-pushed the rpm/net-ssh-krb branch from 0641a43 to 981aa4f Compare August 14, 2017 13:31

ehelms reviewed Aug 24, 2017

View reviewed changes

adamruzicka added 2 commits August 25, 2017 09:20

Address comments

30f8f19

Loosened coupling with smart_proxy_dynflow_core

a6865fe

ehelms reviewed Aug 25, 2017

View reviewed changes

Decouple from smart_proxy_dynflow_core completely

228f9d9

ehelms reviewed Aug 25, 2017

View reviewed changes

adamruzicka added 2 commits August 28, 2017 12:41

address comments

6ae59e5

exclude gssapi tests

2da98ae

Move the gems into SCL

58023ef

iNecas approved these changes Sep 6, 2017

View reviewed changes

dLobatog approved these changes Sep 25, 2017

View reviewed changes

dLobatog merged commit 44d28f1 into theforeman:rpm/develop Sep 25, 2017

ekohl reviewed Sep 28, 2017

View reviewed changes

adamruzicka deleted the rpm/net-ssh-krb branch October 16, 2017 15:11

[RPM] Add rubygem-gssapi and rubygem-net-ssh-krb #1665

[RPM] Add rubygem-gssapi and rubygem-net-ssh-krb #1665

adamruzicka commented May 31, 2017

dLobatog left a comment

adamruzicka commented Jun 15, 2017

ehelms commented Jul 12, 2017

adamruzicka commented Jul 13, 2017

adamruzicka commented Aug 15, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

adamruzicka Aug 25, 2017 •

edited

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

ehelms Aug 24, 2017

adamruzicka Aug 25, 2017

ehelms commented Aug 24, 2017

ehelms Aug 25, 2017

adamruzicka Aug 25, 2017

ehelms Aug 25, 2017

ehelms Aug 25, 2017

ehelms Aug 25, 2017

ehelms commented Aug 30, 2017

iNecas commented Sep 6, 2017

adamruzicka commented Sep 6, 2017 •

edited

iNecas commented Sep 6, 2017

iNecas commented Sep 6, 2017

adamruzicka commented Sep 11, 2017

dLobatog left a comment

dLobatog commented Sep 25, 2017

iNecas commented Sep 25, 2017

ekohl Sep 28, 2017

ekohl Sep 28, 2017

[RPM] Add rubygem-gssapi and rubygem-net-ssh-krb #1665

[RPM] Add rubygem-gssapi and rubygem-net-ssh-krb #1665

Conversation

adamruzicka commented May 31, 2017

dLobatog left a comment

Choose a reason for hiding this comment

adamruzicka commented Jun 15, 2017

ehelms commented Jul 12, 2017

adamruzicka commented Jul 13, 2017

adamruzicka commented Aug 15, 2017

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

adamruzicka Aug 25, 2017 • edited

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

ehelms commented Aug 24, 2017

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

ehelms commented Aug 30, 2017

iNecas commented Sep 6, 2017

adamruzicka commented Sep 6, 2017 • edited

iNecas commented Sep 6, 2017

iNecas commented Sep 6, 2017

adamruzicka commented Sep 11, 2017

dLobatog left a comment

Choose a reason for hiding this comment

dLobatog commented Sep 25, 2017

iNecas commented Sep 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamruzicka Aug 25, 2017 •

edited

adamruzicka commented Sep 6, 2017 •

edited