seporaitis / yum-s3-iam

Yum package manager plugin for private S3 repositories. Uses Amazon IAM & EC2 Roles.
Apache License 2.0
162 stars 99 forks source link

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

Open marcindulak opened 6 years ago

marcindulak commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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