[Bug 1773720] Review Request: golang-github-haproxytech-dataplaneapi - HAProxy Data Plane API

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

 



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



--- Comment #12 from Brandon Perkins <bperkins@xxxxxxxxxx> ---
(In reply to Ryan O'Hara from comment #11)
> (In reply to Brandon Perkins from comment #10)
> > (In reply to Ryan O'Hara from comment #9)
> > 
> > The /etc/logrotate.d directory is owned by the 'logrotate' package:
> > 
> > $ rpm -qf /etc/logrotate.d
> > logrotate-3.15.1-1.fc31.x86_64
> > 
> > This issue is properly satisfied by the logrotate 'Suggests' in the RPM:
> > 
> > $ grep ^Suggests: SPECS/golang-github-haproxytech-dataplaneapi.spec 
> > Suggests: logrotate
> > $ rpm -qp --suggests
> > RPMS/golang-github-haproxytech-dataplaneapi-1.2.4-5.fc31.x86_64.rpm 
> > logrotate
> > 
> > To me, it's better to have a possible orphan directory than to have this
> > package become the owner of the directory.  And, we certainly wouldn't be
> > the first to go down this path.  Quick query shows me:
> > 
> > [bperkins@bperkins haproxytech]$ dnf repoquery --queryformat="%{NAME}"
> > --whatsuggests logrotate
> > mariadb-server
> > plymouth
> > 
> > However, many more do the ownership thing (which just seems wrong to me):
> > 
> > [bperkins@bperkins haproxytech]$ dnf repoquery --queryformat="%{NAME}"
> > --whatprovides /etc/logrotate.d
> > bes
> > copr-dist-git
> > gap-pkg-scscp
> > gerbera
> > kdm-settings
> > lightdm
> > logrotate
> > macromilter
> > openqa
> > ppp
> > psad
> > samba-common
> > sssd-common
> > yast2-filesystem
> > 
> > Or, we could go down what I *really* think is wrong and just ignore the
> > issue completely (which is by far the most popular path).
> > 
> > I'm personally inclined to do what I did, but I can certainly change it.
> 
> Another option would be to *require* logrotate as a dependency. Thoughts?
> I'm on the fence with this one.
> 

Yeah, I thought about that as well.  When I reviewed the command run again, it
is in fact logging by default (which HAProxy does not do by default).  So, log
rotation in this case should be the default and not the exception.  I'll go
ahead and make that change.

> > 
> > Using the %gopkg macro, I don't see how this could be accomplished.  This
> > really doesn't seem like a critical requirement to me.
> 
> I know the above warning is complaining about about the -devel subpackage,
> but I am more curious if we can do something like this:
> 
> BuildRequires:  golang(github.com/haproxytech/config-parser) >= 1.2.0
> 

I'm happy to make that change and I'll do it for client-native as well.

> Sorry, I should have been more specific. On a related note, I saw that the
> dataplaneapi spec file has this requirement:
> 
> Requires:         haproxy >= 1.9
> 
> We might want to make that 2.0 unless we've tested this with 1.9 -- the API
> might have changed. Plus, I never put 1.9 in Fedora.
> 

Will make that change.

> > 
> > Because of using the %gopkg macro, we're kind of stuck with what it creates.
> > Outside of modifying the macro (which is a non-starter), the only thing that
> > could be done would be to shorten the package name, which is also a
> > non-starter.  I think it's an error we just have to live with.  Unless you
> > have any other thoughts.
> 
> I think at the very least we should file a bug against whatever is defining
> those macros and see if they can prevent this from happening.

Bug entered as: https://bugzilla.redhat.com/show_bug.cgi?id=1823915


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