626132 - "Could not parse install.rdf" after validation passed and chose "Add Version"

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect, P1)

Description

[Jorge Villalobos [:jorgev] (he/him)]

I tried to upload a new version of Fire.fm (see attached). The validation passed with 10 warnings and no errors. Then I clicked on "Add Version" and got a message saying "Could not parse install.rdf" in red letters.

I'm pretty sure the install.rdf file in this XPI is OK.

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: Fire.fm 1.4.11

Comment 2

[Wil Clouser [:clouserw]]

I actually just saw this scroll by in the logs and filed bug 626133.  If my hunch is right, we're poorly choosing temporary names and if you try it again it'll work.  Otherwise let me know.

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

Just tried 3 times in a row with no success.

Comment 4

[Jeff Balogh (:jbalogh)]

My zipfile.py (Python 2.6.4):

        if member.filename[-1] == '/':
            if not os.path.isdir(targetpath):
                os.mkdir(targetpath)
            return targetpath

The servers' zipfile.py (Python 2.6.2):


        if member.filename[-1] == '/':
            os.mkdir(targetpath)
            return targetpath

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

Out of curiosity: what's special about Fire.fm that is triggering this bug?

Comment 6

[zigboom]

I'm having the same problem with a Theme that I try to update. 
Nothing changed in the theme since the last update that may be significant, surely nothing in install.rdf. 
It looks like the new review process has a bug.

Comment 7

[zigboom]

I'm having the same problem with a Theme that I try to update. 
Nothing changed in the theme since the last update that may be significant, surely nothing in install.rdf. 
It looks like the new review process has a bug.

Comment 8

[zigboom]

I'm having the same problem with a Theme that I try to update. 
Nothing changed in the theme since the last update that may be significant, surely nothing in install.rdf. 
It looks like the new review process has a bug.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

We got it the first 2 times, zig :P

Comment 10

[zigboom]

hahaha :D 

Repetition is a good way to make sure that the message gets through :)

Comment 11

[Nils Maier [:nmaier]]

(In reply to comment #5)
> Out of curiosity: what's special about Fire.fm that is triggering this bug?

Your XPI/Zip lists contained directories as own entries, e.g. defaults/preferences/. The python stdlib zipfile module that is shipped with the server python installation is horribly broken and will throw an exception, because first it creates the directory and all intermediate directories in |os.makedirs(upperdirs)| and then tries to create the directory again in the snippet Jeff gave |os.mkdir(targetpath)| which then will *always* throw an exception as the directory *always* exists already at this point due to the prior |os.makedirs| call.
So: Include directory entries in your zip = have AMO choke on it.

Correctly, the code from the snippet should read:
    if member.filename[-1] == '/':
        return targetpath
omitting the |os.mkdir| altogether.

You can remedy this by using another way to create the zip (another zip tool), one which omits adding the directory entries.
An XPI just listing "defaults/preferences/firefm.js" instead of also listing "defaults/preferences/" will work just fine.
I created a python script to strip directory entries from an existing zip:
https://gist.github.com/787095

AMO could remedy this by:
* Updating python, as requested in bug 626133
* Removing the offending os.mkdir from zipfile.py (i.e. patching the python installation)
* Checking for directory entries in zips and omitting them (similar to my gist)

I'd guess this warrants a hotfix release to address this particular issue.

Comment 12

[zigboom]

OK, that's interesting, but how to explain a simple theme as mine that got updated numerous times, and now suddenly as soon as the review process changed, got rejected. 
Nothing changed in the directory structure that I'm aware of & there are other reports of the same bug floating in the theme development forums. 

My theme of course has nothing to do with Python.

Here's all the info: 
-------------------------------------------------------------------------------
Add-on contains JAR files with <em:unpack>

Warning: The add-on sets <em:unpack> to true in install.rdf, but contains JAR files. This can result in performance issues. It is recommended that you no longer use JAR files to package your chrome files.
--------------------------------------------------------------------------------
Modification to identity box.

Warning: The identity box (#identity-box) is a sensitive piece of the interface and should not be modified.

Warning: Lines: 968, 972, 977, 978, 982, 995, 999, 1005, 1011, 1017, 1023, 1028, 1029, 1444
chrome/lfox.jar/browser/browser.css
--------------------------------------------------------------------------------
Your add-on passed validation with 0 errors and 2 warnings.
--------------------------------------------------------------------------------
Could not parse install.rdf
--------------------------------------------------------------------------------

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

The upload and file processing code is substantially different. That's the reason uploads that previously worked are failing now.

I'm trying to upload Fire.fm after packaging it with zip -D, which appears to do the trick.

Comment 14

[Nils Maier [:nmaier]]

(In reply to comment #13)
> The upload and file processing code is substantially different. That's the
> reason uploads that previously worked are failing now.
> 
> I'm trying to upload Fire.fm after packaging it with zip -D, which appears to
> do the trick.

Yeah, using InfoZip zip with the -D flag should work, I suppose.

Comment 15

[Jorge Villalobos [:jorgev] (he/him)]

I'm using the basic zip tool, and -D worked well. I just uploaded the new version successfully.

Thank you, Nils!

Comment 16

[zigboom]

Any tip for a theme packing (into a jar or xpi)?

I'm using the latest version of WinRar and have no idea what is the -D flag.

Anyway, I hope this bug will get fixed because my theme just works perfectly so I see no reason to go through all the trouble. 
I'm sure many other submissions will get this bug.

Comment 17

[Jorge Villalobos [:jorgev] (he/him)]

You'll have to check the documentation for an option that says "don't include directories" or something similar.

The bug will be fixed, and hopefully very soon. I'm sure there are many add-ons affected by this, given that directories are included by default by many zip tools.

Comment 18

[zigboom]

Thanks for the info Jorge.

Comment 19

[Jeff Balogh (:jbalogh)]

I added zipfile.py from my local Python 2.6.4 to vendor in https://github.com/jbalogh/zamboni-lib/commit/956f7a8. That should get picked up before the system library.

Comment 20

[krupa raj[:krupa]]

I was able to add a new version of Fire.fm without any errors @ https://addons.allizom.org/z/en-US/developers/addon/firefm/versions/126455

Jorgev, can you please confirm?

Comment 21

[zigboom]

Excuse me guys, I see that the bug is "Resolved Fixed" - Well, it isn't! 
I tried again today to upload my theme (that never failed in such way) & the same bug repeats time after time. 
Using WinRar with the same settings as always, it's the default setting that anyone will use normally and the theme won't upload. 
There's an identical report in the theme developers forum. It's not fixed even though there may be a workaround.

Comment 22

[Wil Clouser [:clouserw]]

It's fixed on staging and will be fixed on the live site tonight.

Comment 23

[zigboom]

Oh, great! Thank you very much for the info.

Comment 24

[Jorge Villalobos [:jorgev] (he/him)]

Yes, the attached file can be uploaded to allizom without the bug happening.

Comment 25

[krupa raj[:krupa]]

verified fixed based on comment 20 and comment 24

Comment 26

[zigboom]

Could someone please confirm when this is fixed on the website?

Comment 27

[Jorge Villalobos [:jorgev] (he/him)]

It should be fixed now, the push was yesterday.

Comment 28

[zigboom]

Attachment: LavaFox V1 - Firefox Theme

Comment 29

[zigboom]

Thanks Jorge for the info but I'm getting the same error. 
Using WinRar as always on default settings (I didn't figure out the -D flag workaround). 
It seems that the bug isn't fixed, just worked around using the -D method. 
I created an attachment of my theme, nothing changed in install.rdf & nothing significant changed in the theme itself regarding compression etc. 

There is one more report in the theme's forum, I'm sure there would be many if theme authors would try to update (most of theme are waiting for RC). 

Please guys, take a look again, the upload mechanism is still broken!

Comment 30

[Jorge Villalobos [:jorgev] (he/him)]

I'm reopening this bug, assuming the push was done yesterday.

Comment 31

[Jeff Balogh (:jbalogh)]

It's fixed on preview. Through a series of unfortunate events this did not make it to prod yesterday, but it really is fixed.

Comment 32

[zigboom]

Thanks for your reply, could someone please update here once it's updated on the website?

Comment 33

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #32)
> Thanks for your reply, could someone please update here once it's updated on
> the website?

This should now be in production, and available for you to check out.

Comment 34

[zigboom]

I confirm that it's solved. Uploaded my themes without any problem. Thanks for all involved in fixing it!

Comment 35

[Jorge Villalobos [:jorgev] (he/him)]

Verified.

Comment 36

[ElbertF]

I'm experiencing the same issue, even when using "zip -D".

    Cannot Parse install.rdf
    Addon missing install.rdf

I only changed the version number in the RDF so I'm pretty sure it's valid, and it's definitely present.

Comment 37

[ElbertF]

Attachment: Wappalyzer add-on

Comment 38

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #36)
> I'm experiencing the same issue, even when using "zip -D".
> 
>     Cannot Parse install.rdf
>     Addon missing install.rdf
> 
> I only changed the version number in the RDF so I'm pretty sure it's valid, and
> it's definitely present.

You're commenting in a verified fixed bug; please file a new one.  Thanks.