Use pickle protocol format version 2.

[nuclearsandwich]

Proposed as a fix for #379.

So far this only #worksonmymachine. Following the queues from @NikolausDemmel in this comment I read through the docs and tried to use the cache created in python3 from python2 without the fix_imports flag and was successful so I did not apply it.

I have not added any tests of the unpickling. I'm not sure how effective such a test would be with a static blob and I don't know if we want to install python3 separately in all of the python2 environments.

[codecov-io]

Codecov Report

Merging #633 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #633   +/-   ##
=======================================
  Coverage   75.29%   75.29%           
=======================================
  Files          30       30           
  Lines        2882     2882           
=======================================
  Hits         2170     2170           
  Misses        712      712

Impacted Files	Coverage Δ
src/rosdep2/sources_list.py	87.17% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc3c601...4cc9b12. Read the comment docs.

[NikolausDemmel]

Cool, thanks for looking into this.

The patch looks reasonable. Also, fix_imports seems to be True by default, so we are using it.

Have you also tried reading a dump created with python 2 in python 3? One thing I could see going wrong is that after loading in python 3 strings are now bytes instead, but maybe not.

Some sort of test would be great for future regressions. A test that could run in just the current version that is being tested (be it pyhton 2 or 3):

• write small cache and compare blob to checked in reference blob (I'm assuming the binary representation is not changing, which would have to be verified).
• load from the checked in blob and do simple query

Except for testing aspect, +1 on this PR

[cottsay]

Relevant documentation for observers: https://docs.python.org/3/library/pickle.html#pickle.dump

Looks like the correct and least disruptive fix to me. It's nice that the docs specifically call out Python2 compatibility.

[NikolausDemmel]

I tested this branch in both directions (updating with py2, reading with py3 and vice versa), and it seems to work fine, where master fails if py2 tries to read a py3 created cache.

+1 for merging as is.

[nuclearsandwich]

@NikolausDemmel I have a branch that tests unpickling a file created with this branch in python2 and python3 produces equivalent data structures I just have to find time to get it cleaned up and pushed.

[wjwwood]

lgtm

[wjwwood]

Looks like the only failure is related to new warnings from flake8, so this should be ok to merge. It would be nice to fix those in a different pr.

[NikolausDemmel]

Looks like the only failure is related to new warnings from flake8, so this should be ok to merge. It would be nice to fix those in a different pr.

There is already #636 ;-)

[wjwwood]

There is already #636 ;-)

Fair enough :)

Sorry I just haven't had the bandwidth to do anything here in a while.

[NikolausDemmel]

No worries, I can relate. Thanks for merging. :-)