464707 - port upload build target to comm-central apps

Status: VERIFIED FIXED

(MailNews Core :: Build Config, defect)

Description

[Robert Kaiser]

bug 454594 created an upload build target for Firefox, we should port this to comm-central apps.

Additionally, we should port the client.py simplification they did along with that.

Comment 1

[Justin Wood (:Callek)]

Note for client.py...

If we use util script in m-c, we need to adjust path
 |sys.path.append("mozilla")| and then |from build.util import check_call| like in other bug

Secondly, *if* we do that, we are essentially breaking new clones/checkouts as new checkouts won't have m-c to grab the file from, so we'd be best off cloning at least this part of |build/| to our repo, which feels like overkill just for this...

If the util file gains anything else for client.py that would be useful, I'd happily add it to our repo, but in the meantime I'm inclined to pass on that optimization.

Comment 2

[Robert Kaiser]

Callek:
Why does |from mozilla.build.util import check_call| not work?

Comment 3

[Justin Wood (:Callek)]

(In reply to comment #2)
> Callek:
> Why does |from mozilla.build.util import check_call| not work?

Two reasons.
#1) m-c may not be available yet, so it will fail out as module not found. (and we won't have a check_call to use at all)
#2) mozilla/ would need an __init__.py file in order to function as a module that python can import from (ie. being able to use it as |mozilla.|* )

The #1 is really the deal breaker for me here. (in any other file under c-c it would be fine, but for client.py that earns a r- )

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> #1) m-c may not be available yet, so it will fail out as module not found. (and
> we won't have a check_call to use at all)

Uh, right, thanks, I almost forgot that it's client.py we're talking about. In that light I agree that probably importing this file into our own build/ is probably a good idea.

Comment 5

[Robert Kaiser]

The new buildbots I just have set up with a bug 485820 configuration fail uploading because that build target is missing, we need this to be able to use the generic factories.

If the client.py stuff is still a plan, it should go into a separate bug as it's orthogonal.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-CC) Add 'upload' target [Checkin: Comment 11]

(Is your review enough for the 3 apps?)

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/ [Checkin: Comment 13 & 14]

Followup fix for bug 454594.

Comment 8

[Serge Gautherie (:sgautherie)]

Locally, with these two patches, I get
{{
SeaMonkey> make -C objdir upload

...
Error: required environment variable UPLOAD_HOST not set
}
as I do for FireFox.

KaiRo, can you try them on our new boxes and report if they're enough or if I missed something less obvious?

Comment 9

[Robert Kaiser]

Comment on attachment 376591 [details] [diff] [review]
(Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/
[Checkin: Comment 13 & 14]

Ted is a better reviewer choice for buildsystem stuff.

Comment 10

[Robert Kaiser]

Comment on attachment 376590 [details] [diff] [review]
(Av1-CC) Add 'upload' target
[Checkin: Comment 11]

Taking over review as I'm the comm-central build system owner anyway, and I have a config at hand that allows testing.

The patch is easy enough that I grant r=me for all apps and platforms on the basis that it (in combination with the m-c part) works fine for Linux SeaMonkey.

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 376590 [details] [diff] [review]
(Av1-CC) Add 'upload' target
[Checkin: Comment 11]


http://hg.mozilla.org/comm-central/rev/505c7833a920

Comment 12

[(not currently active) Ted Mielczarek]

Comment on attachment 376591 [details] [diff] [review]
(Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/
[Checkin: Comment 13 & 14]

a1.9.1+ for this trivial patch as long as it cycles green first.

Comment 13

[Robert Kaiser]

Comment on attachment 376591 [details] [diff] [review]
(Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/
[Checkin: Comment 13 & 14]

Pushed as http://hg.mozilla.org/mozilla-central/rev/42a36f6adadc - waiting for green build cycles before pushing to 1.9.1 (and sorry for missing the r= on that checkin message, I just used what Serge had in there and forgot to add this - corrected locally for the 1.9.1 checkin)

Comment 14

[Robert Kaiser]

Pushed to 1.9.1 as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/340c874848f6 which now and only now makes it really fixed (as comm-central is still officially based on 1.9.1) ;-)

Comment 15

[Serge Gautherie (:sgautherie)]

V.Fixed, per green SeaMonkey-Ports boxes.