#12342: things I don't like about the json <--> hg conversion code
----------------------------+-----------------------------------------------
Reporter: was | Owner: tbd
Type: defect | Status: new
Priority: critical | Milestone: sage-5.0
Component: distribution | Keywords:
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
----------------------------+-----------------------------------------------
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.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12342>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
