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

(DO NOT MERGE) Aptpkg support version comparisons #19791

Closed

Conversation

ryan-lane
Copy link
Contributor

Fixes #19480. Enables support for version comparison operators

@ryan-lane
Copy link
Contributor Author

This PR has an issue. When using '>=mypackage-1.2-1' apt-get install fails with:

E: Unable to locate package "mypackage">

This has something to do with how subprocess is running the command. When I switched to using cmd.run with python_shell=True and a single string as the param it ran without issues.

@jfindlay
Copy link
Contributor

@ryan-lane, I think _parse_version could be better handled by querying dpkg-query -W -f rather than regexping the default format, something like

pkg_data = __salt__['cmd.run']('dpkg-query -W -f "${binary:Package} ${Version}\n" {0}'.format(' '.join(pkgs)))
pkg_dict = {}
for pkg in pkg_data:
    pkg_name, pkg_version = pkg.split()
    pkg_dict[pkg_name] = pkg_version
    # etc.

I'm not sure where the artifacts in your query strings are coming from, but in my experience package managers are picky about how their queries are formatted.

@jfindlay
Copy link
Contributor

I just realized that you're parsing the version requirements, '>', '=', '<', etc. so you can disregard my comment about dpkg-query, unless it can parse requirements somehow, which I doubt.

@ryan-lane
Copy link
Contributor Author

Ping. Anyone have any ideas here? I'd really like this to work, but am stuck with the subprocess issue.

@cachedout
Copy link
Contributor

Re-ping of @terminalmage who might have some good insight here. I will also try and take a look at this tomorrow.

@terminalmage
Copy link
Contributor

@ryan-lane Can you post the debug logging from the command that is being run? Particularly I am interested in the list of args being passed to cmd.run.

@terminalmage
Copy link
Contributor

Also, are you sure this even works?

root@ubuntu:~# apt-get install 'apt-utils>=2.4.2'
Reading package lists... Done
Building dependency tree
Reading state information... Done
E: Unable to locate package apt-utils>

@ryan-lane
Copy link
Contributor Author

It does if you do:

apt-get install apt-utils>=2.4.2

If you wrap it in quotes it fails.
On Jan 27, 2015 10:13 AM, "Erik Johnson" notifications@github.com wrote:

Also, are you sure this even works?

root@ubuntu:~# apt-get install 'apt-utils>=2.4.2'
Reading package lists... Done
Building dependency tree
Reading state information... Done
E: Unable to locate package apt-utils>


Reply to this email directly or view it on GitHub
#19791 (comment).

@terminalmage
Copy link
Contributor

root@ubuntu:~# apt-get install apt-utils>=2.4.2
root@ubuntu:~# ls -l
total 4
-rw-r--r-- 1 root root 578 Jan 27 21:28 =2.4.2

@terminalmage
Copy link
Contributor

@ryan-lane I don't think this is working the way you believe it is working. It appears to simply be running an apt-get install and redirecting output (see below). This results in an upgrade (as it would anyway, if available), leading you to believe that it worked when it did not.

Running with python_shell=False makes python not run the command in a shell, meaning that it will not interpret things like I/O redirection, shell expansion, and other characters with special meaning to the shell (like &).

root@ubuntu:~# apt-get install bash>4.3-7ubuntu1.4
root@ubuntu:~# cat 4.3-7ubuntu1.4
Reading package lists...
Building dependency tree...
Reading state information...
The following packages were automatically installed and are no longer required:
  dctrl-tools dmidecode git git-man liberror-perl libjs-jquery libpgm-5.1-0
  libyaml-0-2 libzmq3 python-async python-crypto python-git python-gitdb
  python-jinja2 python-m2crypto python-mako python-markupsafe python-msgpack
  python-smmap python-yaml python-zmq rsync
Use 'apt-get autoremove' to remove them.
Suggested packages:
  bash-doc
Recommended packages:
  bash-completion
The following packages will be upgraded:
  bash
1 upgraded, 0 newly installed, 0 to remove and 60 not upgraded.
Need to get 576 kB of archives.
After this operation, 0 B of additional disk space will be used.
Get:1 http://archive.ubuntu.com/ubuntu/ trusty-updates/main bash amd64 4.3-7ubuntu1.5 [576 kB]
Fetched 576 kB in 0s (1,106 kB/s)
(Reading database ... 25002 files and directories currently installed.)
Preparing to unpack .../bash_4.3-7ubuntu1.5_amd64.deb ...
Unpacking bash (4.3-7ubuntu1.5) over (4.3-7ubuntu1.4) ...
Processing triggers for man-db (2.6.7.1-1ubuntu1) ...
Setting up bash (4.3-7ubuntu1.5) ...
update-alternatives: using /usr/share/man/man7/bash-builtins.7.gz to provide /usr/share/man/man7/builtins.7.gz (builtins.7.gz) in auto mode

@ryan-lane
Copy link
Contributor Author

Hm. lame. I guess I'll change this to only support =, > and >= and compare the versions internally. Thanks for the help!

@terminalmage
Copy link
Contributor

@ryan-lane No, I don't think that will work, because I don't think that anything besides = is supported in APT. Any usage of angle brackets will be assumed to be input/output redirection. This is why you're getting the error when python_shell=False. If apt-get install supported that syntax, you would not be getting that error.

I wrote the version pinning support in aptpkg.py originally, and specifically only permitted specific versions (i.e. pkgname=version) for that reason.

In order to support version comparison like in other providers, we would need to get all available versions of the package and then include logic to install the newest available version which satisfies the version comparison expression.

I wrote a list_repo_pkgs function in yumpkg.py which can list all available versions of a given package, organized by repository. A similar function in aptpkg.py could be used to find the available versions. Once a suitable version which matches the comparison expression is found, it can be installed using the existing method (i.e. pkgname=version).

@ryan-lane
Copy link
Contributor Author

If you run apt-get install package, it will always install the newest package available. If you only support =, > and >= you can avoid apt-get update if package is >= than the version specified. If there isn't a newer package available and the version specified is greater than the version installed you fail. It's possible to do it without finding all available packages.

Mostly what I'm aiming for is the ability to do >= a known secure package without needing to do an apt-get upgrade every time.

@terminalmage
Copy link
Contributor

OK, so the idea would be to see if the current version fulfills that version comparison? What would you do in the event that a version matching the comparison expression was already installed?

@ryan-lane
Copy link
Contributor Author

Return true :)

@terminalmage
Copy link
Contributor

That would be a bad idea, since pkg.install does not return a bool.

@ryan-lane
Copy link
Contributor Author

I'd return the comparison between old and new, which would be the same.

@terminalmage
Copy link
Contributor

OK, I just wanted to make sure you weren't returning a bool. 😄

@thatch45
Copy link
Member

thatch45 commented Feb 6, 2015

@ryan-lane what is the status here? Should we close this out pending a new PR?

@thatch45 thatch45 added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 6, 2015
@ryan-lane
Copy link
Contributor Author

We're going to followup in the same PR. Most of the code changes in here are still needed. It's just one part that needs to be fixed.

@jfindlay jfindlay mentioned this pull request Mar 12, 2015
@thatch45
Copy link
Member

What is the status here? This PR has been sitting in limbo for a while. Can we close it? Should it stay in limbo for another month?
@ryan-lane

@ryan-lane
Copy link
Contributor Author

I guess let's close it out for now. I'll push another PR later when I have
this working. I haven't been able to prioritize this lately.

On Tue, Mar 17, 2015 at 4:57 PM, Thomas S Hatch notifications@github.com
wrote:

What is the status here? This PR has been sitting in limbo for a while.
Can we close it? Should it stay in limbo for another month?


Reply to this email directly or view it on GitHub
#19791 (comment).

@cachedout
Copy link
Contributor

Closed per @ryan-lane

@cachedout cachedout closed this Mar 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support comparison operators for version in pkg.installed state for apt
5 participants