597837 - Confusing "ValueError: invalid resource hostname" when using cfx run with 'name' in package.json containing spaces

Status: RESOLVED FIXED

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

Description

[Nickolay_Ponomarev]

By mistake I put this in package.json:
{
    "name": "jetpack page-worker test",
    "id": "jid0-saEuBy3Ndh3oeuuYgnzj9ZS5Aqs"
}
To which 'cfx run' complained:

Traceback (most recent call last):
  File "/Users/nickolay/dev/jetpack-sdk/bin/cfx", line 6, in <module>
    cuddlefish.run()
  File "/Users/nickolay/dev/jetpack-sdk/python-lib/cuddlefish/__init__.py", line 501, in run
    include_dep_tests=options.dep_tests
  File "/Users/nickolay/dev/jetpack-sdk/python-lib/cuddlefish/packaging.py", line 267, in generate_build_for_target
    add_dep_to_build(dep)
  File "/Users/nickolay/dev/jetpack-sdk/python-lib/cuddlefish/packaging.py", line 254, in add_dep_to_build
    add_section_to_build(dep_cfg, "lib", is_code=True)
  File "/Users/nickolay/dev/jetpack-sdk/python-lib/cuddlefish/packaging.py", line 232, in add_section_to_build
    validate_resource_hostname(name)
  File "/Users/nickolay/dev/jetpack-sdk/python-lib/cuddlefish/packaging.py", line 65, in validate_resource_hostname
    raise ValueError('invalid resource hostname: %s' % name)
ValueError: invalid resource hostname: jid0-saeuby3ndh3oeuuygnzj9zs5aqs-jetpack page-worker test-lib


Where 'jid0-saeuby3ndh3oeuuygnzj9zs5aqs-jetpack page-worker test-lib' appears to autocomplete for each of the three "tokens" in the package.json's 'name', which confused me greatly.

Should cfx just validate package.json and complain if 'name' contains spaces?

(jetpack tip, http://hg.mozilla.org/labs/jetpack-sdk/rev/0754e897a883)

Comment 1

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

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".

Comment 2

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

I fixed something like this for mixed-case names in another bug, this shouldn't be any harder. Assigning to me.

Comment 3

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

Attachment: Throw a better error for spaces (and some other stuff)

This patch catches single spaces in the 'name', and throws a better error for that case. 

The patch also adds a link to https://jetpack.mozillalabs.com/sdk/1.0b3/docs/dev-guide/addon-development/package-spec.html which has more information on what 'name' requires for the errors from each of the three different tests.

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 521659 [details] [diff] [review]
Throw a better error for spaces (and some other stuff)

Can you change 1.0b3 to latest, and remove the reference to the bug (not really relevant)?

r=me with those changes, thanks!

Comment 5

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

(In reply to comment #4)
> Can you change 1.0b3 to latest, and remove the reference to the bug (not really
> relevant)?
Changing 1.0b3 to latest makes it redirect to https://jetpack.mozillalabs.com/sdk/1.0b4/docs/ no matter what follows "/docs/".

Comment 6

[Dietrich Ayala (:dietrich)]

Then we should remove the doc link. We don't want to be in a position where we have to update this every time there's a release.

Comment 7

[Dietrich Ayala (:dietrich)]

Hm, also can you file a bug for that doc problem? That sounds like bug.

Comment 8

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

Bug 645731 filed.

Comment 9

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

Attachment: patch without stationary links

This is the same patch as before, but it doesn't point users to the 1.0b3 version of the docs. Since we still can't point to /latest/somepage.html, I just dropped the link from the patch.

Carrying r+ forward, unless anyone objects.

I guess this should wait for the thaw before being landed.

Comment 10

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

Nice low-risk polish fix, a=myk for commission during freeze.

Comment 11

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

https://github.com/mozilla/addon-sdk/commit/6784e5f746658c9182de9ab97f4a27a0e5e89010

Comment 12

[Brian Warner [:warner :bwarner]]

incidentally, "if ' ' in name" would be a bit more readable/pythonic than "if name.find(' ') >= 0" .