User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.9.3a5pre) Gecko/20100426 Minefield/3.7a5pre Build Identifier: The XPI file exported by 'cfx xpi' command is named after the value of the "name" property in package.json. If the value of the "name" property contains some invalid characters (\ / * : ? " < > |) which we cannot use as a part of file name, 'cfx xpi' fails to export XPI. Reproducible: Always Steps to Reproduce: 1. set the value of the "name" property in package.json as "Hello * World". 2. try to export XPI by 'cfx xpi' command. Actual Results: Fails to export XPI file with the following error message. IOError: [Errno 22] invalid mode ('wb') or filename: u'Hello * World.xpi' Expected Results: Succcessfully exports XPI file.
I think the XPI file name should be named after the package directory name like: 'hello-world.xpi' (assuming that the name of the package root directory is 'hello-world').
Oh, so theoretically long-ish names with special characters is what the 'fullName' property in the package.json is for: https://jetpack.mozillalabs.com/sdk/0.2/docs/#guide/package-spec 'name' is actually not meant to be something with those funky characters, and it actually defaults to the name of the package's directory if it's not in package.json--though this may conflict with the CommonJS package specification, I'm not sure. In any case, thanks for the report--we should definitely not raise a silly IOError in this instance, at the very least!
Sorry, I missed to read the spec of package.json. Now I understand that I should use "fullName" property instead of "name" to display a funcy name in the Extensions Manager. Should I change the status of the bug to "RESOLVED"? I found another related bug. If the value of "name" property in package.json and the root directory name both contain a white-space such like "hello world", 'cfx run' command fails with the following error message: (C:\jetpack-sdk-0.3) C:\jetpack-sdk-0.3\packages\hello world>cfx run -a firefox [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.import]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: file:///c: /email@example.com/components/harness.js :: buildLoader :: line 163" data: no] (undefined:163) FAIL Should I file this as a separated bug?
Sorry again, the package spec says that: "This name cannot contain spaces."
No, it's not your fault! We're providing terrible error feedback here--we should detect for these kinds of cases in cfx and give more helpful warnings, like: "Sorry, but thee 'name' property of packages/foo/package.json cannot be 'foo bar' because it contains illegal characters. You may want to use the 'fullName' property instead. For more information, see the package.json documentation at static-files/md/dev-guide/package-spec.md." How does something like that sound?
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product. To filter bugmail related to this change, filter on the word "looptid".
It sounds like we should: * validate that the name field only contains valid characters * strip out invalid characters when creating a XPI
Is this still happening?
(Pushing all open bugs to the --- milestone for the new triage system)
Myk: I never got confirmation that this was still happening after setting the triage flag a month ago. Do you know if we already do the things you listed in comment 7?
Yes, this is still happening. To test this, I created an addon using "cfx init", changed its name in package.json to "foo/bar", and called "cfx xpi" to try to package it, whereupon cfx printed the following error with traceback: Traceback (most recent call last): File "c:/Users/myk/Dropbox/addon-sdk/bin/cfx", line 29, in <module> cuddlefish.run() File "c:\Users\myk\Dropbox\addon-sdk\python-lib\cuddlefish\__init__.py", line 637, in run include_dep_tests=options.dep_tests File "c:\Users\myk\Dropbox\addon-sdk\python-lib\cuddlefish\packaging.py", line 322, in generate_build_for_target add_section_to_build(target_cfg, "tests", is_code=True) File "c:\Users\myk\Dropbox\addon-sdk\python-lib\cuddlefish\packaging.py", line 297, in add_section_to_build validate_resource_hostname(name) File "c:\Users\myk\Dropbox\addon-sdk\python-lib\cuddlefish\packaging.py", line 85, in validate_resource_hostname raise ValueError('invalid resource hostname: %s' % name) ValueError: invalid resource hostname: jid1-qwouchluzbmukq-at-jetpack-foo/bar-tests cfx should instead tell me without traceback that my addon's name contains invalid characters, if indeed "/" is an invalid character in addon names. Otherwise, it should strip out that character when using the addon name as a resource hostname, XPI filename, etc. (And of course it should do the same for other such characters.)
(In reply to Myk Melez [:myk] from comment #11) So basically, in https://github.com/mozilla/addon-sdk/blob/3eb964d2c25963a0516c59cd072a08b33ab29013/python-lib/cuddlefish/packaging.py#L76 it should catch the errors raised in those three ifs? (Or just not raise any errors but just modify 'name'?)
(In reply to Wes Kocher (:KWierso) (Jetpack Bugmaster) from comment #12) > (In reply to Myk Melez [:myk] from comment #11) > So basically, in > https://github.com/mozilla/addon-sdk/blob/ > 3eb964d2c25963a0516c59cd072a08b33ab29013/python-lib/cuddlefish/packaging. > py#L76 it should catch the errors raised in those three ifs? (Or just not > raise any errors but just modify 'name'?) Hmm, yes, I think that's right.
Created attachment 577802 [details] Pointer to pull request 278 So this pull request just changes the wording of the error being raised when invalid stuff is detected. It now mentions that the name must be only letters, numbers, dashes and underscores, instead of just "invalid resource hostname". Also, as of some recent commit, it no longer shows the jid-style URI in the error, it just shows the package name's value. The pull request doesn't strip out invalid characters or hide the traceback, but it's at least an improvement over what was there.
Comment on attachment 577802 [details] Pointer to pull request 278 Comments in pull request.
Comment on attachment 577802 [details] Pointer to pull request 278 Ready for another review, Myk.
Comment on attachment 577802 [details] Pointer to pull request 278 Passing review request to markh.
Comment on attachment 577802 [details] Pointer to pull request 278 looks good!
Renaming bug to what the patch actually does.
Commit pushed to https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/95f03637db95a53c1169804e19e54e9cdb43f7b4 Bug 561944 - Give clearer/cleaner errors when validate_resource_hostname() throws. r=@mhammond