weixin_39962394
weixin_39962394
2020-12-28 19:35

[policies] Reset to expected variable order for packagemanager

Commit 82e4ee2 changed the order of variables in packagemanager to what none of the policies expected. This sets it back so query is always first. Red Hat policy is the only one that uses more than one variable for it.

Closes: #1155

Signed-off-by: Bryan Quigley

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • [X] Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • [X] Is the subject and message clear and concise?
  • [X] Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • [X] Does the commit contain a Signed-off-by: First Lastname ?

该提问来源于开源项目:sosreport/sos

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

9条回答

  • weixin_39700220 weixin_39700220 4月前

    -cymru
    Ok thanks, I'll then do a PR with the above later today.

    点赞 评论 复制链接分享
  • weixin_39901943 weixin_39901943 4月前

    Sounds good! 👍

    点赞 评论 复制链接分享
  • weixin_39700220 weixin_39700220 4月前

    Done via #1162

    点赞 评论 复制链接分享
  • weixin_39962394 weixin_39962394 4月前

    Closing this as #1162 is the better way to fix this.

    点赞 评论 复制链接分享
  • weixin_39901943 weixin_39901943 4月前

    Yes: that's a better fix.

    I'll look over this some more though: reading the verify patch back there's a load of completely unnecessary "x = foo if foo else None.." that appears completely redundant.

    点赞 评论 复制链接分享
  • weixin_39700220 weixin_39700220 4月前

    Dear maintainer,

    Do we have an idea as of when this patch will be merge upstream ?

    We would like to release a new sosreport version in Ubuntu beginning of Jan 2018 and this bug is a blocker for us at the moment.

    Regards, Eric

    点赞 评论 复制链接分享
  • weixin_39901943 weixin_39901943 4月前

    we don't have a release planned until April 2018 - you can find our release schedules in the GitHub milestones.

    I've approve this commit, but before it's merged I want us to also address the bug in the Debian policy that caused it to be a problem in the first place.

    If you're happy to pick those patches up from Git and include them in your update, that's fine. Otherwise we can speak to the other Ubuntu and Debian contributors and see about making a point release to include this.

    点赞 评论 复制链接分享
  • weixin_39700220 weixin_39700220 4月前

    -cymru

    Are you referring to what I have posted in PR #1155 ?

    
    diff --git a/sos/policies/debian.py b/sos/policies/debian.py
    index dc7f2e6..258b841 100644
    --- a/sos/policies/debian.py
    +++ b/sos/policies/debian.py
    @@ -10,8 +10,9 @@ class DebianPolicy(LinuxPolicy):
         vendor_url = "http://www.debian.org/"
         report_name = ""
         ticket_number = ""
    -    package_manager = PackageManager(
    -        "dpkg-query -W -f='${Package}|${Version}\\n'")
    +    _debq_cmd = "dpkg-query -W -f='${Package}|${Version}\\n'"
    +    _debv_cmd = "dpkg --verify" 
    +    _debv_filter = ""
         valid_subclasses = [DebianPlugin]
         PATH = "/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games" \
                + ":/usr/local/sbin:/usr/local/bin"
    @@ -20,8 +21,11 @@ class DebianPolicy(LinuxPolicy):
             super(DebianPolicy, self).__init__(sysroot=sysroot)
             self.report_name = ""
             self.ticket_number = ""
    -        self.package_manager = PackageManager(
    -            "dpkg-query -W -f='${Package}|${Version}\\n'")
    +        self.package_manager = PackageManager(query_command=self._debq_cmd,
    +                                              verify_command=self._debv_cmd,
    +                                              verify_filter=self._debv_filter,
    +                                              chroot=sysroot)
    +
             self.valid_subclasses = [DebianPlugin]
    
    点赞 评论 复制链接分享
  • weixin_39901943 weixin_39901943 4月前

    Not directly but those changes cover what I'd like to see: thanks.

    It's not really about aligning to the RHEL policy though, it's just about using the Python interface in the way it is written: Python does not impose any kind of ordering on keyword args, but if the keywords are missing it will still attempt to honour the call by mapping positional call arguments to corresponding keywords in the declaration.

    This behaviour can be unhelpful since at times it masks buggy use of an interface as is the case here.

    in this case the Debian policy invokes the PackageManager call as though it only uses positional arguments. This is undoubtedly wrong and a bug and needs to be fixed.

    The change requested here is actually purely cosmetic: it's just about reverting to the historic argument ordering for this method, which I agree is a positive change since there was never any good reason to have messed around with it, but that is entirely separate from the Debian policy's historical misuse of this interface.

    点赞 评论 复制链接分享

相关推荐