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

un-SCL-ify hammer-cli-katello #10731

Merged
merged 2 commits into from
May 17, 2024

Conversation

ianballou
Copy link

Makes the hammer-cli-katello spec file no longer use ye olde SCL macros from the EL7 days.

@ianballou ianballou changed the title Bump rubygem-hammer_cli_katello to 1.12.0-1 un-SCL-ify hammer-cli-katello Apr 25, 2024
@ianballou ianballou force-pushed the un-scl-hammer-cli-katello branch from 6598c6e to 028f166 Compare April 25, 2024 18:37
Comment on lines 23 to 24
Requires: rubygem(hammer_cli_foreman)
Requires: rubygem(hammer_cli_foreman_tasks)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look at the autorequires. This may be redundant but then we need to update the template too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking again, I think you did this by hand. In 3f4c042 we redid this section to rely on autorequires.

Suggested change
Requires: rubygem(hammer_cli_foreman)
Requires: rubygem(hammer_cli_foreman_tasks)

While we're on the topic of updates, in 10e0424 we started to add Provides for plugins. Could you add that too?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to point out (once again) that since Katello/hammer-cli-katello#924 the nightly packages have been broken: https://ci.theforeman.org/blue/organizations/jenkins/hammer-cli-katello-master-source-release/detail/hammer-cli-katello-master-source-release/204/pipeline

Given they've been broken for 3 months, can we assume they're not tested anyway and we can change it regular gems?

@ianballou
Copy link
Author

Given they've been broken for 3 months, can we assume they're not tested anyway and we can change it regular gems?

There is a testing queue for hammer-cli-katello issues that has built up over the 3 months, but it's small enough for people not to be clamoring about it. We would like to keep the h-c-k nightlies going. Last I heard, Zach was taking a look.

@ekohl
Copy link
Member

ekohl commented Apr 26, 2024

You'll need to recreate Gemfile and define a testing group. Jenkins now doesn't install the development group

@ianballou
Copy link
Author

I added a test group to https://github.com/Katello/hammer-cli-katello/pull/934/files

@ianballou ianballou force-pushed the un-scl-hammer-cli-katello branch 2 times, most recently from 5e0cad8 to 7ae4d44 Compare May 10, 2024 20:12
@ianballou
Copy link
Author

Let's merge #10767 first to unblock nightly if this PR still has issues.

@evgeni evgeni force-pushed the un-scl-hammer-cli-katello branch from 7ae4d44 to 874c84e Compare May 13, 2024 07:11
%{?scl:scl enable %{scl} - << \EOF}
gem build %{gem_name}.gemspec
%{?scl:EOF}
gem build ../%{gem_name}-%{version}.gemspec
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
gem build ../%{gem_name}-%{version}.gemspec
gem build ../%{gem_name}-%{version}%{?prerelease}.gemspec

I think that's the last place missing prerelease.

BuildArch: noarch
Provides: %{?scl_prefix}rubygem(%{gem_name}) = %{version}
Provides: rubygem(%{gem_name}) = %{version}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is automatic nowadays and was dropped in 3f4c042

Comment on lines 23 to 24
Requires: rubygem(hammer_cli_foreman)
Requires: rubygem(hammer_cli_foreman_tasks)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking again, I think you did this by hand. In 3f4c042 we redid this section to rely on autorequires.

Suggested change
Requires: rubygem(hammer_cli_foreman)
Requires: rubygem(hammer_cli_foreman_tasks)

While we're on the topic of updates, in 10e0424 we started to add Provides for plugins. Could you add that too?

@ianballou ianballou force-pushed the un-scl-hammer-cli-katello branch from 874c84e to 8dbf75f Compare May 15, 2024 20:53
@ianballou
Copy link
Author

@ekohl to address the comments about items being different from what the spec automation provides, I rebased, regenerated the entire spec file, and added back in the nightly bits.

@ianballou ianballou force-pushed the un-scl-hammer-cli-katello branch 2 times, most recently from a2a824b to 2215367 Compare May 15, 2024 20:58
@ianballou
Copy link
Author

[2024-05-15T20:58:50.452Z] TASK [Check for a new changelog entry] *****************************************

[2024-05-15T20:58:50.452Z] fatal: [rubygem-hammer_cli_katello]: FAILED! => 

[2024-05-15T20:58:50.452Z]     changed: false

[2024-05-15T20:58:50.452Z]     changelog_epoch_version_release: ''

[2024-05-15T20:58:50.452Z]     msg: Could not find specfile

[2024-05-15T20:58:50.452Z]     specfile_epoch_version_release: ''

I'm unsure why the specfile cannot be found.

@evgeni
Copy link
Member

evgeni commented May 15, 2024

It added another hammer cli katello to plugins in comps/manifest, that confuses it.

@evgeni evgeni force-pushed the un-scl-hammer-cli-katello branch from 2215367 to d80efc9 Compare May 16, 2024 06:52
@evgeni
Copy link
Member

evgeni commented May 16, 2024

I've dropped those additional entries and it built \o/

@ekohl mind having another look?

@ekohl ekohl merged commit 0db3a62 into theforeman:rpm/develop May 17, 2024
4 checks passed
@ianballou ianballou deleted the un-scl-hammer-cli-katello branch May 17, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants