Closed Bug 464707 Opened 11 years ago Closed 11 years ago

port upload build target to comm-central apps

Categories

(MailNews Core :: Build Config, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: kairo, Assigned: sgautherie)

References

Details

(Keywords: fixed1.9.1, Whiteboard: [fixed1.9.1b99])

Attachments

(2 files)

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.
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.
Callek:
Why does |from mozilla.build.util import check_call| not work?
(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- )
(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.
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.
Blocks: 485820
(Is your review enough for the 3 apps?)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #376590 - Flags: review?(bugspam.Callek)
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?
Target Milestone: --- → mozilla1.9.2a1
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.
Attachment #376591 - Flags: review?(benjamin) → review?(ted.mielczarek)
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.
Attachment #376590 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 376590 [details] [diff] [review]
(Av1-CC) Add 'upload' target
[Checkin: Comment 11]


http://hg.mozilla.org/comm-central/rev/505c7833a920
Attachment #376590 - Attachment description: (Av1-CC) Add 'upload' target → (Av1-CC) Add 'upload' target [Checkin: Comment 11]
Attachment #376591 - Flags: review?(ted.mielczarek) → review+
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.
Attachment #376591 - Flags: approval1.9.1+
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)
Attachment #376591 - Attachment description: (Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/ → (Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/ [Checkin: Comment 13]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [needs 1.9.1 landing]
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) ;-)
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 landing]
Attachment #376591 - Attachment description: (Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/ [Checkin: Comment 13] → (Bv1-MC) 1 s/topsrcdir/MOZILLA_DIR/ [Checkin: Comment 13 & 14]
V.Fixed, per green SeaMonkey-Ports boxes.
Status: RESOLVED → VERIFIED
Whiteboard: [fixed1.9.1b5]
Blocks: 494676
Whiteboard: [fixed1.9.1b5] → [fixed1.9.1b99]
You need to log in before you can comment on or make changes to this bug.