https://bugzilla.redhat.com/show_bug.cgi?id=1826621

Neil Horman <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(arunprabhu.vijaya
                   |                            |[email protected])



--- Comment #12 from Neil Horman <[email protected]> ---
Thank you for your quick reply. In response:

>[Arun] 
>1. Sure. We will add all packages present in fedora as BuildRequires and drop 
>from go.mod file & other packages as bundled exception in go.mod file.
Perfect, thank you!
>2. We will also add BuildRequires : git-core & BuildRequires : go-rpm-macros. 
Excellent

>Note: In my fedora machine,am getting this error if I add go-rpm-macros. Can 
>you please let me why I'm getting this error?
>
>[root]# dnf install go-rpm-macros
>Last metadata expiration check: 3:07:12 ago on Wed 13 May 2020 00:52:11 IST.
>No match for argument: go-rpm-macros
>Error: Unable to find a match: go-rpm-macros
Hmm...odd, what Fedora release are you running on?  I managed to install it on
my F32 system just fine.  I wonder if you're using an older fedora release that
doesn't have those macros packaged yet?

>> >1.rmd.x86_64: W: pem-certificate /usr/local/etc/rmd/acl/roles/admin/cert.pem
>> >This is not fixed as these files are only used for testing.
>> I'm not sure I follow here.  What testing are you referring to?  I don't see
>> any %check script in your spec file, so I'm not sure what testing you are
>> doing.  As such, why not just not package the pem certificates at all?

>[Arun] PEM certificates can be used as reference by System Admin to configure. 
>So, it is more like reference files or fast and easy check of functionality.. 
>Sorry for mentioning it as used for testing >purposes.
>https://github.com/intel/rmd/blob/master/etc/rmd/rmd.toml  has reference usage 
>in [default] section. Please refer to it.
Hmm, ok, I see the need, but I'll need to check a little bit on what the policy
is here.  I would hate to have people use this with a default certificate in
your name.  I expect we can mark those in the file manafest with a %conf, and
make a note in the documentation and the config file that production systems
should generate their own cert, but I'll check and confirm.

>> >2. Below error is not fixed as our SW needs to copy config files to 
>> >/usr/local/etc/
>> > rmd.x86_64: E: dir-or-file-in-usr-local /usr/local/etc/rmd/policy.toml
>> >A file in the package is located in /usr/local. It's not permitted for
>> >packages to install files in this directory.
>> >Added exception in rpmlint --> addFilter("E: dir-or-file-in-usr-local") in 
>> >/etc/rpmlint/config
>> Why?  It looks to me like the rmd utility allows you to specify the location
>> of a config file on the command line, so I don't understand why the config
>> files can't be placed in the appropriate directory (%_sysconfdir}/etc) here.
>> Note also that, if you add a systemd unit file as noted in comment 5, you
>> can set the config directory there without having to modify the source
>> defaults.

>[Arun] If all config files need to be moved to /etc/, we will move them from 
>/usr/local/etc/ folder. We also have some install related scripts which needs 
>to be moved from /usr/local/etc/rmd to /etc/rmd. >Please let us know if it is 
>OK?
Yes, that is exactly correct.  You are completely permitted to create
subdirectories under /etc to store your config files in, it just all needs to
be rooted in /etc (or more specifically %{_sysconfdir}, which currently expands
to /etc)

>> The review of the package itself looks ok, though you didn't address the
>> following items from comment 5:
>> [!]: %config files are marked noreplace or the reason is justified.
>>      Note: No (noreplace) in %config(missingok) /usr/bin/scripts
>>      %config(missingok) /usr/bin/etc>
>
>[Arun] This was removed from the latest spec file as we corrected in the spec 
>file by installing directly from the SPEC file in the correct path instead of 
>running script from /usr/bin directory which will >copy these files to 
>/usr/local/etc/. 
Understood, thank you!

>> [!]: Package contains systemd file(s) if in need.  <= you probably want to
>> add a systemd unit to start rmd
>
>[Arun] We will discuss internally and get back if its needed to keep the 
>systemd file for this release.
I'm sorry, I'm not sure I understand.  I didn't see a systemd unit file
previously (hence the error).  I presume you want to run rmd as a daemon,
correct?  If so, I would strongly recommend that you add one, so you have a
good user experience when the package is installed.


>> [!]: Package is not known to require an ExcludeArch tag. <= It should be
>> buildable on non-x86 platforms, you either need to allow that, or document
>> the fact that rmd only operates on data found in x86 systems
>> 
>[Arun] RMD addresses Resource Management for Intel X86 architectures. HW 
>support is documented in 
>https://github.com/intel/rmd/blob/master/docs/Prerequisite.md file. Please let 
>us know if this Ok or needs >further documentation.
Yes, the documentation is fine, if you could just add a comment in the spec
file above the ExclusiveArch tag pointing to that URL, that would be great

>> [!]: Package functions as described. <= description in spec file needs to be
>> more verbose

>[Arun] We added description in the SPEC file in the last version. It has the 
>details, kindly let us know if something more needs to be added. I saw the 
>Lint warning disappear after addition in the >description.
This is the description that was in the latest spec file that you submitted:
%description
Resource Management Daemon (RMD) is a system daemon running on Linux platforms

I'm asking that you provide a little more description around what RMD actually
does (i.e. why people might want to run it).


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]

Reply via email to