Bug 701047 - mirrormanager version not reported in metalink attributes
Summary: mirrormanager version not reported in metalink attributes
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: mirrormanager
Version: 15
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Matt Domsch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-30 15:43 UTC by Steve Tyler
Modified: 2011-11-18 04:29 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-18 04:29:05 UTC
Type: ---


Attachments (Terms of Use)

Description Steve Tyler 2011-04-30 15:43:47 UTC
Description of problem:
There is no mirrormanager version here:

<metalink version="3.0" xmlns="http://www.metalinker.org/" type="dynamic"
pubdate="Sat, 30 Apr 2011 11:54:56 GMT" generator="mirrormanager"
xmlns:mm0="http://fedorahosted.org/mirrormanager">

Comment 1 Matt Domsch 2011-05-01 14:18:19 UTC
There are two reasons for this:
1) the metalink XML DTD does not suggest including a version, nor have a field for including a version, of the generator, therefore I didn't include it.  I suppose it could go into the generator string somehow.

2) The running instance of MM's mirrorlist_server that generates this does not know what its own version is.  Nothing has needed it thus far.

I presume the purpose of including the version is to make it easier to file a bug against the proper version when found?  MM receives very few bug reports (there aren't that many users of it besides yum).

I'm inclined to mark this WONTFIX unless a compelling reason to include it is provided.

Thanks,
Matt

Comment 2 Steve Tyler 2011-05-01 15:13:05 UTC
(In reply to comment #1)
> There are two reasons for this:
> 1) the metalink XML DTD does not suggest including a version, nor have a field
> for including a version, of the generator, therefore I didn't include it.  I
> suppose it could go into the generator string somehow.

There isn't a DTD for metalink documents:
Mirror manager files are not valid XML
https://fedorahosted.org/mirrormanager/ticket/32

Are you still referring to this?
The Metalink Download Description Format
http://tools.ietf.org/html/rfc5854

> 2) The running instance of MM's mirrorlist_server that generates this does not
> know what its own version is.  Nothing has needed it thus far.

Wouldn't that be easy to fix?
Many programs know their own version:
$ python --version
$ gdb --version

> I presume the purpose of including the version is to make it easier to file a
> bug against the proper version when found?  MM receives very few bug reports
> (there aren't that many users of it besides yum).

Having a version for filing bug reports was my first rationale.

A better rationale is that a client can test the version and possibly adapt to the corresponding protocol. I suppose a separate query could retrieve the version of the generator, but this is getting way beyond my expertise ...

> I'm inclined to mark this WONTFIX unless a compelling reason to include it is
> provided.

OK. If the client doesn't need to check the version, then there is no reason to provide it.

> Thanks,
> Matt

Comment 3 Steve Tyler 2011-05-02 06:47:03 UTC
(In reply to comment #1)
...
> I suppose it could go into the generator string somehow.

That's a good idea.

The metalink 3.0 specification does not appear to preclude doing that.

Indeed, "mirrormanager-1.3.7" better describes the generator than simply "mirrormanager". Further, the former would be easy to parse if a client wanted to check the version.

4.2.1.5 generator Attribute
generator = The application used to generate the .metalink file.
http://www.metalinker.org/Metalink_3.0_Spec.pdf

Comment 4 Steve Tyler 2011-05-02 06:56:36 UTC
The Metalink 3.0 Specification has this in an example:
generator="Metalink Gen - http://metalink.packages.ro"

If a URL can go into the generator attribute, surely the version can.

Comment 5 Steve Tyler 2011-05-02 07:27:09 UTC
(In reply to comment #2)
...
> Many programs know their own version:
> $ python --version
> $ gdb --version
...

$ yum --version

$ grep '__version__' /usr/lib/python2.7/site-packages/yum/__init__.py
__version__ = '3.2.28'
__version_info__ = tuple([ int(num) for num in __version__.split('.')])
default_grabber.opts.user_agent += " yum/" + __version__

Comment 6 Steve Tyler 2011-05-02 07:35:13 UTC
Standard way to embed version into python package?
http://stackoverflow.com/questions/458550/standard-way-to-embed-version-into-python-package

Comment 7 Steve Tyler 2011-05-02 09:32:07 UTC
The yum version is maintained in two places: yum.spec and __init__.py:
http://yum.baseurl.org/gitweb?p=yum.git;a=commitdiff;h=fdbc727fc7ce91126fab994262e5ef6283f3217c

That could be error prone.

MM appears to maintain the version in the Makefile and insert it into other files with sed.
http://git.fedorahosted.org/git/?p=mirrormanager;a=commitdiff;h=31d8c6fbdbd3db6f2f8f4be3ecaa3a0735748557

That seems more robust, but what file would be edited?

Comment 8 Steve Tyler 2011-05-07 13:03:05 UTC
(In reply to comment #3)
> ... "mirrormanager-1.3.7" ...

RFC 5854 uses a "/" as a separator:
agent         = token ["/" agent-version]
http://tools.ietf.org/html/rfc5854

Comment 9 Matt Domsch 2011-11-18 01:52:58 UTC
long overdue - the file to get edited is mirrormanager/mirrorlist-server/mirrorlist_server.py, and/or have something create version.py in that directory and have that imported by mirrorlist_server.py and used.

Comment 10 Matt Domsch 2011-11-18 04:29:05 UTC
commit a283e0a4471d3e9f197abd8e838cdd5c5c8675e2
Author: Matt Domsch <Matt_Domsch>
Date:   Thu Nov 17 22:25:57 2011 -0600

    report version in metalink
    
    fixes https://bugzilla.redhat.com/show_bug.cgi?id=701047

diff --git a/Makefile b/Makefile
index 694d519..3f65f9f 100644
--- a/Makefile
+++ b/Makefile
@@ -8,6 +8,7 @@ RELEASE_STRING := $(RELEASE_NAME)-$(RELEASE_VERSION)
 
 SPEC=mirrormanager.spec
 RELEASE_PY=server/mirrormanager/release.py
+VERSION_PY=mirrorlist-server/mirrormanager_version.py
 TARBALL=dist/$(RELEASE_STRING).tar.bz2
 STARTSCRIPT=server/start-mirrormanager
 PROGRAMDIR=/usr/share/mirrormanager/server
@@ -27,7 +28,10 @@ $(SPEC):
 $(RELEASE_PY):
        sed -e 's/##VERSION##/$(RELEASE_VERSION)/' $(RELEASE_PY).in > $(RELEASE_PY)
 
-prep: $(SPEC) $(RELEASE_PY)
+$(VERSION_PY):
+       echo "mirrormanager_version = u'$(RELEASE_VERSION)'" > $(VERSION_PY)
+
+prep: $(SPEC) $(RELEASE_PY) $(VERSION_PY)
        pushd server ;\
        python setup.py egg_info
        echo 'db_module=mirrormanager.model' > server/mirrormanager.egg-info/sqlobject.txt
diff --git a/mirrorlist-server/mirrorlist_server.py b/mirrorlist-server/mirrorlist_server.py
index fd9f8c6..151d258 100755
--- a/mirrorlist-server/mirrorlist_server.py
+++ b/mirrorlist-server/mirrorlist_server.py
@@ -16,6 +16,11 @@ from IPy import IP
 import GeoIP
 from weighted_shuffle import weighted_shuffle
 
+try:
+    from mirrormanager_version import mirrormanager_version
+except:
+    mirrormanager_version=u'dev'
+
 # can be overridden on the command line
 socketfile = '/var/run/mirrormanager/mirrorlist_server.sock'
 cachefile = '/var/lib/mirrormanager/mirrorlist_cache.pkl'
@@ -101,7 +106,7 @@ def metalink_header():
     doc += '<metalink version="3.0" xmlns="http://www.metalinker.org/"'
     doc += ' type="dynamic"'
     doc += ' pubdate="%s"' % pubdate
-    doc += ' generator="mirrormanager"'
+    doc += ' generator="mirrormanager/%s"' % mirrormanager_version
     doc += ' xmlns:mm0="http://fedorahosted.org/mirrormanager"'
     doc += '>\n'
     return doc


Note You need to log in before you can comment on or make changes to this bug.