id,summary,reporter,owner,description,type,status,priority,milestone,component,resolution,keywords,cc,merged,author,reviewer,upstream,work_issues,branch,commit,dependencies,stopgaps
12342,things I don't like about the json <--> hg conversion code,William Stein,tbd,"1. It doesn't print a usage message when the script is just run:
{{{
wstein@sage:/scratch/wstein/sage-4.8-sage.math.washington.edu-x86_64-Linux$ ./sage -sh
Starting subshell with Sage environment variables set.
Be sure to exit when you are done and do not do anything
with other copies of Sage!
Bypassing shell configuration files ...
SAGE_ROOT=/mnt/usb1/scratch/wstein/sage-4.8-sage.math.washington.edu-x86_64-Linux
(sage subshell) sage:sage-4.8-sage.math.washington.edu-x86_64-Linux wstein$ cd spkg/base
SAGE_ROOT=/mnt/usb1/scratch/wstein/sage-4.8-sage.math.washington.edu-x86_64-Linux
(sage subshell) sage:base wstein$ ./json_bundle.py
SAGE_ROOT=/mnt/usb1/scratch/wstein/sage-4.8-sage.math.washington.edu-x86_64-Linux
}}}
There should have been a usage message detailing the options.
2. There really are no tests at all of this. The ticket #3052 where it was introduced talked about the challenge of writing tests, but then seemed to conclude with totally giving up. This is not acceptable. I'm totally opposed to ever introducing code into Sage that does anything useful if it doesn't have tests. If it isn't, tested then it will definitely be broken soon enough. This is exactly the sort of code that is likely to break too, due to Mercurial making some format change, us switching away from Mercurial etc. With no tests, the brokeness of this code could go unnoticed even if we switched all of sage to git.
3. The name bundle.json for the JSON hg repo could clash with an existing file, and is confusing, since it doesn't mention hg at all. It would be much better to use .hg_bundle.json, since by starting with "".hg"" it's clear it is something that is reserved for Mercurial usage.
4. This code needs to be refactored. A bare minimum functionality for this should have been something like:
{{{
$ sage -hg_text_expand .hg
# produce .hg_bundle.json
$ sage -hg_text_collapse .hg_bundle.json
# create .hg or give an error if .hg is there.
}}}
Then the other functionality could be built on that. As it is, just doing the conversion either way runs a bunch of code in the middle of a for loop (over spkg's). Currently the most basic functionality (which could be easily tested) is obfuscated; if it were available other people could easily build tools on top of it to do what they need (e.g., with their own hg repos).
5. A serious issue with this patch is that it seems to already be totally broken. Just take the sage-4.8 tarball, extract it, and type ""make text-expand"". What happens is:
{{{
wstein@sage:/scratch/wstein/2012-01-22/sage-4.8$ time make text-expand
./spkg/base/text-expand
Determined SAGE_ROOT=/scratch/wstein/2012-01-22/sage-4.8
Extracting SPKG: ./spkg/standard/boost-cropped-1.34.1.spkg ...
Extracting SPKG: ./spkg/standard/jinja2-2.5.5.spkg ...
Extracting SPKG: ./spkg/standard/atlas-3.8.4.p1.spkg ...
Extracting SPKG: ./spkg/standard/fortran-20100629.spkg ...
Deconstructing repo at ./spkg/standard/cddlib-094f.p10 ...
************************************************************************
You must compile Sage first using 'make' in the Sage root directory.
(If you have already compiled Sage, you must set the SAGE_ROOT variable
in the file './sage').
************************************************************************
./spkg/base/text-expand: line 23: ./json_bundle.py: No such file or directory
rm: cannot remove `./spkg/standard/cddlib-094f.p10/bundle.hg': No such file or directory
Deconstructing repo at ./spkg/standard/genus2reduction-0.3.p8 ...
...
** and hundreds more similar error messages **
}}}
Maybe I'm doing something wrong, but there are no directions, and if one can't just do ""make text-expand"" from a bare tarball, then there should be an immediate error message to that effect.
6. Looking at the source code, I'm concerned about whether or not *every* .hg repo (e.g., even the base repo) is actually changed to JSON when ""make text-expand"" is run. This is just a concern. I do note that after doing ""make text-expand"" the hg repo in $SAGE_ROOT is gone, with nothing in its place. Not happy about that (maybe this is caused by everything going to hell above):
{{{
wstein@sage:/scratch/wstein/2012-01-22/sage-4.8$ ls -al
total 128
drwxr-xr-x 4 wstein wstein 4096 2012-01-22 08:28 .
drwxr-xr-x 4 wstein wstein 4096 2012-01-22 08:38 ..
-rw-r--r-- 1 wstein wstein 71842 2012-01-19 21:38 COPYING.txt
-rw-r--r-- 1 wstein wstein 366 2012-01-19 21:38 .hgignore
-rw-r--r-- 1 wstein wstein 1949 2012-01-19 21:38 .hgtags
drwxr-xr-x 2 wstein wstein 4096 2012-01-19 21:38 ipython
-rw-r--r-- 1 wstein wstein 4399 2012-01-19 21:38 Makefile
-rw-r--r-- 1 wstein wstein 11673 2012-01-19 21:38 README.txt
-rwxr-xr-x 1 wstein wstein 5046 2012-01-19 21:38 sage
drwxr-xr-x 4 wstein wstein 4096 2012-01-19 21:38 spkg
}}}
Anyway, needs work. ",defect,closed,critical,sage-duplicate/invalid/wontfix,distribution,wontfix,,,,,Marc Mezzarobba,N/A,,,,,