[Bug 1826621] Review Request: RMD - Resource Manager Deamon

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Neil Horman <nhorman@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(arunprabhu.vijaya
                   |                            |n@xxxxxxxxx)



--- Comment #9 from Neil Horman <nhorman@xxxxxxxxxx> ---
Its closer, but the package still has some problems:

It doesn't build, as your build script still attempts to fetch upstream sources
(via the go build command).  you need to add git-core as a build requirement to
get past that.

The package still tries to pull in many packages from upstream as well, instead
of depending on them from the corresponding fedora packages.  Based on this:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/#_bundled_or_unbundled

You need to justify why you need to pull those packages in from upstream
instead of just using the Fedora packages.  By my read, currently of all the
package dependencies listed in your go.mod file, the following packages:
go-spew
monotime
go-querystring
go-interpol
httpmock
go-isatty
go-colorable
http2curl
gi-buffruneio
to-toml
go-difflib
gostub
go-diff
fasthttp

are not yet packaged by fedora, and so would be allowable as bundled
exceptions.  Every other package in go.mod will need a justification here (or
preferably, just drop the dependency in go.mod, and add the requisite
buildrequires to the spec file instead

regarding the errors you mentioned above:


>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?


>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.


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

[!]: Package contains systemd file(s) if in need.  <= you probably want to add
a systemd unit to start rmd


[!]: 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


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


-- 
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 -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
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/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux