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

build/ops: rpm: apply epoch only if %epoch macro is defined #15286

Merged
merged 1 commit into from Jun 1, 2017

Conversation

Projects
None yet
4 participants
@smithfarm
Contributor

smithfarm commented May 25, 2017

This allows SUSE to drop a maintenance-intensive downstream patch.

@smithfarm smithfarm added the build/ops label May 25, 2017

@smithfarm smithfarm requested review from tchaikov and ktdreyer May 25, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

Requires: ceph-mds = %{epoch}:%{version}-%{release}
Requires: ceph-mgr = %{epoch}:%{version}-%{release}
Requires: ceph-mon = %{epoch}:%{version}-%{release}
Requires: ceph-osd = %{?epoch:%{epoch}:}%{version}-%{release}

This comment has been minimized.

@tchaikov

tchaikov May 25, 2017

Contributor

can we define a variable for epoch like

%define _epoch %{?epoch:%{epoch}:}

and use it afterwards ?

This comment has been minimized.

@smithfarm

smithfarm May 25, 2017

Contributor

I think we can, and _epoch is a good name for it. I just don't know whether to use %define or %global ?

This comment has been minimized.

@ktdreyer

ktdreyer May 25, 2017

Member

%global sounds good to me

This comment has been minimized.

@tchaikov

tchaikov May 25, 2017

Contributor

okay, i was just googling for how to define a variable in rpm script, and %define standed out. if you guys think %global is better, i am all for it! =)

build/ops: rpm: apply epoch only if %epoch macro is defined
This allows SUSE to drop a maintenance-intensive downstream patch.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

Thanks for looking; modified and repushed.

@smithfarm

This comment has been minimized.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 25, 2017

@smithfarm thanks for the link!

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

I will install the packages in CentOS and SLE VMs and check the dependencies for epoch sanity.

Epoch: 1
%endif

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

https://docs.fedoraproject.org/ro/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch09s03.html

Warning
Avoid using the Epoch: directive if at all possible. It is far better to use a sane version-numbering scheme than to try to resolve the mess with epoch values. The main problems with using an epoch value are that epochs are hidden from users in most cases, and using epochs can lead to very strange-looking tasks such as a newer package with a version number that looks older than the older package.

@ktdreyer maybe we should avoid using Epoch?

This comment has been minimized.

@smithfarm

smithfarm Jun 1, 2017

Contributor

@tchaikov Apparently, once the Epoch is applied, there is no way to undo it.

This comment has been minimized.

@tchaikov

tchaikov Jun 1, 2017

Contributor

@smithfarm thanks! got it, no U turn then =(

This comment has been minimized.

@ktdreyer

ktdreyer Jun 1, 2017

Member

Right :(

@tchaikov tchaikov merged commit 92ebb70 into ceph:master Jun 1, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@smithfarm smithfarm deleted the smithfarm:wip-cond-epoch branch Jun 1, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 1, 2017

FYI: This broke package installation on centos/rhel, yum can no longer resolve the dependencies because the versions do not match. My first guess is that we are missing : character in the versions.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 1, 2017

Anyway, I am not sure what is the issue here? Why don't you just define epoch to 0 (the default epoch) if you do not want to bump it? The rpm treat "no epoch" the same way as epoch 0. If you did not want to bump the epoch, you could just conditionally make it 0.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 1, 2017

btw: We (well I) originally introduced Epoch because somebody pushed unstable version of ceph into epel and that overrode the packages provided by upstream. At that point, that was pretty much the only way to solve the problem in a sane way (yum-plugin-priorities were still broken, ...).

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 1, 2017

Yeah, I was going to test it on CentOS first, before merging :-(

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 1, 2017

Maybe the %global is too far up in the file? Opening another PR to address.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 2, 2017

Thanks, that other PR helped and I can now install the packages.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

Thanks, that other PR helped and I can now install the packages.

Yeah, I was sweating bullets early this morning :-) Good to know it's fixed.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

Anyway, I am not sure what is the issue here? Why don't you just define epoch to 0 (the default epoch) if you do not want to bump it?

I believe I tried explicitly setting Epoch: 0 and OBS refused to build it. SUSE does not allow epochs in the spec file at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment