471427 - upload en-US release builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Comment 1

[[e-mail] (:bhearsum)]

Attachment: fix upload target for win32 installer, complete mar, and optionally other files

This is mostly the same as the l10n-upload-% target I landed last week. We have to separate out the win32 installer otherwise we'll end up with empty quotes on other platforms, which is interpreted as the cwd. I couldn't do the same sort of logic with the MARs on this one since we don't have a MOZ_MAKE_COMPLETE_MAR sortof var here. Only checking for existence here is fine though, imo.

The UPLOAD_EXTRA_FILES portion is currently going to be used for the $platform_info.txt files we must upload for releases. We'll probably have other uses for it later, though.

Comment 2

[[e-mail] (:bhearsum)]

Attachment: buildbotcustom patch for uploading en-US release builds

This is similar to the l10n release build patch in that it subclasses the base build class for some nightly/release specific logic. The operations taking place for nightly builds have not changed at all - the upload step has just been moved to the subclass. The release builds are using the new upload target. I did not move the nightly builds to this in the interest of not making a big change right before I'll be gone for a week. I'll be moving them to the new target early in Q1.

The only other thing here is adding the 'env' parameter to make package/installer/mar so MOZ_PKG_PRETTYNAMES is applied correctly for release builds.

I gave this a go on staging and everything seemed to work properly.

Comment 3

[[e-mail] (:bhearsum)]

Attachment: master-side patch for en-US release build uploading

Not much to say about this one. It just gets nightly/release builds using the right Factory class and adds the appropriate parameters to the ReleaseBuildFactory calls.

Comment 4

[Chris Cooper [:coop] (he/him)]

Comment on attachment 354713 [details] [diff] [review]
buildbotcustom patch for uploading en-US release builds

> +            uploadEnv['UPLOAD_SSH_KEY'] = '~/.ssh/%s' % self.stageSshKey

Does it make sense for us to check whether this key actually exists before calling the upload target?

Comment 5

[[e-mail] (:bhearsum)]

(In reply to comment #4)
> (From update of attachment 354713 [details] [diff] [review])
> > +            uploadEnv['UPLOAD_SSH_KEY'] = '~/.ssh/%s' % self.stageSshKey
> 
> Does it make sense for us to check whether this key actually exists before
> calling the upload target?

It's kindof tough to, I'm inclined to say "meh".

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 354712 [details] [diff] [review]
fix upload target for win32 installer, complete mar, and optionally other files

I'm not really happy with calling upload.py multiple times. We should be able to let make calculate all of this, and pass it in one go. You should just be able to leverage $(wildcard) here, which will return an empty string if the file doesn't exist, ( http://www.gnu.org/software/make/manual/make.html#Wildcard-Function ) so something like:
$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) "$(DIST)/$(PACKAGE)"
$(wildcard $(DIST)/$(COMPLETE_MAR)) $(wildcard "$(INSTALLER_PACKAGE)")

Not sure about UPLOAD_EXTRA_FILES. Are you intending that to be a list of files, or just a single file name or what? If a list of files, then you'll probably need something like:
$(if $(UPLOAD_EXTRA_FILES),$(foreach f,$(UPLOAD_EXTRA_FILES),$(wildcard $(DIST)/$(f)))

Comment 7

[[e-mail] (:bhearsum)]

Attachment: upload target, all in one shot, UPLOAD_EXTRA_FILES fixed

Alright, I've implemented this the way you suggested. All files will be passed to upload.py at the same time, and the QUOTED_WILDCARD function you gave me fixes all of the problems I had trying to do this before.

You're absolutely right about UPLOAD_EXTRA_FILES, too, and I've fixed that.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 354871 [details] [diff] [review]
upload target, all in one shot, UPLOAD_EXTRA_FILES fixed

+# deal with them.
+space = $(empty) $(empty)

You should define $(empty) first, just:
empty :=
(I don't know that make really cares, but for sanity's sake.)

I think you should use line continuations on the upload.py line, probably split it after the first $(DIST), then one arg per line after that will make it easier to read.

Looks good, though!

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 354714 [details] [diff] [review]
master-side patch for en-US release build uploading

changeset:   634:41da1d8c2b2d

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 354713 [details] [diff] [review]
buildbotcustom patch for uploading en-US release builds

Checking in factory.py;
/cvsroot/mozilla/tools/buildbotcustom/process/factory.py,v  <--  factory.py
new revision: 1.67; previous revision: 1.66
done

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 354871 [details] [diff] [review]
upload target, all in one shot, UPLOAD_EXTRA_FILES fixed

checked in w/ nits fixed
m-c: changeset:   23195:be8e227b1d8a
1.9.1: changeset:   22557:c9b32bedc274