494509 - [silme] silme.io.jar broken in 0.5 and trunk

Status: RESOLVED FIXED

(Mozilla Localizations :: Infrastructure, defect)

Description

[Adrian Kalla [:adriank]]

Attachment: patch that solves all three described problems

Unfortunatelly, there are a few problems with silme.io.jar in the Silme 0.5 release, which are making that module unusuable.

1.) we have a missing 's' here: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/jar.py#l66 (should be "elems=" instead of "elem-")

2.) because we open the default FileFormat methods in that way: 
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/jar.py#l32
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/jar.py#l37
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/jar.py#l42
, we force the use of this methods:
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/clients.py#l295
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/2af263121b1d/lib/silme/io/clients.py#l300
which are of course rising errors.

3.) Is not a critical problem like the previous two, but is slowing down the read process of a jar-file a few hunderd times (at least on my PC): 
we try to get blobs, entitylists and l10nobjects, even if we have an emtpy filename.


Attached: a simple patch that solves all three problems.

Comment 1

[Seth Bindernagel]

I believe that this patch is blocking the release of adriank's "compare-locales2" version 0.1.  

Can we get a review and determine if we are going to release Silme 0.5.1 or if this will be an enhancement for Silme 0.7. 

If it is blocking compare-locales2, then I believe we should release Silme 0.5.1 to clear the way for adriank.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yes, it'll get 0.5.1 release, and it has to happen ASAP

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 379275 [details] [diff] [review]
patch that solves all three described problems

The elem->elems and skipping empty filenames are good and r+, but the FileFormat switch requires more work.

Currently you're regressing behavior by removing arguments comparing to original code.

I'm going to strip your patch of that part and commit what works and we'll go from there.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch for 2 out of the three problems

commited patch

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/9e8786a8dc46
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/cc5b9a98f58f

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Patch for the third issue

This patch fixes the third issue reported here. It actually uses super() function and keeps all arguments intact.

Adrian, will you do the review, pls? :)

Comment 7

[Adrian Kalla [:adriank]]

Comment on attachment 380660 [details] [diff] [review]
Patch for the third issue

Hmmm, I didn't know about 'super' before, so something new learned again :)

review+, of course

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/82d39604702e
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/9c47ad0d420b

fixed! :)

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Just a second thought. Adrian, doesn't it mean that we will now fail to create a subpackage for an empty directory in zip file?

Comment 10

[Adrian Kalla [:adriank]]

That could be correct. But only if the directory was completely empty.

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

wondering if we should care about such case. worth filing bug?