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

use noreplace for config; set default values of name, version, release #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcindulak
Copy link

I needed to build an RPM of this plugin, and prefer not to pass any arguments to rpmbuild.

See https://copr.fedorainfracloud.org/coprs/marcindulak/yum-plugin-s3-iam/
If someone prefers to pass name, version, release on the cli it is still possible as before this commit with, e.g.:

rpmbuild -bb --define "_sourcedir $PWD" --define 'version 1.2.3' yum-plugin-s3-iam.spec

Another necessary change was to use config noreplace for config files, otherwise the config file will be overwritten by a newer RPM version installed.

@nkadel
Copy link

nkadel commented Jun 6, 2018

The double use of "name", "version", and "release" as variable is unnecessary and confusing, as you've attempted to address in your patch. It is simply not a normal practice in any popular SRPM's. All of those values can be simply hardcoded in the .spec file, as they are in almost all RPM source code, or set with a *distinct variable name, as they are in samba.spec. samba.spec uses a hardcoded "Name:", and a "samba_version" "samba_release" variables used to set "Version:" and "Release:" in a consistent and legible fashion.

In addition, "Release:" should really include "%{?dist}" to shtat versions compiled for different releases get the dist setting. Take a look at samba.spec, from Fedora or CentOS for examples.

@nkadel
Copy link

nkadel commented Jun 6, 2018

Switching the config files to "noreplace" is quite sensible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants