improved changes to allow retrieval of alternate links in sitemaps, see #360

docs/topics/spiders.rst

@@ -560,6 +560,25 @@ SitemapSpider
 
         By default, all sitemaps are followed.
 
+    .. attribute:: use_alternate_links
+
+        Specifies if alternate links for one ``url`` should be followed. These
+        are links for the same website in another language passed within
+        the same ``url`` block.
+        
+        For example::
+       
+            <url>
+                <loc>http://example.com/</loc>
+                <xhtml:link rel="alternate" hreflang="de" href="http://example.com/de"/>
+            </url>
+
+        With ``use_alternate_links`` set, this would retrieve both URLs. With
+        ``use_alternate_links`` disabled, only ``http://example.com/`` would be
+        retrieved.
+
+        Default is ``use_alternate_links`` disabled.
+
 
 SitemapSpider examples
 ~~~~~~~~~~~~~~~~~~~~~~

scrapy/contrib/spiders/sitemap.py

@@ -13,10 +13,6 @@ class SitemapSpider(BaseSpider):
     sitemap_follow = ['']
 
     def __init__(self, *a, **kw):
-        self._alternate = False
-        if 'alternate' in kw and kw.pop('alternate') == True:
-            self._alternate = True
-
         super(SitemapSpider, self).__init__(*a, **kw)
         self._cbs = []
         for r, c in self.sitemap_rules:
@@ -41,7 +37,7 @@ class SitemapSpider(BaseSpider):
 
             s = Sitemap(body)
             if s.type == 'sitemapindex':
-                for loc in iterloc(s, self._alternate):
+                for loc in iterloc(s, self.use_alternate_links):
                     if any(x.search(loc) for x in self._follow):
                         yield Request(loc, callback=self._parse_sitemap)
             elif s.type == 'urlset':
@@ -74,6 +70,6 @@ def iterloc(it, alt=False):
         yield d['loc']
 
         # Also consider alternate URLs (xhtml:link rel="alternate")
-        if alt == True and 'alternate' in d:
+        if alt and 'alternate' in d:
             for l in d['alternate']:
                 yield l

scrapy/tests/test_utils_sitemap.py

@@ -171,6 +171,7 @@ Disallow: /forum/active/
                 href="http://www.example.com/schweiz-deutsch/"/>
             <xhtml:link rel="alternate" hreflang="en"
                 href="http://www.example.com/english/"/>
+            <xhtml:link rel="alternate" hreflang="en"/><!-- wrong tag without href -->
         </url>
     </urlset>""")
         

scrapy/utils/sitemap.py

@@ -12,7 +12,7 @@ class Sitemap(object):
     (type=sitemapindex) files"""
 
     def __init__(self, xmltext):
-        xmlp = lxml.etree.XMLParser(recover=True)
+        xmlp = lxml.etree.XMLParser(recover=True, remove_comments=True)
         self._root = lxml.etree.fromstring(xmltext, parser=xmlp)
         rt = self._root.tag
         self.type = self._root.tag.split('}', 1)[1] if '}' in rt else rt
@@ -25,10 +25,11 @@ class Sitemap(object):
                 name = tag.split('}', 1)[1] if '}' in tag else tag
 
                 if name == 'link':
-                    d.setdefault('alternate', []).append(el.get('href'))
+                    if 'href' in el.attrib:
+                        d.setdefault('alternate', []).append(el.get('href'))
                 else:
                     d[name] = el.text.strip() if el.text else ''
-            
+
             if 'loc' in d:
                 yield d