Alternative patch to support REP-143 #365

wjwwood · 2014-12-04T19:44:26Z

This patch just changes the hashing function to support lists of strings rather than just a single string. This allows the cache to be hashed on a list of urls rather than a single one.

The list of urls is also passed to the internal self.url in the DataSource object, but this isn't a problem in any code paths which use the rosdistro files as a source from what I can tell. The self.url seems to only be used for debug output, including the index file at the root of the cache which lists the files that make up the cache. In this case you get something like this:

# First with regular rosdep
% rosdep update
reading in sources list data from /etc/ros/rosdep/sources.list.d
Hit https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/gentoo.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
Hit https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml
Ignore legacy gbpdistro "groovy"
Ignore legacy gbpdistro "hydro"
Query rosdistro index https://raw.github.com/ros/rosdistro/master/index.yaml
Add distro "groovy"
Add distro "hydro"
Add distro "indigo"
updated cache in /Users/william/.ros/rosdep/sources.cache

% cat /Users/william/.ros/rosdep/sources.cache/index
#autogenerated by rosdep, do not edit. use 'rosdep update' instead
yaml https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml osx
yaml https://github.com/ros/rosdistro/raw/master/rosdep/gentoo.yaml gentoo
yaml https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
yaml https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml fuerte
yaml https://raw.github.com/ros/rosdistro/master/groovy/distribution.yaml groovy
yaml https://raw.github.com/ros/rosdistro/master/hydro/distribution.yaml hydro
yaml https://raw.github.com/ros/rosdistro/master/indigo/distribution.yaml indigo

# Then with the modified rosdep and rosdistro
% ROSDISTRO_INDEX_URL=https://raw.githubusercontent.com/ros/rosdistro/ec2test/index.yaml /tmp/rosdep_rep_143/bin/rosdep update
reading in sources list data from /etc/ros/rosdep/sources.list.d
Hit https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/gentoo.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
Hit https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
Hit https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml
Ignore legacy gbpdistro "groovy"
Ignore legacy gbpdistro "hydro"
Query rosdistro index https://raw.githubusercontent.com/ros/rosdistro/ec2test/index.yaml
Add distro "groovy"
Add distro "hydro"
Add distro "indigo"
updated cache in /Users/william/.ros/rosdep/sources.cache

% cat /Users/william/.ros/rosdep/sources.cache/index
#autogenerated by rosdep, do not edit. use 'rosdep update' instead
yaml https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml osx
yaml https://github.com/ros/rosdistro/raw/master/rosdep/gentoo.yaml gentoo
yaml https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
yaml https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml fuerte
yaml ['https://raw.githubusercontent.com/ros/rosdistro/ec2test/groovy/distribution.yaml'] groovy
yaml ['https://raw.githubusercontent.com/ros/rosdistro/ec2test/hydro/distribution.yaml'] hydro
yaml ['https://raw.githubusercontent.com/ros/rosdistro/ec2test/indigo/distribution.yaml'] indigo

Which I think is ok. I can optionally change it so that for the self.url it just takes the first url, but my opinion is that this patch is good as it stands.

There does not appear to be an InvalidSourceFile exception defined anywhere in rosdep...

dirk-thomas · 2014-12-04T20:18:25Z

LGTM
I was worried about the list in the index file when I looked at it before.

tfoote · 2014-12-04T21:42:27Z

src/rosdep2/sources_list.py

@@ -319,7 +320,7 @@ def download_default_sources_list(url=DEFAULT_SOURCES_LIST_URL):
    data = f.read().decode()
    f.close()
    if not data:
-        raise InvalidSourceFile("cannot download defaults file: empty contents")
+        raise RuntimeError("cannot download defaults file: empty contents")


Why change this exception?

Doesn't exist, it would be better maybe to create the exception, but this only slightly better than the current behavior which is NameError.

tfoote · 2014-12-04T21:44:59Z

+1

tfoote · 2014-12-06T01:20:03Z

I've run into an issue testing this branch inside catkin_support module.

I have a test rosdistro here with a dual entry: https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/index.yaml

It passes fine on the command line.

==> git-bloom-generate -y rosdebian --prefix release/indigo indigo -i 0
Generating source debs for the packages: ['test_deployment_one', 'test_deployment_two']
Debian Incremental Version: 0
Debian Distributions: ['saucy', 'trusty']
Releasing for rosdistro: indigo

Pre-verifying Debian dependency keys...
Running 'rosdep update'...
Traceback (most recent call last):
  File "/tmp/deploy_ros_buildfarm/venv/bin/git-bloom-generate", line 9, in <module>
    load_entry_point('bloom==0.5.14', 'console_scripts', 'git-bloom-generate')()
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/commands/git/generate.py", line 249, in main
    run_generator(generator, args)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/commands/git/generate.py", line 127, in run_generator
    gen.pre_modify)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/commands/git/generate.py", line 101, in try_execute
    retcode = func(*args, **kwargs)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/generators/debian/generator.py", line 565, in pre_modify
    fallback_resolver=missing_dep_resolver)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/generators/common.py", line 164, in resolve_dependencies
    peer_packages, retry=True)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/generators/common.py", line 117, in resolve_rosdep_key
    view = get_view(os_name, os_version, ros_distro)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/bloom/generators/common.py", line 80, in get_view
    value = get_catkin_view(ros_distro, os_name, os_version, False)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/rosdep2/catkin_support.py", line 108, in get_catkin_view
    sources_loader = SourcesListLoader.create_default(matcher=sources_matcher)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/rosdep2/sources_list.py", line 594, in create_default
    sources = load_cached_sources_list(sources_cache_dir=sources_cache_dir, verbose=verbose)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/rosdep2/sources_list.py", line 495, in load_cached_sources_list
    return parse_sources_data(cache_data, origin=cache_index, model=model)
  File "/tmp/deploy_ros_buildfarm/venv/local/lib/python2.7/site-packages/rosdep2/sources_list.py", line 366, in parse_sources_data
    raise InvalidData("line:\n\t%s\n%s"%(line, e), origin=origin)
InvalidData: line:
    yaml ['https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/indigo/distribution.yaml', 'https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/indigo/custom.yaml'] indigo
url must be a fully-specified URL with scheme, hostname, and path: ['https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/indigo/distribution.yaml',

<== Error running command '['/tmp/deploy_ros_buildfarm/venv/bin/git-bloom-generate', '-y', 'rosdebian', '--prefix', 'release/indigo', 'indigo', '-i', '0']'
Release failed, exiting.
root@9d81161d794f:/tmp/deploy_ros_buildfarm# cat /etc/ros/rosdep/sources.list.d/20-default.list      
# os-specific listings first
yaml https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml osx

# generic
yaml https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
gbpdistro https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml fuerte

# newer distributions (Groovy, Hydro, ...) must not be listed anymore, they are being fetched from the rosdistro index.yaml instead
root@9d81161d794f:/tmp/deploy_ros_buildfarm#

wjwwood · 2014-12-06T01:24:47Z

Yeah, so contrary to the comments in the rosdep source code that file is read again, but I'm not sure it actually being used. Either way I'll have to find a way to work around this real quick.

wjwwood · 2014-12-06T01:48:59Z

Ok, I pushed a new commit that joins the urls by ^, which is not allowed in urls, so that the key is a single item with no spaces in it, which should allow the cached source loaders stuff to work.

wjwwood · 2014-12-06T01:49:33Z

So, now the cache index looks like this:

#autogenerated by rosdep, do not edit. use 'rosdep update' instead
yaml https://github.com/ros/rosdistro/raw/master/rosdep/osx-homebrew.yaml osx
yaml https://github.com/ros/rosdistro/raw/master/rosdep/gentoo.yaml gentoo
yaml https://github.com/ros/rosdistro/raw/master/rosdep/base.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/python.yaml
yaml https://github.com/ros/rosdistro/raw/master/rosdep/ruby.yaml
yaml https://github.com/ros/rosdistro/raw/master/releases/fuerte.yaml fuerte
yaml https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/indigo/distribution.yaml^https://raw.githubusercontent.com/tfoote/rosdistro/test_deployment/indigo/custom.yaml indigo

dirk-thomas · 2014-12-07T20:59:59Z

+1 on merging this and releasing rosdep 0.11.0.

Alternative patch to support REP-143

wjwwood added 2 commits December 4, 2014 11:38

update to handle REP-143 multi-distribution files

2cee91f

exchange InvalidSourceFile with RuntimeError

fb90525

There does not appear to be an InvalidSourceFile exception defined anywhere in rosdep...

This was referenced Dec 4, 2014

support multiple distribution files #362

Closed

use rosdistro API instead of parsing yaml manually #364

Closed

tfoote reviewed Dec 4, 2014
View reviewed changes

change strategy for writing urls to cache index

9a9f7d7

wjwwood added a commit that referenced this pull request Dec 8, 2014

Merge pull request #365 from ros-infrastructure/wjw_rep143

600e905

Alternative patch to support REP-143

wjwwood merged commit 600e905 into master Dec 8, 2014

wjwwood deleted the wjw_rep143 branch December 8, 2014 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alternative patch to support REP-143 #365

Alternative patch to support REP-143 #365

wjwwood commented Dec 4, 2014

dirk-thomas commented Dec 4, 2014

tfoote Dec 4, 2014

wjwwood Dec 4, 2014

tfoote commented Dec 4, 2014

tfoote commented Dec 6, 2014

wjwwood commented Dec 6, 2014

wjwwood commented Dec 6, 2014

wjwwood commented Dec 6, 2014

dirk-thomas commented Dec 7, 2014

Alternative patch to support REP-143 #365

Alternative patch to support REP-143 #365

Conversation

wjwwood commented Dec 4, 2014

dirk-thomas commented Dec 4, 2014

tfoote Dec 4, 2014

Choose a reason for hiding this comment

wjwwood Dec 4, 2014

Choose a reason for hiding this comment

tfoote commented Dec 4, 2014

tfoote commented Dec 6, 2014

wjwwood commented Dec 6, 2014

wjwwood commented Dec 6, 2014

wjwwood commented Dec 6, 2014

dirk-thomas commented Dec 7, 2014