Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#156 closed defect (fixed)

Inconsistency in using cjson vs json

Reported by: nirbheek Owned by: marcopg
Priority: trivial Milestone:
Component: Sugar Version: Unspecified
Severity: Unspecified Keywords:
Cc: garycmartin, tomeu Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

Some packages import json whereas others import cjson. Examples:

sugar-jhbuild uses json:
./scripts/report.py: import json

browse uses json:
./model.py:import json

etc..

sugar, sugar-toolkit, sugar-datastore use cjson in various places.

The moduleset system deps mention python-cjson, but do NOT mention json-py as a dependency (which is implicit on Ubuntu and Fedora but not on other distros)

Change History (16)

comment:1 follow-up: Changed 12 years ago by nirbheek

Changes made to "browse" activity, and merge request filed

comment:2 in reply to: ↑ 1 ; follow-up: Changed 12 years ago by nirbheek

Changes made to "sugar-jhbuild", and merge request filed

More will come as I spot them.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 12 years ago by nirbheek

The "chat" activity was using simple-json instead of cjson. Changes made and merge request filed

comment:4 follow-up: Changed 12 years ago by marcopg

Posted a comment on your jhbuild commit. Saying it here just to make sure the gitorious workflow works fine, we never tested comments and merge requests so far.

comment:5 in reply to: ↑ 4 Changed 12 years ago by nirbheek

Replying to marcopg:

Posted a comment on your jhbuild commit. Saying it here just to make sure the gitorious workflow works fine, we never tested comments and merge requests so far.

I didn't get an email notification for the comment, and I don't see any place where I can edit the settings for that. I've replied to your comment; do you see an email notification?

comment:6 follow-up: Changed 12 years ago by garycmartin

This is news to me, could someone actually explicitly state the requirement here, is it json, or cjson? Or even better post something to the sugar dev list for activity authors. I'm using json in Moon at the recommendation of Tomeu (OLPC XO dist has both json and cjson). Thanks. --G

comment:7 Changed 12 years ago by garycmartin

  • Cc garycmartin added

comment:8 in reply to: ↑ 6 ; follow-ups: Changed 12 years ago by nirbheek

Replying to garycmartin:

This is news to me, could someone actually explicitly state the requirement here, is it json, or cjson? Or even better post something to the sugar dev list for activity authors. I'm using json in Moon at the recommendation of Tomeu (OLPC XO dist has both json and cjson). Thanks. --G

Here's a roundup of the json-mess in Python:

There's json-py which is installed by default in Ubuntu and Fedora (and hence by extension in the XO dist). This is a pure-python implementation of a module for json processing.

There's python-cjson which is a (very fast) C module for json processing, which is used in sugar, sugar-toolkit, sugar-datastore etc.

There's simple-json which is packaged in Ubuntu and Fedora, but not installed by default. It is worth mentioning that simple-json was merged into Python 3k (similar to how part of pyxml and sqlite3 were added to python 2.6).

The interface for all three are extremely similar:
cjson.decode() == json.read() == simplejson.loads()
cjson.encode() == json.write() == simplejson.dumps()

Now, there are packages which are using these three interchangeably.

I've been writing ebuilds for packaging the XO stack on Gentoo, and json-py is *not* packaged on Gentoo. Simply because there are no packages that use it (most packages use simple-json).

It would be excellent (from Gentoo's POV, and otherwise) if everything used *one* of these (especially since the interfaces of all three are very similar). And since cjson is the fastest, most widely used, and is already a system dep, IMO it should be used.

comment:9 in reply to: ↑ 8 Changed 12 years ago by alsroot

Replying to nirbheek:

I've been writing ebuilds for packaging the XO stack on Gentoo, and json-py is *not* packaged on Gentoo. Simply because there are no packages that use it (most packages use simple-json).

btw there is GentooOnSugar. We could merge our projects, thats a nice idea to have only one sugar overlay for Gentoo :)

comment:10 Changed 12 years ago by erikos

nirbheek: I did merge your browse changes - thanks for that!

comment:11 in reply to: ↑ 8 Changed 12 years ago by garycmartin

Replying to nirbheek:

Replying to garycmartin:

This is news to me, could someone actually explicitly state the requirement here, is it json, or cjson? Or even better post something to the sugar dev list for activity authors. I'm using json in Moon at the recommendation of Tomeu (OLPC XO dist has both json and cjson). Thanks. --G

Here's a roundup of the json-mess in Python:

Thanks for all the back story, I'll look to move over to cjson (I just have one trivial Activity use to change so far).

comment:12 in reply to: ↑ 3 Changed 12 years ago by nirbheek

Replying to nirbheek:

The "chat" activity was using simple-json instead of cjson. Changes made and merge request filed

Oops, that's the browse merge request again :)

The chat merge request is at http://git.sugarlabs.org/projects/chat/repos/mainline/merge_requests/5

Replying to erikos:

nirbheek: I did merge your browse changes - thanks for that!

Thanks erikos :)

comment:13 Changed 12 years ago by marcopg

I suggest to open one ticket for each module, otherwise it's really difficult to track progress.

For sugar-jhbuild, I'd prefer to just change the method invocations instead of doing tricks. Can you update your patch?

comment:14 Changed 12 years ago by marcopg

Ignore my last comment. I read the rationale for supporting json and it make sense. So I merged it, thanks!

comment:15 follow-up: Changed 12 years ago by tomeu

  • Cc tomeu added

Can we close this now?

comment:16 in reply to: ↑ 15 Changed 12 years ago by nirbheek

  • Resolution set to fixed
  • Status changed from new to closed

Replying to tomeu:

Can we close this now?

Hmm, I didn't find any other inconsistencies, so closing for now. Will reopen (or file separate bug{s}) if I find anything else. Thanks everyone!

Note: See TracTickets for help on using tickets.