662471 - Uppercase directory names raises package name value error

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[arky [:arky]]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Git Master 

~/addon-sdk/bin/cfx run
Traceback (most recent call last):
  File "/Users/arky/addon-sdk/bin/cfx", line 29, in <module>
    cuddlefish.run()
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/__init__.py", line 679, in r
un                                                                             
    include_dep_tests=options.dep_tests
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 319, in 
generate_build_for_target                                                      
    add_section_to_build(target_cfg, "tests", is_code=True)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 294, in 
add_section_to_build                                                           
    validate_resource_hostname(name)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 78, in v
alidate_resource_hostname                                                      
    raise ValueError('package names need to be lowercase: %s' % name)
ValueError: package names need to be lowercase: jid1-9jt9biif6spzaa-Test-tests

Reproducible: Always

Comment 1

[arky [:arky]]

~/addon-sdk/bin/cfx init                                            
* lib directory created
* data directory created
* test directory created
* doc directory created
* README.md written
* package.json written
* test/test-main.js written
* lib/main.js written
* doc/main.md written

Your sample add-on is now ready.
Do "cfx test" to test it and "cfx run" to try it.  Have fun!

$ ~/addon-sdk/bin/cfx run 
No 'id' in package.json: creating a new ID for you.
package.json modified: please re-run 'cfx run'

$ ~/addon-sdk/bin/cfx run
Using binary at '/Applications/Firefox.app/Contents/MacOS/firefox-bin'.
Using profile at '/var/folders/Tt/TtQ5xD2-GR8gzUEqzYvByU+++TY/-Tmp-/tmpc9AJPp.mo
zrunner'.                                                                      
info: The add-on is running.
(works)

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Can we get a little more information about what you're doing that causes the problem?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

The error message is intentional, as package names do need to be lowercase.  But perhaps you're seeing it under circumstances in which you would not expect this error?  F.e. if your package name was lowercase, or if you saw it when you created an addon using "cfx init" (except that comment 1 appears to indicate that things worked fine when you used "cfx init").

Any additional information you can provide would be very helpful!

Comment 4

[arky [:arky]]

Here is the testcase

1. Make a directory name with Capital letters 

$  mkdir FOOBAR

2. Change and do cfx init 

$ cd FOOBAR; cfx init 

3. Running 'cfx' test raises errors

$ cfx test 
Traceback (most recent call last):
  File "/Users/arky/addon-sdk/bin/cfx", line 29, in <module>
    cuddlefish.run()
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/__init__.py", line 679, in run
    include_dep_tests=options.dep_tests
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 319, in generate_build_for_target
    add_section_to_build(target_cfg, "tests", is_code=True)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 294, in add_section_to_build
    validate_resource_hostname(name)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 78, in validate_resource_hostname
    raise ValueError('package names need to be lowercase: %s' % name)
ValueError: package names need to be lowercase: foobar-FOOBAR-tests

4. Running 'cfx run' also raises errors

$ cfx run
Traceback (most recent call last):
  File "/Users/arky/addon-sdk/bin/cfx", line 29, in <module>
    cuddlefish.run()
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/__init__.py", line 679, in run
    include_dep_tests=options.dep_tests
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 319, in generate_build_for_target
    add_section_to_build(target_cfg, "tests", is_code=True)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 294, in add_section_to_build
    validate_resource_hostname(name)
  File "/Users/arky/addon-sdk/python-lib/cuddlefish/packaging.py", line 78, in validate_resource_hostname
    raise ValueError('package names need to be lowercase: %s' % name)
ValueError: package names need to be lowercase: jid1-vhdudadeba9pig-FOOBAR-tests

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Yikes, indeed, that's a pretty bad initial experience with a command that we encourage users to use to build their first addon, and it seems likely to happen reasonably often, which makes this issue feel like a blocker.  Marking accordingly.

Brian: it looks like cfx init needs to make sure to lowercase the name of the package that it generates based on the name of the directory.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: lowercases name

Indeed, lowercasing the directory name when writing it to the "name" property in package.json seems to solve the problem.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Shouldn't .lower() be applied up above where 'addon' is defined, so that readme.md's "name" exactly matches package.json?

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

But yeah, cfx didn't complain about uppercase letters with Myk's patch applied.

Comment 9

[Brian Warner [:warner :bwarner]]

Attachment: v2: like v1, but retain mixed-case fullName, and add test

I see your patch, and raise you one unit test and a mixed-case fullName.

(your v1 patch is fine. If you'd rather go with the safer/smaller change, just apply it instead of reviewing my v2 form)

Comment 10

[Brian Warner [:warner :bwarner]]

Comment on attachment 537957 [details] [diff] [review]
patch v1: lowercases name

looks fine to me, but consider v2 instead

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 538067 [details] [diff] [review]
v2: like v1, but retain mixed-case fullName, and add test

This is small and safe enough and the better fix. r=myk

Comment 12

[Brian Warner [:warner :bwarner]]

Landed in https://github.com/mozilla/addon-sdk/commit/e5f80951c785a8d7ddfe0b209b1e6add1586f0cf . Should probably be cherry-picked to 1.0 branch.

Comment 13

[arky [:arky]]

Looks like the SDK has trouble directory names with spaces or unicode names.


* With spaces 
raise ValueError('package names cannot contain spaces or periods: %s' % name)

* With Unicode name (tức)

raise ValueError('invalid resource hostname: %s' % name)

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to comment #13)
> Looks like the SDK has trouble directory names with spaces or unicode names.
> 
> 
> * With spaces 
> raise ValueError('package names cannot contain spaces or periods: %s' % name)
> 
> * With Unicode name (tức)
> 
> raise ValueError('invalid resource hostname: %s' % name)

Should probably file that as a separate bug, as this one is fixed.