Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More multiple-version support for pkg.list_pkgs() #2837

Merged
merged 9 commits into from
Dec 10, 2012

Conversation

terminalmage
Copy link
Contributor

I did zypper and both yumpkg modules. I have a few important notes to make, though:

  1. I changed the yum modules to parse rpm instead of yum. This provides a considerable performance boost because the yum overhead is not needed when all you want is to list the installed packages. I saw my test VM go from ~10 secs to run list_pkgs to 1 sec.

  2. (IMPORTANT) The yumpkg modules have been changed so that the package architecture is displayed in the version string. This is important because RedHat-based systems like Fedora, CentOS, and RHEL don't name the x86_64 and i686 versions of a package differently, they'll just be the same name but with different architectures. Note that this change was not needed for zypper (another RPM-based provider) since Suse puts "-32bit" in the 32-bit versions of packages that get installed on x86_64 hosts.

  3. Due to salt job queries #2 above, we may want to think of a way to make salt use the minion's "cpuarch" grain to manually specify the arch to install when no arch is provided, to keep backwards compatibility in states. I'll bring this up on the mailing list for further discussion.

  4. The previous version of yumpkg.py's list_pkgs function can be found here: https://gist.github.com/4238643

Also, break out the logic that converts the string value to a list of
strings when there are multiple packages and put it in pkg_resource.py
so that the other package providers can use it.
I also switched list_pkgs so that it parses the rpm command's output
instead of using yum. This reduces a lot of yum overhead and speeds up
this function considerably.
This is important because many packages have versions for both x86_64
and i686 installed.
@SEJeff
Copy link
Contributor

SEJeff commented Dec 9, 2012

For #3, I believe that is left up to yum to decide on redhat based systems. If I recall correctly, yum looks at the version of glibc you have installed. If you have both 32 and 64 bit glibc installed, it will install the package for both architectures if it is available often times. Where possible, I like salt doing as little magic as possible. Besides, how will salt know if the 32 bit package is i686 or i386 for 32 bit packages? Additionally, it is nice as an admin using salt to have it install foo.x86_64 or foo.i686 that easily in a state.

@terminalmage
Copy link
Contributor Author

OK, per my discussion with @thatch45 I'm rolling back the multiple-version support (as well as arch-specific support for yum) and we'll shoot for 0.10.7. This req should be ready to be merged.

Note that all the pkg providers with the exception of openbsdpkg have been modified to use a common function from pkg_resource.py to add packages to the dict that list_pkgs eventually returns. Presently this function just does what the other package providers were already doing (setting pkgs[name] = version), but once we're ready to implement multiple-version support, pkg_resource.add_pkg can be modified to use the code I have commented out, and multiple versions of the same package will be supported.

I've tested all platforms that I modified (except for Gentoo) and confirmed that they work.

Note that merging this will cause a merge conflict, since @SEJeff and I both did cleanup of StringTypes on some of the same areas. One of the places he cleaned up code was in pacman.py. The logic from this function was broken out in my branch and put into pkg_resource to provide a common interface for the other pkg providers. This part of the code will definitely be a merge conflict, and my version should be the one used to resolve the conflict. The rest of the conflicts, if there are any, would likely be in pkg_resource.py and should be pretty straightforward in their resolution.

@thatch45 thatch45 merged commit 868aa1b into saltstack:develop Dec 10, 2012
@thatch45
Copy link
Contributor

Please double check that the merge in pacman.py is clean

@terminalmage
Copy link
Contributor Author

The type check is not needed, it is part of pkg_resource.add_pkg now. Also, I put back a try/except that @SEJeff had me add to keep malformatted lines from causing a traceback.

See #2854

@thatch45
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants