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

query madison API directly to find source package (Closes: #909753) #122

Merged
merged 1 commit into from Sep 29, 2018

Conversation

Projects
None yet
3 participants
@anarcat
Contributor

anarcat commented Sep 27, 2018

The tool used by the JavaScript team to track progress on packaging,
js_task_edit1, has trouble dealing with binary packages that have a
different name than the source package. For example, in the OpenPGP
packaging page2, a bunch of babel packages are marked as missing in
Debian even though they are actually present (e.g. babel-code).

This is because of that bit of rmadison code in mapper.py:

    madison = _getstatusoutput(
        'rmadison -u debian "%s" | tac | grep source' % result['name'])

If you look at the output for babel-core, you'll see this can't
possibly work:

$ rmadison -u debian node-babel-core
node-babel-core | 6.26.0+dfsg-3 | testing    | all
node-babel-core | 6.26.0+dfsg-3 | unstable   | all

The "-S" does not help here: it would show binary packages if we
somehow magically found the right source package, but not the
reverse. There's no way the commandline rmadison tool can give us that
information, because that's a limitation of the API.

At least that's what I thought at first. As it turns out, there's an
undocumented python output format hidden deep in the entrails of
Dak3 which, in turn, actually indicate the source package associated
with any binary package found. This nasty business requires us to do
an actual HTTP query in Python, but rmadison does that anyways, so
doing so is not really slower.

This patch fixes the problem: return the source package instead of the
binary package and things all fall into place. As a bonus, we sort the
version numbers with Python's distutils which should be more reliable
than tac (if that's even what that thing was doing).

One downside is that this might return testing instead of
unstable if both have the same version (because that's the first
match madison returns, and why tac was used) but I would counter
it's more interesting to find the older suite with the latest version
than the later suite, as that's obviously always unstable. This
gives more information about the state in Debian (e.g. it might even
be stable if we're lucky!)

@pravi

This comment has been minimized.

Contributor

pravi commented Sep 27, 2018

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 27, 2018

so this is what I mentioned in the last graf of the commitlog:

One downside is that this might return testing instead of unstable if both have the same version (because that's the first match madison returns, and why tac was used) but I would counter it's more interesting to find the older suite with the latest version than the later suite, as that's obviously always unstable. This gives more information about the state in Debian (e.g. it might even be stable if we're lucky!)

This is by design. I'm happy to learn the thing is in backports! It means the package will work even down in backports. Now, if the version in backports that shows up is lower than the version in unstable, that's another problem, and one we'd have to deal with possibly in other scenarios (e.g. security updates) anyways. This, specifically, comes up because Python's distutils.version does not know about Debian-specific version syntax like ~. I wonder if we could pull in python-apt to get such version comparison routines or if there's a better one that could handle this correctly?

@pravi

This comment has been minimized.

Contributor

pravi commented Sep 27, 2018

This seems to be not a big issue, only cosmetic issue as can be seen in the example below. It is showing the highest upstream version.

pravi@nishumbha:~$ npm2deb view glob
Name:                                   glob
Version:                                7.1.3
Description:                            None
Homepage:                               https://github.com/isaacs/node-glob#readme
License:                                ISC
Debian:                                 node-glob (7.1.3-1) (testing)
pravi@nishumbha:~$ rmadison node-glob
node-glob  | 4.0.5-1~bpo70+1 | wheezy-backports   | source, all
node-glob  | 4.0.5-1         | oldstable          | source, all
node-glob  | 4.0.5-1         | oldstable-kfreebsd | source, all
node-glob  | 7.1.1-1         | stable             | source, all
node-glob  | 7.1.3-1         | testing            | source, all
node-glob  | 7.1.3-1         | unstable           | source, all

@pravi

pravi approved these changes Sep 27, 2018

Tested OK.

@pravi

This comment has been minimized.

Contributor

pravi commented Sep 27, 2018

@anarcat further testing lead to this error.


 npm2deb create @ava/write-file-atomic
Traceback (most recent call last):
  File "/usr/bin/npm2deb", line 7, in <module>
    sys.exit(main(sys.argv))
  File "/usr/lib/python3/dist-packages/npm2deb/scripts.py", line 157, in main
    args.func(args)
  File "/usr/lib/python3/dist-packages/npm2deb/scripts.py", line 293, in create
    npm2deb.start()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 77, in start
    self.create_control()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 277, in create_control
    args['Depends'] = self._get_Depends()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 538, in _get_Depends
    name = mapper.get_debian_package(dep)['name']
  File "/usr/lib/python3/dist-packages/npm2deb/mapper.py", line 82, in get_debian_package
    if distutils.version.LooseVersion(vers) > result['version']:
  File "/usr/lib/python3.6/distutils/version.py", line 64, in __gt__
    c = self._cmp(other)
  File "/usr/lib/python3.6/distutils/version.py", line 337, in _cmp
    if self.version < other.version:
TypeError: '<' not supported between instances of 'int' and 'str'
@pravi

Test npm2deb create @ava/write-file-atomic and fix.

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 27, 2018

that's distutils freaking out on Debian's version numbering scheme. it tries to compare ":" with 1 in the version numbers and, obviusly, fails. a workaround is to avoid using an epoch in the default version number, but this goes back to our previous discussion about comparing debian version numbers correctly - there might be other cases this fails, unfortunately - especially with packages with epochs. :(

@anarcat anarcat force-pushed the anarcat:find-source branch from 196c6c1 to 08a4cd6 Sep 27, 2018

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 27, 2018

the new commit fixes the immediate error, but we should find some better parser for debian version strings somewhere. python-apt's package.Version object could provide what we need, but i wonder if it's okay to add a new dependency...

@pravi

This comment has been minimized.

Contributor

pravi commented Sep 28, 2018

I think python-apt would be fine to add.

@pravi
npm2deb create @ava/write-file-atomic
Traceback (most recent call last):
  File "/usr/bin/npm2deb", line 7, in <module>
    sys.exit(main(sys.argv))
  File "/usr/lib/python3/dist-packages/npm2deb/scripts.py", line 157, in main
    args.func(args)
  File "/usr/lib/python3/dist-packages/npm2deb/scripts.py", line 293, in create
    npm2deb.start()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 77, in start
    self.create_control()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 277, in create_control
    args['Depends'] = self._get_Depends()
  File "/usr/lib/python3/dist-packages/npm2deb/__init__.py", line 538, in _get_Depends
    name = mapper.get_debian_package(dep)['name']
  File "/usr/lib/python3/dist-packages/npm2deb/mapper.py", line 83, in get_debian_package
    if vers > result['version']:
NameError: name 'vers' is not defined

Seems like a simple typo. Changing vers to version fixes the issue.

for suite, versions in packages[0][result['name']].items():
for version, source in versions.items():
version = distutils.version.LooseVersion(version)
if vers > result['version']:

This comment has been minimized.

@pravi

pravi Sep 28, 2018

Contributor

typo: 'vers' should be 'version'

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 28, 2018

right - i renamed a few variables at some point and probably screwed it up... i'll factor in the python-apt depends so we get proper version comparison anyways and get back to you.

@anarcat anarcat force-pushed the anarcat:find-source branch from 08a4cd6 to 0272f16 Sep 28, 2018

query madison API directly to find source package (Closes: #909753)
The tool used by the JavaScript team to track progress on packaging,
js_task_edit[1], has trouble dealing with binary packages that have a
different name than the source package. For example, in the OpenPGP
packaging page[2], a bunch of babel packages are marked as missing in
Debian even though they are actually present (e.g. babel-code).

This is because of that bit of rmadison code in mapper.py:

        madison = _getstatusoutput(
            'rmadison -u debian "%s" | tac | grep source' % result['name'])

If you look at the output for `babel-core`, you'll see this can't
possibly work:

    $ rmadison -u debian node-babel-core
    node-babel-core | 6.26.0+dfsg-3 | testing    | all
    node-babel-core | 6.26.0+dfsg-3 | unstable   | all

The "-S" does not help here: it would show binary packages if we
somehow magically found the right source package, but not the
reverse. There's no way the commandline rmadison tool can give us that
information, because that's a limitation of the API.

At least that's what I thought at first. As it turns out, there's an
undocumented `python` output format hidden deep in the entrails of
Dak[3] which, in turn, actually indicate the source package associated
with any binary package found. This nasty business requires us to do
an actual HTTP query in Python, but rmadison does that anyways, so
doing so is not really slower.

This patch fixes the problem: return the source package instead of the
binary package and things all fall into place. As a bonus, we sort the
version numbers with Python's distutils which should be more reliable
than `tac` (if that's even what that thing was doing).

One downside is that this might return `testing` instead of `unstable`
if both have the same version (because that's the first match madison
returns, and why `tac` was used) but I would counter it's more
interesting to find the older suite with the latest version than the
later suite, as that's obviously always `unstable`. This gives more
information about the state in Debian. For example, it might even be
`stable` (but not `backports`) if we're lucky!

[1]: https://salsa.debian.org/js-team/js-task-wiki-edit
[2]: https://wiki.debian.org/Javascript/Nodejs/Tasks/openpgp
[3]: https://ftp-team.pages.debian.net/dak/epydoc/dak.daklib.ls-pysrc.html#L77

@anarcat anarcat force-pushed the anarcat:find-source branch from 0272f16 to 8065ff3 Sep 28, 2018

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 28, 2018

alright, after a little bit of fighting with apt_pkg, i got it to work and we should have proper version comparisons now:

$ npm2deb view glob
Name:                                   glob
Version:                                7.1.3
Description:                            None
Homepage:                               https://github.com/isaacs/node-glob#readme
License:                                ISC
Debian:                                 node-glob (7.1.3-1) (testing)
$ npm2deb view 'write-file-atomic'
Name:                                   write-file-atomic
Version:                                2.3.0
Description:                            None
Homepage:                               https://github.com/iarna/write-file-atomic
License:                                ISC
Debian:                                 node-write-file-atomic (2.3.0-1) (testing)

i believe that, before the patch, the latter would have shown the version from backports (or crashed, i'm not sure). :) It would arguably have been great to have the backports version in there, but I think it's better to have a consistent and reliable codebase than rely on Python's incomplete comparison algorithms that might crash on Debian versions.

@pravi

pravi approved these changes Sep 28, 2018

Tested OK. Looks good. @anarcat thanks!

cc @shanavas786

@shanavas786

(y)

@shanavas786 shanavas786 merged commit 2eb559c into LeoIannacone:master Sep 29, 2018

@anarcat anarcat deleted the anarcat:find-source branch Sep 29, 2018

@anarcat

This comment has been minimized.

Contributor

anarcat commented Sep 29, 2018

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment